none
Migrating legacy code to 2008 R2 standards...

    Question

  • Hi ,

    I'm trying to rewrite the below code which uses tmp tables developed in 2000 version of SQL to 2008 R2 .

    any help in tuning query is appreciated..

    -- Load all ip_id and skill combinations from tblSwDM_stg_SYNONYM into #temp_Complete_Skill_List INSERT #temp_Complete_Skill_List SELECT DISTINCT sm.switch_id, s.value, max(s.item_name), max(s.descr) FROM tblSwDM_stg_SYNONYM s (NOLOCK) INNER JOIN tblSwDM_SwitchMaster sm (NOLOCK) ON s.cms_id = sm.cms_id AND s.acd_no = sm.acd_number WHERE s.item_type = 'split' GROUP BY sm.switch_id, s.value -- Append all additional ip_id and skill combinations from tblSwDM_stg_DSPLIT_UNIQUE into #temp_Complete_Skill_List INSERT #temp_Complete_Skill_List SELECT DISTINCT stg.ip_id, stg.split, NULL, NULL FROM tblSwDM_stg_DSPLIT_UNIQUE stg (NOLOCK) LEFT OUTER JOIN #temp_Complete_Skill_List tmp ON stg.ip_id = tmp.switch_id AND stg.split = tmp.skill WHERE tmp.switch_id IS NULL AND tmp.skill IS NULL ---- The above code will be later used to load the table below.. UPDATE tblSwDM_Dictionary SET end_effective_date = @end_effective_date FROM tblSwDM_Dictionary INNER JOIN #temp_Updated_Skill ON tblSwDM_Dictionary.row_id = #temp_Updated_Skill.row_id WHERE tblSwDM_Dictionary.end_effective_date IS NULL INSERT INTO tblSwDM_Dictionary ([dictionary_type_id], [switch_id], [value], [name], [description], [start_effective_date]) SELECT DISTINCT dictionary_type_id, switch_id, skill, NULLIF(skill_name, ''), NULLIF(skill_description, ''), @start_effective_date FROM #temp_Updated_Skill

    Thank you ,

    vishal.


    Tuesday, December 24, 2013 6:59 PM

Answers

  • This partial script should work just fine. I see no deprecated features being sued, so you expect it to behave the same.

    If you need it optimized, then start by timing each individual step, so you'll know what to focus on.

    Some general advice:

    • The DISTINCT keyword in the first query is confusing, since it already has a GROUP BY on the same columns
    • Make sure your temp table has proper indexes, because that is relevant for both the 2nd and 3rd query
    • It looks like the DISTINCT keyword in the 4th query is not needed. If so, then you should definitely remove it
    • The 3rd query does not need to use the proprietary UPDATE ... FROM syntax, and might benefit from a rewrite to standard SQL


    Gert-Jan

    Tuesday, December 24, 2013 8:10 PM
  • In addition to Gert-Jan's suggestions, take out those NOLOCK. If blocking is a concern, SQL 2008 offers a feature not available in SQL 2000: ALTER DATABASE db SET READ_COMMITTED_SNAPSHOT ON.

    When the database is in this mode, the default isolation level is implemented with the version store, which means a reader cannot block a write and vice versa. The query reads the data from the version store that was current when the query started running (except for functions called from the query which may read later data.)

    There are some considerations: expect tempdb usage to go up, because this where the version store lives. There are situations when reading from the version store is undesirable, particulary when you are reading data from validations. You can force the other behaviour with READCOMMITTEDLOCK.

    NOLOCK is any case a dangerous game, and you should never use it, unless you perfectly understand the implications. But this can be very difficult. Reading dirty data is one thing. But you can also fail to read committed data, and that is really bad.


    Erland Sommarskog, SQL Server MVP, esquel@sommarskog.se
    Tuesday, December 24, 2013 11:16 PM

All replies

  • This partial script should work just fine. I see no deprecated features being sued, so you expect it to behave the same.

    If you need it optimized, then start by timing each individual step, so you'll know what to focus on.

    Some general advice:

    • The DISTINCT keyword in the first query is confusing, since it already has a GROUP BY on the same columns
    • Make sure your temp table has proper indexes, because that is relevant for both the 2nd and 3rd query
    • It looks like the DISTINCT keyword in the 4th query is not needed. If so, then you should definitely remove it
    • The 3rd query does not need to use the proprietary UPDATE ... FROM syntax, and might benefit from a rewrite to standard SQL


    Gert-Jan

    Tuesday, December 24, 2013 8:10 PM
  • In addition to Gert-Jan's suggestions, take out those NOLOCK. If blocking is a concern, SQL 2008 offers a feature not available in SQL 2000: ALTER DATABASE db SET READ_COMMITTED_SNAPSHOT ON.

    When the database is in this mode, the default isolation level is implemented with the version store, which means a reader cannot block a write and vice versa. The query reads the data from the version store that was current when the query started running (except for functions called from the query which may read later data.)

    There are some considerations: expect tempdb usage to go up, because this where the version store lives. There are situations when reading from the version store is undesirable, particulary when you are reading data from validations. You can force the other behaviour with READCOMMITTEDLOCK.

    NOLOCK is any case a dangerous game, and you should never use it, unless you perfectly understand the implications. But this can be very difficult. Reading dirty data is one thing. But you can also fail to read committed data, and that is really bad.


    Erland Sommarskog, SQL Server MVP, esquel@sommarskog.se
    Tuesday, December 24, 2013 11:16 PM
  • >> I'm trying to rewrite the below code which uses TMP tables developed in 2000 version of SQL to 2008 R2<<

    How much can you do to upgrade the schema? You have a ton of ISO-111798 violations in the data element names. For example, the prefix “tbl-' is absurdly redundant and it is called “tbling” as a design flaw.

    We do not use temp tables in good SQL. This is how an old COBOL programmer fakes using a scratch tape instead of using a derived table, VIEW or CTE. You even do a step-by-step tape merge in SQL! 

    DISTINCT and GROUP BY are seldom used together. But we have no DDL or other specs, so we do not know. The correct form is INSERT INTO; INSERT is dialect.

    The use of NULLIF(skill_name, '') and NULL constant columns is usually a sign of bad design. How can the skill name be an empty string in a valid schema? Where is the explicit “unknown” value? Why create NULLs and then overwrite them in another process step? (answer: This is how we did data processing with punch cards in the 1960's!)

    The term “master” is not RDBMS; it is from tape files and network databases. 

    Why do you have multiple names for the same data element? What is the generic, vague “value”? Generic vague “skill” (skill_name, skill_level, skill_code, etc), “split”? Likewise, there is no such thing as a “type_id” in RDBMS. Do you have a “blood_type” or a “blood_type_id”? See the redundancy? People actually will say “blood_type_id_value” or worse. 

    Did you know that the 1970's Sybase UPDATE..FROM.. does not work or port? Today we might use a MERGE statement. 

    But my preference would be a 
    CREATE VIEW Complete_Skill_List (switch_id, something_value, item_name, something_description)
    so that it is always correct and current. It will also probably be faster than constantly doing disk seeks and updates. 

    Any help?

    --CELKO-- Books in Celko Series for Morgan-Kaufmann Publishing: Analytics and OLAP in SQL / Data and Databases: Concepts in Practice Data / Measurements and Standards in SQL SQL for Smarties / SQL Programming Style / SQL Puzzles and Answers / Thinking in Sets / Trees and Hierarchies in SQL

    Wednesday, December 25, 2013 12:28 AM
  • I would replace all the temp tables with CTEs, the syntax would not be wildly different, but it would be a LOT more efficient.

    Wow - I'd not seen NULLIF before, thanks for the education :)


    Christian Graus


    Wednesday, December 25, 2013 10:37 AM