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

seqexpr: ON UPDATE sequence expressions contain by-name references #77145

Closed
postamar opened this issue Feb 28, 2022 · 2 comments · Fixed by #81372
Closed

seqexpr: ON UPDATE sequence expressions contain by-name references #77145

postamar opened this issue Feb 28, 2022 · 2 comments · Fixed by #81372
Assignees
Labels
A-schema-changer-impl Related to the implementation of the new schema changer A-schema-descriptors Relating to SQL table/db descriptor handling. 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

@postamar
Copy link
Contributor

postamar commented Feb 28, 2022

I noticed this during recent declarative schema changer work.

CREATE SEQUENCE s;
CREATE TABLE foo (k INT PRIMARY KEY, v INT NOT NULL ON UPDATE (nextval('s')));
SELECT crdb_internal.pb_to_json('cockroach.sql.sqlbase.Descriptor', descriptor)->'table'->'columns' FROM system.descriptor WHERE id = 'foo'::REGCLASS::INT;

yields

  [{"id": 1, "name": "k", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"defaultExpr": "nextval(104:::REGCLASS)", "id": 2, "name": "v", "onUpdateExpr": "nextval(\'s\':::STRING)", "type": {"family": "IntFamily", "oid": 20, "width": 64}, "usesSequenceIds": [104]}]

Instead we expect: "onUpdateExpr": "nextval(104:::REGCLASS)"

Jira issue: CRDB-13432

@postamar postamar added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-schema-descriptors Relating to SQL table/db descriptor handling. labels Feb 28, 2022
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Feb 28, 2022
@postamar
Copy link
Contributor Author

Relates somewhat to #76137

@postamar
Copy link
Contributor Author

This involves also running a descriptor upgrade migration to ensure that all SeqIdentifiers in exprs are exclusively by-ID .

@postamar postamar added the A-schema-changer-impl Related to the implementation of the new schema changer label Mar 8, 2022
@craig craig bot closed this as completed in e6e623f May 23, 2022
@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
A-schema-changer-impl Related to the implementation of the new schema changer A-schema-descriptors Relating to SQL table/db descriptor handling. 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
None yet
Development

Successfully merging a pull request may close this issue.

3 participants