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.

Sign up for GitHub

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

Jump to bottom

sql/schemachanger: remove remaining add column limitations for declarative schema changer #80545

Open
3 of 7 tasks
fqazi opened this issue Apr 26, 2022 · 3 comments
Open
3 of 7 tasks

sql/schemachanger: remove remaining add column limitations for declarative schema changer #80545

fqazi opened this issue Apr 26, 2022 · 3 comments
Labels
A-schema-changer-impl Related to the implementation of the new schema changer C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
SQL Foundations

Comments

@fqazi
Copy link
Collaborator

fqazi commented Apr 26, 2022 •
edited by ajwerner

The declarative schema changer is currently disabled when adding columns in the following scenarios:

  • Legacy schema changer will be used whenever serial and generated columns are observed.
  • Regional by row tables will default to the legacy schema changer since we are missing proper support for updating zone configs
  • Columns with the unique constraint will default to the legacy schema changer
  • Expressions using sequences will default to the legacy schema changer (DEFAULT, ON UPDATE, COMPUTED) (see sql: use declarative schemachange for add column sequence exprs #81782)
  • Better errors can be surfaced on backfill errors for the declarative schema changer. At least one case falls to the legacy one today for this purpose (already fixed in the original PR)
  • Tests which need the MVCC backfiller are currently using the legacy schema changer
  • REFERENCES foreign key constraints

Jira issue: CRDB-15351
@fqazi fqazi added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-schema-deprecated Use T-sql-foundations instead labels Apr 26, 2022
@blathers-crl blathers-crl bot added this to Triage in SQL Foundations Apr 26, 2022
@fqazi fqazi changed the title sql/schemachanger: remove remaining limitations for declarative schema changer sql/schemachanger: remove remaining add column limitations for declarative schema changer Apr 26, 2022
@fqazi fqazi mentioned this issue Apr 26, 2022
Merged
@ajwerner ajwerner moved this from Triage to 22.2 General in SQL Foundations Apr 26, 2022
@postamar
Copy link
Contributor

Thanks for filing this and collecting all the known odds and ends. Do we also need to fall back to the legacy schema changer for ON UPDATE in addition to DEFAULT?

@fqazi
Copy link
Collaborator Author

fqazi commented May 17, 2022

Yes that's true for all expression types let me fix the TODO @postamar

craig bot pushed a commit that referenced this issue May 17, 2022
@fqazi @maryliag
@otan @rickystewart
Merge #76983 #81077 #81371 #81381
9e64c79 
76983: sql/schemachanger: enable add column by default in declarative schema changer r=fqazi a=fqazi

These changes will enable add column by default in the declarative schema changer and the following fixes:

1. Add support for retrying schema changes when retriable errors are encountered
2. Add support for refreshing stats during declarative schema changes
3. Block add column when cross-database references are observed in the declarative schema changer
4. Block implicit record types inside expressions for the declarative schema changer
5. Add schema change-related event log entries
6. Properly combine multiple declarative schema change operations inside CTEs, which is required for support geometry related builtins that add columns
7. Enforce schema usage related privileges in the declarative schema changer
8. Add add column related telemetry
9. Generate appropriate errors when a newly added column conflicts with a system column
10. Support empty column family names
11. Add columns in the user-specified order inside the declarative schema changer
12. Enforce constraints when adding new columns, specifically detecting if default values will violate them

Along with the following known issues and limitations (tracked by issue: #80545)
1. Legacy schema changer will be used whenever serial and generated columns are observed.
2. Regional by row tables will default to the legacy schema changer since we are missing proper support for updating zone configs
3. Columns with the unique constraint will default to the legacy schema changer
4. Default expressions using sequences will default to the legacy schema changer
5. A number of tests cases depending on the MVCC backfiller are using the legacy schema changer for now

81077: ui: update dates to 24h UTC r=maryliag a=maryliag

This commit updates all dates to use a 24h format in UTC

Fixes #78442

Release note (ui change): Update all dates to use 24h format in UTC

81371: tree: extend disallow_imports_test r=ajwerner a=otan

Now blocking roachpb and util/log.

Release note: None

81381: bazel: augment `disallowed_imports_test` to allow disallowing `c-deps` r=ajwerner a=rickystewart

Due to bitrot some of these assertions are already failing. I've filed
the follow-up bugs #81375, #81378, and #81380 for these cases.

I also replaced the `VerifyNoImports` test used by a few packages with
`disallowed_imports_test`.

Release note: None

Co-authored-by: Faizan Qazi <faizan@cockroachlabs.com>
Co-authored-by: Marylia Gutierrez <marylia@cockroachlabs.com>
Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>
@jlinder jlinder added sync-me and removed sync-me labels May 20, 2022
@postamar postamar moved this from General to Backlog in SQL Foundations Sep 19, 2022
@postamar postamar added the A-schema-changer-impl Related to the implementation of the new schema changer label Sep 19, 2022
@postamar postamar self-assigned this Nov 10, 2022
@postamar postamar moved this from Backlog to In Flight in SQL Foundations Nov 10, 2022
@postamar
Copy link
Contributor

I've unassigned myself considering that I'm very unlikely to close this before going on leave. I think I assigned myself this one in the first place because I was interested in owning the overall story. Perhaps the list in the issue description needs some grooming, I haven't checked.

@postamar postamar removed their assignment Jan 30, 2023
@postamar postamar moved this from In Flight to Triage in SQL Foundations Jan 30, 2023
@ajwerner ajwerner moved this from Triage to Backlog in SQL Foundations Jan 31, 2023
@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
Assignees
No one assigned
Labels
A-schema-changer-impl Related to the implementation of the new schema changer C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
SQL Foundations
  
Backlog
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jlinder @fqazi @postamar