Skip to content

sql: cache sequence descriptors#28576

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
knz:20180814-cache-seqdesc
Aug 14, 2018
Merged

sql: cache sequence descriptors#28576
craig[bot] merged 1 commit intocockroachdb:masterfrom
knz:20180814-cache-seqdesc

Conversation

@knz
Copy link
Copy Markdown
Contributor

@knz knz commented Aug 14, 2018

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.

@knz knz requested review from a team August 14, 2018 13:20
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@knz knz force-pushed the 20180814-cache-seqdesc branch from 4dc38d6 to 9722d1c Compare August 14, 2018 13:21
`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.
@BramGruneir
Copy link
Copy Markdown
Member

LGTM

@knz
Copy link
Copy Markdown
Contributor Author

knz commented Aug 14, 2018

thank you!

bors r+

craig bot pushed a commit that referenced this pull request Aug 14, 2018
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>
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 14, 2018

Build succeeded

@craig craig bot merged commit ab37973 into cockroachdb:master Aug 14, 2018
@knz knz deleted the 20180814-cache-seqdesc branch August 14, 2018 14:06
@vilterp
Copy link
Copy Markdown
Contributor

vilterp commented Aug 15, 2018

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.

@knz
Copy link
Copy Markdown
Contributor Author

knz commented Aug 15, 2018

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.

@vilterp
Copy link
Copy Markdown
Contributor

vilterp commented Aug 16, 2018

Excellent; thanks!

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.

4 participants