Right Join vs Left Join - My joins are broken and I don't understand why

Question

http://sqlfiddle.com/#!3/ef71e/1

I rather drastically cut down the fiddle, but I think the intent shines through. [Def] is a [DefaultClassification] table, and [Cls] is a [Classification] table that has some old records that need to be added in. The DefaultClassification table will be used in the future to spawn new groups of records into yet another table, and we're "unbreaking" an existing bit of logic/data with this process.

I did my best to distill the entire thing down to the barest essentials, but I've got another similar process that runs right after this one, so I'm looking to learn best practices and why this query is broken.

When I wrote this I intended it to run in a 2008+ environment, scripted or manually in SSMS. I don't know that it won't be scripted, but it might. Right now it's entirely run by hand. The rollback and select at the top above the print messages at the end are because it doesn't work right, so this lets me validate it before committing. I would prefer a valid script that I don't have to muck with, however

The specific issue I have is on line 62 of that revision one link http://sqlfiddle.com/#!3/ef71e/1 and it looks like this:

RIGHT OUTER JOIN [Def] tcd

if I flip that RIGHT to a LEFT it does the insert but doesn't detect duplicates (to prevent insertion) and if I do the RIGHT it detects duplicates but doesn't handle insertions.

What did I break? And Why?

---

And I'm told the use of the @@ERROR was bad juju, but I don't know why, so bonus points (a 100 rep bounty) for that explanation too.

Answer 1

Why is @@ERROR bad juju?

In SQL Server 2005, using @@ERROR makes life harder than it needs to be for detecting and trapping errors.

1) You must test after every statement. Just from a maintenance perspective, my dog the goggles will do nothing to protect you. You can also argue that all the copy/paste logic for error handling will obscure the "true" purpose of your code.

2) @@ERROR is too volatile. In the following code, line 2 throws error 8134. Line 3 I select the value and 8134 is consumed. Why? Because the SELECT statement didn't raise an error. Even if it had, it would have been a different error than the one that failed.

DECLARE @ErrorCode int
SELECT 1/0 AS div_zero
SELECT @@error AS [@@error]
SET @errorCode = @@error
SELECT @errorCode AS errorCode

3) Even if you handle the @@ERROR well with the above 2, my challenge around it is that error still "comes through" at least in SSMS. Run the first half of the query and even though I have logically handled the error, it will still percolate back to the caller.

SET NOCOUNT ON

DECLARE @ErrorCode int;
SELECT 1/0 AS div_zero;
SET @errorCode = @@error;
IF @ErrorCode <> 0
BEGIN
    -- I did something here to handle the error
    -- SSMS still reports that the Query completed with errors
    print 'developer electrocuted';
END
GO

Contrast that with

-- error handled *and* query executes successfully
BEGIN TRY
    -- This will report query executed successfully
     SELECT 1/0 AS handled_div_zero;
END TRY
BEGIN CATCH
SELECT
    ERROR_NUMBER()AS error_number --returns the number of the error.
,   ERROR_SEVERITY() AS error_severity --returns the severity.
,   ERROR_STATE()AS error_state  --returns the error state number.
,   ERROR_PROCEDURE() AS error_procedure --returns the name of the stored procedure or trigger where the error occurred.
,   ERROR_LINE() AS error_line --returns the line number inside the routine that caused the error.
,   ERROR_MESSAGE() AS error_message; --returns the complete text of the error message. The text includes the values supplied for any substitutable parameters, such as lengths, object names, or times.
END CATCH

SQL Server 2012 finishes what 2005 started and provides THROW This allows you to handle your error in your CATCH block and then instead of smashing your call stack, you are now able to percolate that same error back up to the caller. Otherwise, you're reduced to having to raiserror each time you try to fix a level in the call stack.

Answer 2

Ok, I wrote an article on SQL Server Central here that may help you with your left/right outer join questions. However if you are trying to remove duplicates in this case I think what you are looking for is a NOT EXISTS.

SELECT 
      tTCD.ClsID
    , tTCD.ParID
    , tTCD.Name
    , tTCD.IsExpense
    , tTCD.CD
FROM @Default tTCD
WHERE NOT EXISTS (
        SELECT 1 FROM [Def] tcd
            WHERE  tcd.ClsID = tTCD.ClsID
            AND tcd.Name = tTCD.Name
            AND tcd.IsExpense = tTCD.IsExpense)
AND tTCD.IsExpense IS NOT NULL

Your existing code using a RIGHT OUTER JOIN has a WHERE clause on the LEFT table. Since you are excluding NULLs and on a RIGHT OUTER JOIN any time there is not a match you will get a NULL on the LEFT table. Using the NOT EXISTS should help more. However your test data doesn't actually have any duplicates so it's hard to tell.

Answer 3

You cannot just swap right and left joins. They mean two different things. You also need to understand the effect of having a where condition on the table on the left side of a right join or the right side of a left join. See this link to understand why this changes teh join to an inner join. http://wiki.lessthandot.com/index.php/WHERE_conditions_on_a_LEFT_JOIN

See if this code is what you want:

 SELECT 
          tTCD.ClsID
        , tTCD.ParID
        , tTCD.Name
        , tTCD.IsExpense
        , tTCD.CD
   FROM  [Def] tcd
   LEFT OUTER JOIN @Default tTCD
        ON  tcd.ClsID = tTCD.ClsID
        AND tcd.Name = tTCD.Name
        AND tcd.IsExpense = tTCD.IsExpense
        AND tTCD.IsExpense IS NOT NULL

For the future is is generally best to use only left joins.