Cursor replacement suggestions

Question

Post reply

Cursor replacement suggestions

frederico_fonseca

SSCoach

Points: 16154
More actions
August 22, 2021 at 10:26 pm
Answer
#3922645

alongside what has already been suggested (plenty of it and with indication of what how you could/should approach the issue).

hire a rather good SQL Developer to do this for you.

Viewing 15 posts - 31 through 45 (of 56 total)

You must be logged in to reply to this topic. Login to reply

Bruin SSCrazy Points: 2058 More actions · Answer 1

Any thoughts to how to improve this SP and the overall process?

Thanks.

Jeff Moden SSC Guru Points: 1004340 More actions · Answer 2

The code is RBAR and super-logging on steroids. It calls a proc to write to the log at virtually every point and it's also running a string splitter for every row. I understand why someone might want to make all dynamic SQL related variables NVARCHAR(MAX) but we'd need to check other places and make sure that they're not being used as criteria anywhere. It also calls a string parser multiple times for each row. I can only imagine what the string parser code looks like.

My thought is that it could be rewritten do it all and write multiple rows to the error log using a much more set based approach and that's what I'd aim for.

Without such a rewrite, the code is pretty much doomed to being slow and resource intensive.

And, no... this isn't going to be easy. Someone that is very well versed at writing imperative/procedural code wrote the code and it needs to be converted to set based logic along with the proc that's calling it. It IS one hell of a detailed logging system and I can't take that away from anyone.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

Bruin SSCrazy Points: 2058 More actions · Answer 3

Thanks for the assessment ... any thoughts of how to detangle the RBAR.

Phil Parkin SSC Guru Points: 246960 More actions · Answer 4

Bruin wrote:

Thanks for the assessment ... any thoughts of how to detangle the RBAR.

I don't think that we are in a position to take you much further with this task. The process is very complex and needs a fundamental rewrite in order to speed things up significantly.

Only someone who has

a) Full knowledge of what the existing code is doing

b) Full knowledge of how the process should (rather than 'does') work, and

c) A sound grip of how to write high-performing T-SQL

is in a position to get this job done well.

The absence of evidence is not evidence of absence
- Martin Rees
The absence of consumable DDL, sample data and desired results is, however, evidence of the absence of my response
- Phil Parkin

ScottPletcher SSC Guru Points: 101098 More actions · Answer 5

You could do some major clean up on your existing code:

1. Don't use nvarchar unless you absolutely must have it, use varchar instead.

2. Don't use (MAX) unless you absolutely must have it. Use a reasonable length for the data instead, one that can't be exceeded but is not ridiculously over-long.

3. Encode most of the strings to a text_code (or _id) equivalent, using a lookup table. This includes all the standard messaging strings, for Write_Error_Log, such as:

'INFORMATION' / 'Beginning procedure (Capture_Inline_Quality)' / 'Capture_Inline_Quality' / 'Declaring internal variables' / ...

Only custom text should be the full text, *all* standard text should be encoded. The text_code (or _id) would be NULL for custom text.

If you're unsure how to do that, I can walk thru doing this, while keeping the code clear.

4. Encode the Attribute column in the Equip_Attribute table to a single byte numeric code rather than using long strings like 'Inactive', 'Current Spool', etc.. Same would apply to the Quality_Test_Attribute table.

5. *Cluster* the Equip_Attribute table on ( Quality_Equip_ID, Attribute ). Similarly, *cluster* the Quality_Test_Attribute table on ( Test_ID, Attribute ) [remembering that Attribute should now be a tinyint code representing the string, not the full string itself].

6. Since Write_Error_Log is called *so many* times, need to see the code for it too. It needs to be very tight code, an INSERT and get out (although in the short term it could also do encoding of string values, until you get the main code changed).

There are a couple of more things, but I don't have time for them right now. Plus, you may not be interested in doing this at all, so no need for me to keep going until that's determined.

SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".

Bruin SSCrazy Points: 2058 More actions · Answer 6

I'm on board with your suggestions may need to help to get started with the process.

Thanks again...

frederico_fonseca SSCoach Points: 16154 More actions · Answer 7

don't waste time with small changes that will have minor impact on the process.

Do worry about trying to put on paper what the process should do to a whole set, instead of what it should do to a row or to a column.

you need to ignore all those that have a empty date or that are older than 4 hours? do so

you need to ignore all those that do not have a particular attribute? do so on the same query

-- insert BAD records onto error table

select *
into #errors
from (
select *
     , case when i1.inputdate is null or i1.inputdate < dateadd(hour, -4,getdate()) then 'Y' else 'N' as invalid_date
     , case when at.attributekey is null then 'Y' else 'N' as invalid_attribute
from input_table i1
left outer join attribute_table at
    on at.attributekey = i1.attribute
) t
where t.invalid_date = 'Y'
or t.Invalid_Attribute = 'Y'

-- now process the GOOD records

insert into my_main_table
select *

from (
select *
     , case when i1.inputdate is null or i1.inputdate < dateadd(hour, -4,getdate()) then 'Y' else 'N' as invalid_date
     , case when at.attributekey is null then 'Y' else 'N' as invalid_attribute
from input_table i1
left outer join attribute_table at
    on at.attributekey = i1.attribute
) t
where t.invalid_date = 'N'
and t.Invalid_Attribute = 'N'

Jeff Moden SSC Guru Points: 1004340 More actions · Answer 8

I'm with Frederico on this... I wouldn't try to salvage the old code. The only thing I'd take from the old code is what the overall goals for ALL the rows (i.e, the whole set) are.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

Bruin SSCrazy Points: 2058 More actions · Answer 9

If I was going to try a re-write, do you have any suggestions?

THx.

Phil Parkin SSC Guru Points: 246960 More actions · Answer 10

Bruin wrote:

If I was going to try a re-write, do you have any suggestions?
THx.

Frederico provided some.

The absence of evidence is not evidence of absence
- Martin Rees
The absence of consumable DDL, sample data and desired results is, however, evidence of the absence of my response
- Phil Parkin

Bruin SSCrazy Points: 2058 More actions · Answer 11

The 4 hour check was put in so if the process that is writing to the Import table has issues or is down, when restarted this

data could be outside of the scope of valid records. This shouldn't be a issue from the sql side as along as the RBAR is keeping pace with the rows hitting the Import queue.

Thanks.

Bruin SSCrazy Points: 2058 More actions · Answer 12

Bruin

SSCrazy

Points: 2058

August 22, 2021 at 9:15 pm

#3922637

ANy more suggestions for the re-write?

Jeff Moden SSC Guru Points: 1004340 More actions · Answer 13

Bruin wrote:

ANy more suggestions for the re-write?

If you want any more advice on this one, send money. 😀

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

ScottPletcher SSC Guru Points: 101098 More actions · Answer 14

ScottPletcher wrote:

You could do some major clean up on your existing code:
...6. Since Write_Error_Log is called *so many* times, need to see the code for it too. It needs to be very tight code, an INSERT and get out (although in the short term it could also do encoding of string values, until you get the main code changed).

I don't see the code for Write_Error_Log. There's no point in adjusting just part of the code, all of it needs reviewed or there's no point trying to adjust the code.

SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".