-
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
opt: fix crdb_internal.decode_plan_gist to work with unknown index #84682
Conversation
Release note (bug fix): crdb_internal.decode_plan_gist will no longer produce an internal error when it is used to decode a plan gist for which no schema information is available.
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 2 of 2 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @cucaroach and @rytaft)
pkg/sql/opt/exec/explain/plan_gist_factory.go
line 496 at r1 (raw file):
func (u *unknownTable) Index(i cat.IndexOrdinal) cat.Index { return &unknownIndex{}
Interesting. I guess decoding can call this function even if IndexCount
returns 0
? I had assumed nothing would call this function without IndexCount
first.
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.
TFTR!
bors r+
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @cucaroach and @mgartner)
pkg/sql/opt/exec/explain/plan_gist_factory.go
line 496 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
Interesting. I guess decoding can call this function even if
IndexCount
returns0
? I had assumed nothing would call this function withoutIndexCount
first.
Yea, there are some functions in emit.go
that just assume every non-virtual table has a primary index.
Build failed (retrying...): |
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 2 of 2 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mgartner)
Build succeeded: |
Release note (bug fix):
crdb_internal.decode_plan_gist
will no longerproduce an internal error when it is used to decode a plan gist for which
no schema information is available.