Skip to content

Keep an all-hex temp table name whole - #585

Merged
erikdarlingdata merged 2 commits into
devfrom
fix/temp-table-name-all-hex
Sep 28, 2026
Merged

erikdarlingdata merged 2 commits into
devfrom
fix/temp-table-name-all-hex

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 28, 2026 •

Copy link
Copy Markdown
Owner

What does this PR do?

CleanTempTableName no longer turns a short hex-like name into a bare #.

SQL Server stores a #temp table under an internal name. That name is the table's own name, padded with underscores to 116 characters, with a 12-character hex suffix after it. Plans show that internal name.

CleanTempTableName strips the padding and the suffix. It found the suffix by skipping trailing hex digits. For a name that is all hex after the #, that skip ran to the start of the name. So #deadbeef1 came back as #, and so did a table variable's internal name such as #A1B2C3D4.

PerformanceMonitor fixed this in its copy of the parser (commit b31e5d18, "Fix shared-lib defects found in Fable code review"). This PR ports the same guard:

  1. If no underscore comes immediately before the hex run, the name does not match the internal pattern. The function returns it unchanged.
  2. The function never returns a bare #.

The function has three callers, so the wrong name reached three places:

  • the table name that the plan shows on the operator (ShowPlanParser.RelOp.cs)
  • the table name in the missing index script (ShowPlanParser.Warnings.cs)
  • rule 28's temp table match (PlanAnalyzer.Detection.cs)

An audit of the analyzer code in the two repos found this item. The audit compares PerformanceStudio and PerformanceMonitor in both directions.

Which component(s) does this affect?

  • Desktop App (PlanViewer.App)
  • Core Library (PlanViewer.Core)
  • CLI Tool (PlanViewer.Cli)
  • SSMS Extension (PlanViewer.Ssms)
  • Tests
  • Documentation

How was this tested?

  • New TempTableNameTests, 10 cases:
    • Three internal names reduce to the table's own name. One of them is #deadbeef with its padding and suffix, so an all-hex table name still comes back whole.
    • Six names without padding come back unchanged: #deadbeef1, #A1B2C3D4, #OldUsers, #t, ##global and Users. Before this change, the first two came back as #.
    • An internal name with only padding between the # and the suffix comes back whole. Before this change, it came back as #. The review asked for this case.
  • The golden files did not change. No fixture plan has an all-hex temp table name.
  • Full suite before the added case: 903 passed, 0 failed, 1 skipped. The Release build has 0 compiler warnings. Windows only.

Checklist

  • I have read the contributing guide
  • My code builds with zero warnings (Release build through dotnet test)
  • All tests pass (dotnet test)
  • I have not introduced any hardcoded credentials or server names

🤖 Generated with Claude Code

https://claude.ai/code/session_01G625JBNh45iTR1hpT4CxNR

CleanTempTableName found an internal temp table name's hex suffix by skipping trailing hex
digits. For a name that is all hex after the #, such as #deadbeef1 or a table variable's
internal name like #A1B2C3D4, the skip ran to the start and the name came back as a bare #.
Ported from PerformanceMonitor b31e5d18: a name with no underscore padding before its hex
run is returned unchanged, and the function never returns a bare #.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G625JBNh45iTR1hpT4CxNR
@claude

claude Bot commented Sep 28, 2026

Copy link
Copy Markdown

Reviewed. This is a clean, minimal fix — traced the guard logic by hand against several inputs and it holds up:

  • #deadbeef1 / #A1B2C3D4 (no underscore before/at the hex run once trailing hex is skipped) → correctly bail out via the new i == 0 || name[i] != '_' check and return unchanged.
  • Normal padded internal names (#OldUsers + underscores + 12 hex) → still strip correctly to the visible name.
  • The pathological case of an all-underscore "visible name" (hex-skip lands on _, but the underscore-skip then walks all the way to i == 0) is now also guarded by i > 0 on the final check, so it falls through to returning the untouched original string instead of collapsing to "#". Good defense-in-depth given the input (plan XML) is untrusted and this attribute isn't otherwise validated.

Minor, non-blocking: that last all-underscore scenario isn't covered by a test case (the 6 "unchanged" cases all hit the earlier i == 0 guard from the hex-skip, not the i > 0 guard after the underscore-skip). Not required since the code already handles it correctly, but a 7th case would close the loop on coverage for the exact line changed on line 40.

No concerns with the three call sites (ShowPlanParser.RelOp.cs, ShowPlanParser.Warnings.cs, PlanAnalyzer.Detection.cs) — none build SQL from this value, so no injection surface here. No version bumps or SSMS project touched, so N/A there.

@claude

claude Bot commented Sep 28, 2026

Copy link
Copy Markdown

Reviewed. Traced the new guard logic against all test cases by hand — the i == 0 || name[i] != '_' early-out and the i > 0 re-check after stripping padding both do what the PR description claims, and the edge cases (all-hex name, name that's nothing but padding) are correctly covered by the new tests. CleanTempTableName's output only feeds a display label / rule-28 match string, not generated T-SQL, so no injection concern. No version-bump or linked-file conventions apply (doesn't touch PlanViewer.Ssms or PlanViewer.Web). Looks good.

@erikdarlingdata
erikdarlingdata merged commit 4d8a82b into dev Sep 28, 2026
3 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/temp-table-name-all-hex branch September 28, 2026 00:43
@erikdarlingdata erikdarlingdata mentioned this pull request Sep 29, 2026
2 of 8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant