builtins: fix potential panic in crdb_internal.encode_key #42456
Conversation
rohany
added this to Open PRs
in SQL Features (Deprecated - use SQL Experience board)
via automation
|
This change is |
rohany
force-pushed
the
encode_key_panic
branch
from
a2c4624
to
fb0c090
Compare
There was a problem hiding this comment.
I'm still going to pick on the release note. For one thing we should call it out as a "bug fix" rather than a "sql change". For another, we should make it clear that the SHOW RANGE FROM INDEX command is affected since that is the most likely way a user would hit this. (They're unlikely to be calling encode_key directly, right?) And finally I would avoid implementation-level language like "table descriptor" if possible since it's not meaningful to users.
How about something like this?
Release note (bug fix): For tables with dropped indexes, the SHOW RANGE FROM INDEX command sometimes returned incorrect results or an error. Fixed the underlying issue in the encode_key built-in.
Also I would say it's not necessary to say "This should be in 19.2.1" in the commit message. You can just backport it now.
|
Sorry about the back and forth -- what you're saying makes sense. I'll use the updated release note.
Oh, I didn't know that the branch was open for 19.2.1, i'll open up a backport after this. |
crdb_internal.encode_key was indexing into a table descriptor's indexes slice using index ID's, when it should have been using `FindIndexByID`. Release note (bug fix): For tables with dropped indexes, the SHOW RANGE FOR ROW command sometimes returned incorrect results or an error. Fixed the underlying issue in the crdb_internal.encode_key built-in.
rohany
force-pushed
the
encode_key_panic
branch
from
fb0c090
to
1539f95
Compare
|
bors r+ |
42448: sql/mutations: add stats injector + refactor r=mjibson a=mjibson 42456: builtins: fix potential panic in crdb_internal.encode_key r=rohany a=rohany crdb_internal.encode_key was indexing into a table descriptor's indexes slice using index ID's, when it should have been using `FindIndexByID`. Release note (bug fix): For tables with dropped indexes, the SHOW RANGE FOR ROW command sometimes returned incorrect results or an error. Fixed the underlying issue in the crdb_internal.encode_key built-in. Co-authored-by: Matt Jibson <matt.jibson@gmail.com> Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
Build succeeded |
crdb_internal.encode_key was indexing into a table descriptor's
indexes slice using index ID's, when it should have been using
FindIndexByID.Release note (bug fix): For tables with dropped indexes, the SHOW RANGE FOR ROW command sometimes returned incorrect
results or an error. Fixed the underlying issue in the crdb_internal.encode_key built-in.