-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RID Lookup operator shown as Key Lookup (Clustered) #49
Comments
Need more unit tests for plans that include |
In case this helps, here's a bunch of plans from PasteThePlan. You can do text search inside these for strings: http://14days.brentozar.com/pastedplans.zip That link will self-destruct in a couple of weeks. If you want to see how any of 'em look in the current (older) version of html-query-plan, take the file name and replace the one in the URL here, without sqlplan: |
@BrentOzar Nice - Thanks! |
* Add test plan for JustinPealing#49 * Add instructions to build minified and unminified versions * Add test plan for JustinPealing#50 * JustinPealing#50 - Add description for Adaptive Join node * JustinPealing#50 - Add placeholder icon for Adaptive Join * JustinPealing#50 - Don't show "Actual Join Type" on estimated plans * JustinPealing#50 - Add Estimated Join Type for Adaptive Joins * JustinPealing#50 - Add "Is Adaptive" to tooltips Also make sure that "Actual Join Type" doesn't show for other node types. * JustinPealing#50 - Add "Adaptive Threshold Rows" to tooltips * JustinPealing#50 - Add "Hash Keys Probe" to tooltip * JustinPealing#50 - Add "Outer References" to tooltip * Re-order properties in the tooltip * JustinPealing#49 - Correct node text for RID Lookup * JustinPealing#52 - add index kind to "Index Scan" nodes The @IndexKind attribute (if present) is appended to the end of the node title (e.g. "Clustered Index Seek (Clustered)"). Note: The special cases for Key Lookup and RID Lookup still need adjusting * Fix JustinPealing#51 - Node title for "Columnstore Index Scan" Implemented as a choose statement, as special cases for RID Lookup and Key Lookup special cases will also be added soon. * Add test cases for JustinPealing#49 * Refactor logic for Key Lookup and RID Lookup Combined with the logic that adds (@IndexKind) to the end of the node to make it more robust. * Add test plan for JustinPealing#54 * JustinPealing#54 - Add placeholder icon for Index Spool * Update README.md * Fix JustinPealing#54 - Add description for "Index Spool" node * JustinPealing#53 - Show missing indexes Missing indexes are shown underneath the query text in green. The CSS for these header rows needed adjusting to make sure that all rows (the query text and the indexes) don't just appear on top of each other, as each row needs to be absolutely positioned to ensure that the text is truncated properly, that all batches in the supplied XML are shown with the same width. * Update screenshot for 2.3 release * Add warning icon for JustinPealing#55 * Refactor icon handling This change now means there is a single template that makes the <div class="qp-icon-X" /> elements. This means I now have a single place to put logic for sub-icons (e.g. warning / parallel icons) * Remove UTF-16 from xml declarations These files aren't actually UTF-16 encoded (because then the unit tests fail) - having these incorrect declarations causes debugging in VS to fail. * JustinPealing#55 - Show warning icons Warning icons should be shown on any node with a s:Warnings element. The tooltip will show detail of what those warnings are (but doesn't quite yet) * Add parallel icon for JustinPealing#57 * Add parallel icon when @parallel=true For JustinPealing#57. Either 1 or true are valid values here. * Add "No Join predicate" warning to tooltip For #JustinPealing#56 * Add test case for JustinPealing#61 * Cached plan size is in KB, not B Fix JustinPealing#60 * Remove .xml from test plan filename + fix encoding * Start refactor ToolTipDetails to use call-template The template for the "Warnings" section needs access to the s:MissingIndexes element, which is unfortunately on the parent node if we match by s:Warnings. I can change the template to match on `*[s:Warnings]`, but then because we also match on */* when applying ToolTipDetails this means that we actually match on the previous node as well. I tried refacforing so that we don't need to match on */*, but that caused all sorts of issues. The short version is that using apply-templates when calling `ToolTipDetails` isn't a great idea, so I'm going to slowly refactor to using call-template instead. Its too big a change to do all at once, so I'm just going to use apply-template as well, as call-template and slowly refactor templates into the latter. * Use import instead of require for test plans * qp.xslt imported rather than required * Switch to `npm run test` instead of `npm run karma` * Load test plans in plans.js Turns out its going to be easier to move to TypeScript using the require syntax, not the import syntax. Its also easier if all of the loading of test plans is handled in a single file, rather than scattered over the test files. * Dity up karma.config.js * Replace usages of innerText with a helper function To make transition to typescript easier. * Convert to TypeScript * Update version number in package.json * Add some TypeScript annotations * Add type annotations to helper.ts * Add type annotations for tooltip.ts * Add type annotations to svgLines.ts * Fix JustinPealing#61 - Add unmatched indexes to tooltip The mode="QpTr" template matches on either s:RelOp or s:Stmt... nodes, however in the case of s:RelOp the s:Warnings element is a direct child but for s:Stmt... the s:Warnings element is actually a child of s:QueryPlan. For now I've worked around this by adding a hack to change context to the s:QueryPlan element, but a better solution would be to see if mode="QpTr" template should instead match on the s:QueryPlan element instead. * Fx warning icons for top level nodes Caused because we match on s:Stmt... nodes in the QpTr template, instead of matching on s:QueryPlan * Add test plan for JustinPealing#63 * update * change main entry * update original xslt
* Add test plan for JustinPealing#49 * Add instructions to build minified and unminified versions * Add test plan for JustinPealing#50 * JustinPealing#50 - Add description for Adaptive Join node * JustinPealing#50 - Add placeholder icon for Adaptive Join * JustinPealing#50 - Don't show "Actual Join Type" on estimated plans * JustinPealing#50 - Add Estimated Join Type for Adaptive Joins * JustinPealing#50 - Add "Is Adaptive" to tooltips Also make sure that "Actual Join Type" doesn't show for other node types. * JustinPealing#50 - Add "Adaptive Threshold Rows" to tooltips * JustinPealing#50 - Add "Hash Keys Probe" to tooltip * JustinPealing#50 - Add "Outer References" to tooltip * Re-order properties in the tooltip * JustinPealing#49 - Correct node text for RID Lookup * JustinPealing#52 - add index kind to "Index Scan" nodes The @IndexKind attribute (if present) is appended to the end of the node title (e.g. "Clustered Index Seek (Clustered)"). Note: The special cases for Key Lookup and RID Lookup still need adjusting * Fix JustinPealing#51 - Node title for "Columnstore Index Scan" Implemented as a choose statement, as special cases for RID Lookup and Key Lookup special cases will also be added soon. * Add test cases for JustinPealing#49 * Refactor logic for Key Lookup and RID Lookup Combined with the logic that adds (@IndexKind) to the end of the node to make it more robust. * Add test plan for JustinPealing#54 * JustinPealing#54 - Add placeholder icon for Index Spool * Update README.md * Fix JustinPealing#54 - Add description for "Index Spool" node * JustinPealing#53 - Show missing indexes Missing indexes are shown underneath the query text in green. The CSS for these header rows needed adjusting to make sure that all rows (the query text and the indexes) don't just appear on top of each other, as each row needs to be absolutely positioned to ensure that the text is truncated properly, that all batches in the supplied XML are shown with the same width. * Update screenshot for 2.3 release * Add warning icon for JustinPealing#55 * Refactor icon handling This change now means there is a single template that makes the <div class="qp-icon-X" /> elements. This means I now have a single place to put logic for sub-icons (e.g. warning / parallel icons) * Remove UTF-16 from xml declarations These files aren't actually UTF-16 encoded (because then the unit tests fail) - having these incorrect declarations causes debugging in VS to fail. * JustinPealing#55 - Show warning icons Warning icons should be shown on any node with a s:Warnings element. The tooltip will show detail of what those warnings are (but doesn't quite yet) * Add parallel icon for JustinPealing#57 * Add parallel icon when @parallel=true For JustinPealing#57. Either 1 or true are valid values here. * Add "No Join predicate" warning to tooltip For #JustinPealing#56 * Add test case for JustinPealing#61 * Cached plan size is in KB, not B Fix JustinPealing#60 * Remove .xml from test plan filename + fix encoding * Start refactor ToolTipDetails to use call-template The template for the "Warnings" section needs access to the s:MissingIndexes element, which is unfortunately on the parent node if we match by s:Warnings. I can change the template to match on `*[s:Warnings]`, but then because we also match on */* when applying ToolTipDetails this means that we actually match on the previous node as well. I tried refacforing so that we don't need to match on */*, but that caused all sorts of issues. The short version is that using apply-templates when calling `ToolTipDetails` isn't a great idea, so I'm going to slowly refactor to using call-template instead. Its too big a change to do all at once, so I'm just going to use apply-template as well, as call-template and slowly refactor templates into the latter. * Use import instead of require for test plans * qp.xslt imported rather than required * Switch to `npm run test` instead of `npm run karma` * Load test plans in plans.js Turns out its going to be easier to move to TypeScript using the require syntax, not the import syntax. Its also easier if all of the loading of test plans is handled in a single file, rather than scattered over the test files. * Dity up karma.config.js * Replace usages of innerText with a helper function To make transition to typescript easier. * Convert to TypeScript * Update version number in package.json * Add some TypeScript annotations * Add type annotations to helper.ts * Add type annotations for tooltip.ts * Add type annotations to svgLines.ts * Fix JustinPealing#61 - Add unmatched indexes to tooltip The mode="QpTr" template matches on either s:RelOp or s:Stmt... nodes, however in the case of s:RelOp the s:Warnings element is a direct child but for s:Stmt... the s:Warnings element is actually a child of s:QueryPlan. For now I've worked around this by adding a hack to change context to the s:QueryPlan element, but a better solution would be to see if mode="QpTr" template should instead match on the s:QueryPlan element instead. * Fx warning icons for top level nodes Caused because we match on s:Stmt... nodes in the QpTr template, instead of matching on s:QueryPlan * Add test plan for JustinPealing#63 * update * change main entry * update original xslt
Raised in microsoft/azuredatastudio#391
The text was updated successfully, but these errors were encountered: