BrentOzarULTD / SQL-Server-First-Responder-Kit

sp_Blitz, sp_BlitzCache, sp_BlitzFirst, sp_BlitzIndex, and other SQL Server scripts for health checks and performance tuning.
http://FirstResponderKit.org
Other
3.37k stars 996 forks source link

sp_BlitzCache and sp_BlitzQueryStore -- Do we want more support for cursor plans? #1027

Closed BlitzErik closed 7 years ago

BlitzErik commented 7 years ago

We offer limited support for parsing cursor plans to look for optimistic, and non-forward only cursors.

Dynamic and Fast Forward cursors inhibit parallelism.

We can get the cursor reason here:

image

Or do the standard check, just under the StmtCursor

image

Assuming we even want this at all.

BlitzErik commented 7 years ago

Thinking this through a little bit, what if we just replace StmtCusor with StmtSimple in the plan xml? We'd only do this in the temp table for parsing, so the stuff we display from the global temp table wouldn't be altered. It would seem that all the checks we do on regular plan xml would then run on cursor plans too, without having to run a whole second set of checks or anything. Might have to look at if sql handle or whatever still works to link things together.

BrentOzar commented 7 years ago

That's genius!

On Jul 23, 2017, at 10:36 AM, BlitzErik notifications@github.com wrote:

Thinking this through a little bit, what if we just replace StmtCusor with StmtSimple in the plan xml? We'd only do this in the temp table for parsing, so the stuff we display from the global temp table wouldn't be altered. It would seem that all the checks we do on regular plan xml would then run on cursor plans too, without having to run a whole second set of checks or anything. Might have to look at if sql handle or whatever still works to link things together.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

BlitzErik commented 7 years ago

Well, maybe. It could also be very slow. I don't think replace value of works on /nodes/, only @values. So it'd be running `REPLACE(xmlcol, 'StmtCursor', 'StmtSimple') on XML :'(

BlitzErik commented 7 years ago

Ugh, yeah Msg 2356, Level 16, State 1, Line 36 XQuery [T.x.modify()]: The target of 'replace value of' must be a non-metadata attribute or an element with simple typed content, found 'element(Features,xdt:untyped) ?'