Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm previewing the data I'm about to delete, but this SQL looks a little redundant. Is there a better way to write this?

declare @history table(pid int)
insert into @history select pid from plans p where p.pidSynergy = 'P0022' and p.pid != 2885

select * from forecast_FTEs where pid in (select pid from @history)
select * from forecast_phases where pid in (select pid from @history)
select * from budget_FTEs where pid in (select pid from @history)
select * from budget_phases where pid in (select pid from @history)
select * from plans where pid in (select pid from @history)
share|improve this question
    
You should add a primary key to the table variable and only insert distinct not null values. Also consider using recompile so it takes account of correct table variable cardinality. –  Martin Smith Jul 25 at 20:01
    
Also consider habitually using two part naming for the column referenced in the inner part of the in to avoid this type of error. stackoverflow.com/questions/4594733/… –  Martin Smith Jul 25 at 20:10

2 Answers 2

p is a cryptic table alias and that != is not ANSI compliant, you should try and use <> instead. Also, the select * from is inefficient because it has to go back to the information schema and look up every column, you should try and only select the columns you really need.

share|improve this answer
    
Beat me to it. Welcome to Code Review. –  RubberDuck Jul 25 at 15:48
    
Thanks for the tips, but as this is an uncommon and manual action, i'm more interested in shorter code, perhaps by somehow creating an array of pids so i can just type where pid in pids. –  Cees Timmerman Jul 25 at 15:49
    
The reasoning for avoiding * is bogus. It would have to validate column metadata anyway. Whilst generally * should be avoided in production queries I wouldn't worry about it in adhoc interactive queries as described in the OP. Except if the table contains wide columns. –  Martin Smith Jul 25 at 20:05
up vote 3 down vote accepted

Not really.

  • LEFT JOIN returns an exponential amount of records, so is no good for SELECT. (540540 records in my case, 378378 after deleting 10 records of one table, saving 20 of 70 seconds)

  • INNER JOIN also leaves out incomplete entries. (360360 records in 48 seconds)

  • IN (a, b, c) is the same as OR, and is only efficient up to 64 items. The maximum number of values is limited by memory and time.

  • That leaves string hacking, which is also limited by length and might add character escaping problems.

TABLE is the only way to store an array in TSQL, so WHERE column IN (SELECT column FROM @table_var) is the shortest generic way to match a set of values.

share|improve this answer
    
In can handle more than 64 items. Though long in lists should generally be avoided. –  Martin Smith Jul 25 at 20:07
    
Good catch, I did not realize my query would return a cartesian product. I went and marked my answer as deleted since it is wrong. –  Phrancis Jul 25 at 20:22

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.