current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 1 down vote favorite

I have the following query:

SELECT * 
INTO ##TempStaffT
FROM Staff HF 
WHERE  HF.[businessline_id]='T'
AND HF.[offices_id] IN (SELECT * FROM ##TempParamOffice)
AND HF.[specialism_id]IN (SELECT * FROM ##TempParamSpecialism)
AND HF.[onetouch]IN (SELECT * FROM ##TempParamConsultant)

 SET @sql4 = N'

INSERT INTO ##TempFees(' + @columns1 + ',CALF_AN ,BusinessName)
SELECT distinct ' + @columns2 + ',p.CALF_AN ,CB.[TempBusinessName] as BusinessName
   FROM 
     dbo.WTEFAC EF 
  inner JOIN  dbo.WTFAC F ON EF.EFAC_NUM = F.EFAC_NUM 
  inner JOIN  dbo.WTFACINFO BS ON F.FAC_NUM = BS.FAC_NUM 
  inner JOIN dbo.WTLFAC LF ON F.FAC_NUM = LF.FAC_NUM 
  inner JOIN dbo.WTRUBVARIANTEFAC WRU ON LF.RINT_ID = WRU.RINT_ID 
  inner JOIN dbo.WTACUMFAC WTA ON WRU.RUV_ID = WTA.RUV_ID 
  inner JOIN ##CUM_CODEHT WTA1 ON WTA.CUM_ID = WTA1.CUM_ID 
  inner JOIN dbo.WTVTAT TAT ON BS.TIE_ID = TAT.TIE_ID AND BS.RFAN_ID = TAT.RFAN_ID AND BS.PER_ID = TAT.PER_ID AND BS.CNT_ID = TAT.CNT_ID
  inner JOIN dbo.PYCONTRAT CC ON TAT.PER_ID = CC.PER_ID AND TAT.CNT_ID = CC.CNT_ID 
  inner JOIN dbo.WTMISS M ON CC.PER_ID = M.PER_ID AND CC.CNT_ID = M.CNT_ID 
  inner JOIN dbo.WTCNTI COT1 ON M.PER_ID = COT1.PER_ID AND M.CNT_ID = COT1.CNT_ID
  inner JOIN dbo.WTQUAEU Q ON COT1.TIE_ID = Q.TIE_ID AND COT1.QEU_CDE = Q.QEU_CDE
  inner JOIN dbo.WTSCCT C ON CC.RGPCNT_ID = C.RGPCNT_ID AND CC.PER_ID = C.PER_ID AND CC.CNT_ID = C.CNT_ID --AND''SECT3'' = C.STTQ_COD 
  INNER JOIN ##TempStaffT HF ON C.VAPO_CODE = HF.onetouch  COLLATE Latin1_General_CI_AS
  inner JOIN 
##TempA AS p  ON p.CNT_ID = COT1.CNT_ID  inner JOIN 
 ##TempB AS p1  ON p1.CNT_ID = COT1.CNT_ID
  CROSS JOIN [dbo].[CustBusinessTable] CB
WHERE  CB.[TempBusinessName]=''Pure Temp Fees''

GROUP BY p.CALF_AN,CB.[TempBusinessName]
'
;
PRINT @sql4;

My problem is that the query above took 20 min to be executed because ##TempStaffT contains too many rows. How can I optimize it? I use many temp tables but it seems to not be working.

share|improve this question
1  
You have a query, but you have not specified what it does at all. Please explain a bit about what your query does, and what all the cryptic names and aliases mean. –  Simon André Forsberg 2 hours ago

1 Answer 1

up vote 3 down vote accepted

Code Review Stack Exchange advice

It really helps if you explain, in plain English, what your code does or is supposed to do. As it stands, nobody has any idea what your code is supposed to do, and the best we can do is guess. With that in mind...

Nitpicks

Your aliases make no sense. Your table names are cryptic enough (you probably can't change that) but to alias WTFAC as F certainly doesn't help. WTFACINFO as BS to me only suggests this: WTF BS.

Your capitalization of SQL keywords is all over the place. Make it all caps, or all lower case, or whatever. Just stick to one style and be consistent.

Your indentations are not very helpful; in fact I find them confusing at best. Use indentations that are meaningful to make the code easier to sift through. For example, this:

inner JOIN dbo.WTVTAT TAT ON BS.TIE_ID = TAT.TIE_ID AND BS.RFAN_ID = TAT.RFAN_ID AND BS.PER_ID = TAT.PER_ID AND BS.CNT_ID = TAT.CNT_ID

Why not instead write:

INNER JOIN dbo.WTVTAT TAT 
    ON BS.TIE_ID = TAT.TIE_ID 
    AND BS.RFAN_ID = TAT.RFAN_ID 
    AND BS.PER_ID = TAT.PER_ID 
    AND BS.CNT_ID = TAT.CNT_ID

END YOUR SQL STATEMENTS PLEASE

Not using the delimiter ; throughout your query makes it much more difficult to understand the logic of your code. Use ; and even GO if need be. MS SQL Server is forgiving on syntax errors (try PostgreSQL for fun), that doesn't mean you shouldn't follow good practices of ending statements properly.

SELECT *

SELECT * 
INTO ##TempStaffT
FROM Staff HF

enter image description here

Do you really need every column from this table? Perhaps you do, but more likely not. This could improve performance if you are more verbose about selecting exactly the columns you want and no more.
SELECT * makes the SQL engine query the information schema to get all the information about every column in the table, it can slow things down.

CROSS JOIN!?

This:

CROSS JOIN [dbo].[CustBusinessTable] CB

My view is that, unless you know you need a CROSS JOIN, you don't need a cross join. They are insanely slow and that is likely why your query takes so long. If you indeed know you need the cartesian product of [dbo].[CustBusinessTable] and ##TempB then I guess you're out of luck. But I have a feeling there is a better way to do this.

share|improve this answer

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.