-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
stmtdiagnostics: remove unused JSON trace datum #65583
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, noooooo! You are removing the traces from the bundle, which are super useful. 2d05a54 stopped writing it to the system table, not the bundle.
Reviewable status: complete! 0 of 0 LGTMs obtained
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @yuzefovich)
pkg/sql/explain_bundle.go, line 167 at r1 (raw file):
b.addDistSQLDiagrams() b.addExplainVec() traceJSON := b.addTrace()
I see - I need to remove unused local variable traceJSON
- my bad, will update shortly.
In 2d05a54 we stopped writing JSON trace datums to the system table, but some remnants of that work still remained. This commit cleans that up. Release note: None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, PTAL.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @yuzefovich)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 2 files at r1, 1 of 1 files at r2.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @yuzefovich)
TFTR! bors r+ |
Build succeeded: |
In 2d05a54 we stopped writing JSON
trace datums to the system table, but some remnants of that work still
remained. This commit cleans that up.
Release note: None