-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
sql: cache sequence descriptors #28576
Conversation
4dc38d6
to
9722d1c
Compare
`nextval()` was using uncached sequence descriptors. There is no reason to do so anymore. Release note (performance improvement): SQL sequences receive a slight performance boost in the `nextval()` built-in function.
9722d1c
to
ab37973
Compare
LGTM |
thank you! bors r+ |
28573: roachtest: fix queue failure message r=petermattis a=tschottdorf It was using a global variable instead of the one it wanted. Touches #28372. Release note: None 28576: sql: cache sequence descriptors r=knz a=knz Required for the tests in #28575. `nextval()` was using uncached sequence descriptors. There is no reason to do so anymore. Release note (performance improvement): SQL sequences receive a slight performance boost in the `nextval()` built-in function. Co-authored-by: Tobias Schottdorf <tobias.schottdorf@gmail.com> Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
Build succeeded |
Nice! Out of curiosity, what changed that allowed this? I remember being confused about which function to use to get the table descriptor (cached, not cached, etc) when I first wrote this. |
When you first wrote this we had bugs in the lookup of descriptors that were created in the same txn so a call to nextval would hang. Vivek fixed this over the course of the past few months so now it's safe. |
Excellent; thanks! |
Required for the tests in #28575.
nextval()
was using uncached sequence descriptors.There is no reason to do so anymore.
Release note (performance improvement): SQL sequences receive a slight
performance boost in the
nextval()
built-in function.