Skip to content
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

Update sequences to be stored by ID when doing backup/restore #60942

Closed
the-ericwang35 opened this issue Feb 22, 2021 · 2 comments · Fixed by #82172
Closed

Update sequences to be stored by ID when doing backup/restore #60942

the-ericwang35 opened this issue Feb 22, 2021 · 2 comments · Fixed by #82172
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@the-ericwang35
Copy link
Contributor

the-ericwang35 commented Feb 22, 2021

Describe the problem

There now exists functionality for newly created sequences to be renamed, since we are now storing references to sequences via their IDs. Currently, if we backup sequences in earlier versions (before this renaming functionality) and then restore them in newer versions (that do support sequence renaming), these restored sequences still behave like old sequence. This means that they are still stored via their names, and they cannot be renamed.

Expected behavior
We want restored sequences to behave like sequences created in newer versions. That is, restored sequences should be stored by ID and have the ability to be renamed. This means we'll have to update the existing tests as well (see #60957)

Jira issue: CRDB-3097

@the-ericwang35 the-ericwang35 added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Feb 22, 2021
craig bot pushed a commit that referenced this issue Feb 23, 2021
60957: sql: add backup/restore tests for sequences r=the-ericwang35 a=the-ericwang35

This patch just adds tests for backup/restore with respect to sequences.
Now that all new sequences are referenced by ID and can be renamed,
we want to test that previously created sequences are not affected,
and still follow the old rules (i.e. cannot be renamed and
and are referenced by their names.
Part of #51090

In the future, we'll want to convert these old sequences to
new sequences (i.e. can be renamed and are referenced by ID, 
see #60942) which means these tests will need to change, but
for now, we'll keep the old sequences as is.


Release note (None): add backup/restore tests for sequences

60972: sql: error if SURVIVE is set with no regions r=ajstorm a=otan

Release justification: bug fix to new functionality

Release note: None

60993: colexec: remove duplication around bazel utility functions r=yuzefovich a=yuzefovich

Previously, we had essentially duplicated `.bzl` files in two packages
that need to generate code using `execgen`, but this can be avoided by
defining a separate `.bzl` file as an extension used in both places.

Release note: None

61008: TeamCity: update build agents to use the latest sudo r=rail a=rail

Release note: None

61024: authors: add erikgrinaker r=erikgrinaker a=erikgrinaker

Release note: none

61025: authors: add fqazi to authors r=fqazi a=fqazi

authors: add fqazi to authors

Release note: None

61026: authors: add abarganier to authors r=abarganier a=abarganier

Release note: None

Co-authored-by: Eric Wang <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Alex Barganier <[email protected]>
@adityamaru
Copy link
Contributor

This will be addressed in 21.2 once we get the green light from sql-schema and potentially after we address the bigger issue of restore and migrations as detailed in - #58611.

craig bot pushed a commit that referenced this issue Mar 19, 2021
61126: sql: add version upgrade test for sequences r=ajwerner a=the-ericwang35

This patch just adds some version upgrade tests
for sequences. Now that all new sequences are
referenced by ID and can be renamed, we want to test
that previously created sequences are not affected,
and still follow the old rules (i.e. cannot be renamed and
and are referenced by their names.
This test creates some sequences in 20.2.5, upgrades the
cluster, and verifies this behaviour.
Part of #51090.

In the future, we'll want to convert these old sequences to
new sequences (i.e. can be renamed and are referenced by ID, 
see #60942) which means these tests will need to change, but
for now, we'll keep the old sequences as is.

Release note: None

Release justification: only adds tests for existing functionality

62203: sql: capture sequences used in cast exprs r=ajwerner a=the-ericwang35

Previously, `GetSequenceFromFunc` only looked
for `*tree.DString` and `*tree.DOid`. However,
it's also possible for the sequence to be given via
a `*tree.CastExpr` (for example, `nextval('s'::regclass)`).
In this case, the sequence was not being captured,
which meant no back references were created and
no sequence encoding was done.
This PR addresses this by adding an additional
case to capture these sequences.

This will probably need to be backported.

Release note: None

Co-authored-by: Eric Wang <[email protected]>
@adityamaru adityamaru removed their assignment May 26, 2021
@jlinder jlinder added the T-sql-schema-deprecated Use T-sql-foundations instead label Jun 16, 2021
@ajwerner
Copy link
Contributor

This seems important and like a rather bad bug involving ON UPDATE. I'd even consider backporting this.

@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants