views:

65

answers:

2

I am hoping someone can help interpret the last code line (@Active=1....) for this SQL clause:

SELECT DISTINCT LOC_ID
    ,LOC_CODE
    ,ADDR_LINE_1
    ,ADDR_LINE_2
    ,ADDR_LINE_3
    ,CITY
    ,STATE
    ,COUNTRY
    ,POSTAL_CODE
    ,COMPANY
    ,OPERATION_TYPE
    ,PROCESS
        ,ADDR_LINE_1 + ',' +ADDR_LINE_2+ ',' + CITY + '-' + COUNTRY + ' ' + POSTAL_CODE AS ADDRESS
        ,COMPANY + '.' + CENTER + '.' + OPERATION_TYPE + '.' + PROCESS AS Proc_Combo
    ,CASE   WHEN INACTIVE_DATE IS NULL THEN 'Active'
        WHEN INACTIVE_DATE IS NOT NULL THEN 'InActive'
    ELSE 'UNKNOWN' END AS INACTIVE_DATE
    ,CASE   WHEN INSTANCE_ID = 1 THEN 'AP' 
        WHEN INSTANCE_ID = 2 THEN 'GAP'     
    ELSE 'ALL' END AS INSTANCE_ID
FROM HR_Locations
WHERE
    (@STATE IS NOT NULL AND STATE =@STATE OR @STATE='' AND STATE =STATE OR @STATE IS NULL AND 1=1) 
AND
    (@ACTIVE=1 AND INACTIVE_DATE IS NULL OR @ACTIVE=2 AND INACTIVE_DATE IS NOT NULL OR @ACTIVE=0 AND 1=1 OR @ACTIVE IS NULL)  

I have encountered code like this, only this is not contained in an "IF" block

+1  A: 

Not sure what the 1=1 is for ...

@ACTIVE=1 AND INACTIVE_DATE IS NULL OR
@ACTIVE=2 AND INACTIVE_DATE IS NOT NULL OR
@ACTIVE=0 AND 1=1 OR
@ACTIVE IS NULL

It appears as though the where clause is validating the @Active flag and the inactive date to make sure that if the status is active then the inactive is null and vise-versa.

Andy Evans
would you agree that an IF statement would have made this easier to read? (not bashing the original writer of the code, just want to know if keeping it simple is the way to go)
user279521
PLEASE bash the original writer of the code. It's terrible :).The technique itself isn't that bad (checking @Active states with ORs), but it, as written, is just unreadable. Needs parentheses and preferably to be spaced out across lines (one for each @active state).
MisterZimbu
@user It is fairly normal to express this sort of logic in the WHERE clause rather than an IF statement when working with SQL - it would have been helpful for the original developer to have used parenthesis however!
Kragen
Thanks @Kragen. That keeps my self confidence in tact!
user279521
+3  A: 

More easily understood when re-written as

(@ACTIVE=1 AND INACTIVE_DATE IS NULL)
OR (@ACTIVE=2 AND INACTIVE_DATE IS NOT NULL)
OR (@ACTIVE=0 AND 1=1)
OR @ACTIVE IS NULL

The part that seems interesting to me is (@ACTIVE=0 AND 1=1). This will AND the condition @ACTIVE=0 with the value TRUE. It seems to me it might be a typo. I've seen things like (@ACTIVE=0 OR 1=1) to make the condition sort of "optional", but in this case it looks unecessary. Can you try running they query without AND 1=1 to see if the results are the same?

FrustratedWithFormsDesigner
I will give it a try.
user279521
The query seems to be generated by code. In that case, '1=1' is a flag, if the 'variable' is written as '0' then that condition is not evaluated: as it returns false for any @active value, and is between ORs, then the where clause will return 'true' depending only on the other conditions.
Diego Pereyra
@Frustrated, the records returned count is the same !!
user279521
@user279521: hehe... ok so go ask the original developer for a *good* explanation!
FrustratedWithFormsDesigner