Skip to content
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

Add index kind to index scan nodes #52

Closed
JustinPealing opened this issue Dec 26, 2017 · 0 comments
Closed

Add index kind to index scan nodes #52

JustinPealing opened this issue Dec 26, 2017 · 0 comments
Milestone

Comments

@JustinPealing
Copy link
Owner

SSMS shows the index kind in brackets to index scan operations, e.g.

image

If this information is not present in the plan XML this is not shown in the node label

image

Want to replicate this behaviour in html-query-plan

@JustinPealing JustinPealing added this to the 2.3 milestone Dec 26, 2017
JustinPealing added a commit that referenced this issue Dec 26, 2017
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
anthonydresser pushed a commit to anthonydresser/html-query-plan that referenced this issue Jan 18, 2018
* 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
anthonydresser pushed a commit to anthonydresser/html-query-plan that referenced this issue Nov 16, 2019
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant