July 3, 2014 at 9:13 am
Edit: Actually title should be returns true when should false.
Am I doing this all wrong? I want to check a table to see if a record already exists, if it doesn't then insert it, else do nothing:
IF NOT EXISTS
(SELECT 1 FROM Table1 WHERE col1 = 'Test')
BEGIN
INSERT INTO Table1 (col1) VALUES ('Test')
END
The value 'Test' is already in the database yet, the code is saying it's not and trying to insert it, resulting in duplicate key errors.
Am I missing something here?
July 3, 2014 at 9:19 am
The exact code you have posted is absolutely correct.
It might be something else in your real code which makes it fail.
Also, I would recommed to write such inserts with WHERE clause instead of IF, something like:
INSERT Table1 (col1)
SELECT 'Test'
WHERE NOT EXISTS(SELECT 1 FROM Table1 WHERE Col1 = 'Test')
July 3, 2014 at 9:20 am
CREATE TABLE #Table1 (col1 VARCHAR(10))
IF NOT EXISTS
(SELECT 1 FROM #Table1 WHERE col1 = 'Test')
BEGIN
INSERT INTO #Table1 (col1) VALUES ('Test')
END
Looks fine, works as expected (inserts the first time, never again)
Is this a simplification of some real code? Can you post the real code?
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
July 3, 2014 at 9:34 am
An alternative worth considering, since you have a primary key constraint, is to skip the existence check, and instead to handle the error when it occurs.
John
July 3, 2014 at 9:45 am
Here is the code. What is weird is, I can get this to work as expected on my test system and if I write out the IF NOT EXIST part explicitly on the production system, but the procedure below on the production system doesn't work.
This is the cut down code for the procedure. It has a lot more to it than this, and I realise I could do it in a different way without having to check each row, but this is a one time job to fix an issue and the procedure below gives the jist of the problem!
I need to compare what is in one table called RawDate_Impprt, with a table called UniqueCodes and insert any codes and their tracking reference if they are missing.
Rawdata_import table holds a code value and a tracking reference.
CREATE TABLE RawData_Import
(ID int identity (1,1),
Code nvarchar(20),
TrackingReference bigint)
CREATE TABLE UniqueCodes
(Code nvarchar(20),
TrackingReference bigint)
INSERT INTO RawData_Import VALUES ('aaa', 1),('bbb', 2), ('ccc', 3);
INSERT INTO UniqueCodes VALUES ('bbb',2)
Procedure:
USE [CodeDb]
GO
/****** Object: StoredProcedure [dbo].[sp_UcodeImport_ProcessData] Script Date: 07/03/2014 15:11:53 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE PROCEDURE [dbo].[sp_UcodeImport_ProcessData]
AS
BEGIN
SET NOCOUNT ON;
DECLARE @uCode nvarchar(20),
@TrackingRef bigint,
@codeCnt int;
SET @codeCnt = 0;
-- Get the count of the codes to process.
SELECT @codeCnt = count(*) FROM [CodeDB].[dbo].[Rawdata_Import];
-- Loop through each code in raw table and process:
WHILE 1 <= @codeCnt
BEGIN
BEGIN TRY
BEGIN TRANSACTION
SET @uCode = null;
-- Get the code
SELECT @uCode = Code, @TrackingRef = TrackingReference
FROM [CodeDB].[dbo].[Rawdata_Import]
WHERE ID = @codeCnt;
-- Check if code exists in UniqueCode table.
IF NOT EXISTS (SELECT 1
FROM [CodeDB].[dbo].[UniqueCodes]
WHERE Code = @uCode)
BEGIN
-- Code not found, so insert the new code
INSERT INTO [CodeDB].[dbo].[UniqueCodes]
(Code,TrackingReference)
VALUES
(@uCode, @TrackingRef)
END
-- Commit the transaction
COMMIT;
END TRY
-- Catch any errors:
BEGIN CATCH
-- Rollback any work done.
ROLLBACK
END CATCH
-- Reduce the count to do the next code:
SET @codeCnt = @codeCnt -1;
END
END
Executing the above code on a test system works, annoyingly!, but on the live system doing exactly the same thing (just more work after the IF EXISTS check), doesn't work. It just sees any codes which do exist as not existing.
Very strange!
July 3, 2014 at 9:47 am
John Mitchell-245523 (7/3/2014)
An alternative worth considering, since you have a primary key constraint, is to skip the existence check, and instead to handle the error when it occurs.John
That is true, the catch block of my code would mean I could just skip the existing data if duplicate key error was thrown. I would still like to understand what is going on though!! 🙂
July 3, 2014 at 9:58 am
Not sure about the exists, but it doesn't look like a loop is needed at all...
CREATE TABLE #RawData_Import
(ID int identity (1,1),
Code nvarchar(20),
TrackingReference bigint)
CREATE TABLE #UniqueCodes
(Code nvarchar(20),
TrackingReference bigint)
INSERT INTO #RawData_Import VALUES ('aaa', 1),('bbb', 2), ('ccc', 3);
INSERT INTO #UniqueCodes VALUES ('bbb',2)
CREATE PROCEDURE UcodeImport_ProcessData
AS
BEGIN
BEGIN TRY
BEGIN TRANSACTION
INSERT INTO #UniqueCodes (Code,TrackingReference)
SELECT Code, TrackingReference
FROM #Rawdata_Import ri
WHERE NOT EXISTS (SELECT 1 FROM #UniqueCodes uc WHERE ri.code = uc.Code)
COMMIT;
END TRY
BEGIN CATCH
-- Handle the error, don't ignore it!
ROLLBACK
END CATCH
END
p.s. Don't name procedures sp_*, they're not system procedures, they shouldn't get the system procedure prefix
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
July 3, 2014 at 10:00 am
This may be the issue:
WHERE ID = @codeCnt;
Something tells me that the value of ID does not correspond to the value of @codeCnt
ID is an identity. How many times have you inserted and deleted records from this table? ID does not start over at 1 unless you re-seed the table.
And, Gail's code will work a lot better than what you wrote.
Michael L John
If you assassinate a DBA, would you pull a trigger?
To properly post on a forum:
http://www.sqlservercentral.com/articles/61537/
July 3, 2014 at 10:04 am
Not sure about the exists, but it doesn't look like a loop is needed at all...
As well as there is no need in transaction control over single insert statement...
Something tells me that is not a full story from OP...
July 3, 2014 at 2:19 pm
Thanks Gail, that's very useful example. My only reason for doing a loop is that there is extra work being done before the insert which I've excluded for clarity. I.e there are extra columns in the final insert table and the data for those columns has to come from another table based on the original raw data, as well as some updates of other tables. Oh, and I know I must not use SP for non system procedures, so feel very embarrassed for sticking that in! I've even told off some of our developers for doing it in the past! I am by no means a SQL expert, but I should have spotted my lazy typing!:blush:
Michael, the identity value isn't an a problem in my initial example as I have only updated the raw data once so far, but it isn't something I noticed and will need to take into account if this type of work is repeated. Thanks for that.
SSCrazy, you're correct, there is a lot more to this, but I didn't want to cloud the problem with all the extra details. As mentioned above there is other work being done before the insert and some of that is to update reference tables etc. If the final insert fails, I need to rollback those changes across all tables, hence wrapping this in a transaction.
I'm still not sure why I cant get the basic premise of the not exists to work in the procedure, even though if I take out just the problem code and get it to print "found" or "not found" in a separate query it works fine. Using the SSMS debugger I can see that the first value it searches for it finds in the table so ignores it and goes round the loop to the next value, but even though this exists in the base table, it doesn't find it. I'll keep plugging away and see what I can find, but I may just do as suggested and use the constraint to ignore the duplicate data.
Thanks all for you help.
July 3, 2014 at 2:29 pm
Could be there's something in the code you left out which is doing something odd. But we can't tell.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
July 3, 2014 at 6:25 pm
Maddave (7/3/2014)
John Mitchell-245523 (7/3/2014)
An alternative worth considering, since you have a primary key constraint, is to skip the existence check, and instead to handle the error when it occurs.John
That is true, the catch block of my code would mean I could just skip the existing data if duplicate key error was thrown. I would still like to understand what is going on though!! 🙂
Sure... and you could even make the unique index with the "IGNORE_DUP_KEY" option turned on and remove the need for a catch. But, programming by exception can be costly especially if a rollback occurs. I strongly recommend against it.
--Jeff Moden
Change is inevitable... Change for the better is not.
July 3, 2014 at 7:13 pm
Might be worth making sure you're even getting to that part of the code. Sometimes code gets complicated enough that the branching you THINK is happening, isn't happening the way you were thinking it was.
I'd put that "found/not found" directly in the SP, and see what you might be getting then.
----------------------------------------------------------------------------------
Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?
July 4, 2014 at 3:26 am
Success! The problem was in the raw data!! I was using a bulk insert to populate the raw data table, which came from a text file. There was an extra tab after each line which was getting added on to the end of each code. So, when it was searching for it, it wasn't found. I've removed the tab and now it works fine. I didn't think a blank space of type "tab" would be added to the varchar data type and cause it to have extra white space at the end.
Anyway, thanks for all your help, I've learnt lots of other stuff here on how best to do this type of thing too.
Viewing 14 posts - 1 through 13 (of 13 total)
You must be logged in to reply to this topic. Login to reply