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

Merge ALTER TABLE ddl with CREATE TABLE in case of Primary Key, Unique and Check Constraints #2330

Merged
merged 18 commits into from
Feb 21, 2025

Conversation

sanyamsinghal
Copy link
Collaborator

@sanyamsinghal sanyamsinghal commented Feb 17, 2025

Describe the changes in this pull request

TODO: Disable the feature.

  • Added a new transformer struct with method MergeConstraints(). In future we can add more to it.

  • the new table.sql won't have ALTER TABLE ADD CONSTRAINT ddl in case of Primary Key, Unique and Check Constraints
    For example: the following ALTER ddls will be merged with CREATE

  • Performance Improvement - This change will reduce the time to half (2x improvement) for cases where PK is in a separate ALTER TABLE ddl than CREATE TABLE

     CREATE TABLE all_constraints (
         col1 INT,
         col2 TEXT
     );
	-- CHECK Constraint
    ALTER TABLE all_constraints
        ADD CONSTRAINT c_ck CHECK (col1 > 0);

    -- PRIMARY KEY Constraint
    ALTER TABLE all_constraints
        ADD CONSTRAINT c_pk PRIMARY KEY (col1);

    -- UNIQUE Constraint
    ALTER TABLE all_constraints
        ADD CONSTRAINT c_uk UNIQUE (col2);
        
    -- Above 4 DDLs will be merged into 1
     CREATE TABLE all_constraints (
           col1 int, 
           col2 text, 
           CONSTRAINT c_ck CHECK (col1 > 0), 
           CONSTRAINT c_pk PRIMARY KEY (col1), 
           CONSTRAINT c_uk UNIQUE (col2)
     );

Describe if there are any user-facing changes

table.sql file will contain fewer ALTER table ddls.

How was this pull request tested?

Added Unit tests.
Existing end to end tests are enough.
Verified with some end to end tests also.

Does your PR have changes that can cause upgrade issues?

Component Breaking changes?
MetaDB No
Name registry json No
Data File Descriptor Json No
Export Snapshot Status Json No
Import Data State No
Export Status Json No
Data .sql files of tables No
Export and import data queue No
Schema Dump Yes (table.sql will contain lesser DDLs)
AssessmentDB No
Sizing DB No
Migration Assessment Report Json No
Callhome Json No
YugabyteD Tables No
TargetDB Metadata Tables No

@sanyamsinghal sanyamsinghal self-assigned this Feb 17, 2025
@sanyamsinghal
Copy link
Collaborator Author

sanyamsinghal commented Feb 17, 2025

@sanyamsinghal sanyamsinghal marked this pull request as ready for review February 18, 2025 10:20
Copy link
Collaborator

@makalaaneesh makalaaneesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yet to review tests.

@sanyamsinghal
Copy link
Collaborator Author

sanyamsinghal commented Feb 20, 2025

Copy link
Collaborator

@makalaaneesh makalaaneesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a nit.

@sanyamsinghal sanyamsinghal merged commit 0c48c76 into main Feb 21, 2025
66 checks passed
@sanyamsinghal sanyamsinghal deleted the sanyam/merge-alter branch February 21, 2025 05:49
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.

3 participants