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

More ALTER TABLE Support #868

Merged
merged 12 commits into from
Oct 22, 2024
Merged

More ALTER TABLE Support #868

merged 12 commits into from
Oct 22, 2024

Conversation

fulghum
Copy link
Contributor

@fulghum fulghum commented Oct 17, 2024

Adds support for additional ALTER TABLE syntax:

  • Adding a UNIQUE constraint
  • Adding a CHECK constraint
  • Dropping constraints

Adding support for check constraints triggered an issue with string literal value quoting that affects check constraints and column defaults. The fix was to make expression.Literal.String() match the behavior of GMS' expression.Literal.String() method and quote string literals. This required fixing another spot where we had been adding in quotes for string literals, as well as a small change in GMS (dolthub/go-mysql-server#2710).

Fixes:

 
Regresions Report:
The regressions listed below are a little tricky to read, but everything seems to be working correctly as far as I can tell. In the first regression listed (INSERT INTO inhg VALUES ('foo');), this query now fails, because a previous statement now executes correctly to add a check constraint to a table, but our CREATE TABLE LIKE logic incorrectly copies over check constraints.

The rest of the regressions listed seem to actually be working correctly and I'm unable to repro problems with them, and they aren't reporting any errors in the regression report. For example, I've confirmed that the regression reported for ALTER TABLE inhx add constraint foo CHECK (xx = 'text'); actually executes correctly without error now, while on main it returns the error: ALTER TABLE with unsupported constraint definition type *tree.AlterTableAddConstraint.

@dolthub dolthub deleted a comment from github-actions bot Oct 21, 2024
@dolthub dolthub deleted a comment from github-actions bot Oct 21, 2024
Copy link
Contributor

github-actions bot commented Oct 21, 2024

Main PR
Total 42090 42090
Successful 12807 12844
Failures 29283 29246
Partial Successes1 4893 4895
Main PR
Successful 30.4277% 30.5156%
Failures 69.5723% 69.4844%

${\color{lightgreen}Progressions}$

alter_table

QUERY: ALTER TABLE onek ADD CONSTRAINT onek_unique1_constraint UNIQUE (unique1);
QUERY: ALTER TABLE onek ADD CONSTRAINT onek_check_constraint CHECK (unique1 >= 0);
QUERY: ALTER TABLE onek ADD CONSTRAINT onek_unique1_constraint UNIQUE (unique1);
QUERY: alter table atacc1 add constraint atacc_test1 check (test+test2<test3*4);
QUERY: insert into atacc1 (test,test2,test3) values (4,4,2);
QUERY: alter table atacc1 add check (test2>test);
QUERY: insert into atacc1 (test2, test) values (3, 4);
QUERY: alter table atacc2 add constraint foo check (test2>0);
QUERY: insert into atacc2 (test2) values (-3);
QUERY: alter table atacc2 add constraint foo check (test2>0);
QUERY: alter table atacc3 add constraint foo check (test2>0);
QUERY: alter table atacc1 add constraint atacc_test1 unique (test, test2);
QUERY: alter table atacc1 add unique (test2);
QUERY: alter table atacc1 add constraint atacc1_constr_or check(test_a is not null or test_b < 10);
QUERY: alter table atacc1 drop constraint atacc1_constr_or;
QUERY: alter table atacc1 add constraint atacc1_constr_a_valid check(test_a is not null);
QUERY: alter table atacc1 add constraint atacc1_constr_b_valid check(test_b is not null);
QUERY: alter table anothertab
  add unique(f1,f4);
QUERY: ALTER TABLE test_inh_check ADD CONSTRAINT bmerged CHECK (b > 1);
QUERY: ALTER TABLE IF EXISTS tt8 ADD CHECK (f BETWEEN 0 AND 10);
QUERY: ALTER TABLE tt9 ADD CHECK(c > 1);
QUERY: ALTER TABLE tt9 ADD CONSTRAINT foo CHECK(c > 3);
QUERY: ALTER TABLE tt9 ADD UNIQUE(c);
QUERY: ALTER TABLE tt9 ADD UNIQUE(c);
QUERY: ALTER TABLE tt9 ADD CONSTRAINT tt9_c_key2 CHECK(c > 6);
QUERY: ALTER TABLE tt9 ADD UNIQUE(c);
QUERY: alter table defpart_attach_test_d add check (a > 1);

constraints

QUERY: ALTER TABLE unique_tbl ADD CONSTRAINT unique_tbl_i_key
	UNIQUE (i) DEFERRABLE INITIALLY DEFERRED;

create_table

QUERY: alter table defcheck_def add check (b <= 0 and b is not null);

${\color{red}Regressions}$

create_table_like

QUERY:          /* Doesn't copy constraint */
INSERT INTO inhg VALUES ('foo');
RECEIVED ERROR: Check constraint "inhg_chk_702ogqm4" violated (errno 1105) (sqlstate HY000)

create_table_like

QUERY: /* Single entry with value 'text' */
ALTER TABLE inhx add constraint foo CHECK (xx = 'text');

inherit

QUERY: alter table p1 add constraint p2chk check (ff1 > 10);
QUERY: alter table ac add constraint ac_check check (aa is not null);
QUERY: insert into ac (aa) values (NULL);
QUERY: alter table ac drop constraint ac_check;
QUERY: alter table ac add check (aa is not null);
QUERY: insert into ac (aa) values (NULL);
QUERY: alter table ac add constraint ac_check check (aa is not null);
QUERY: alter table ac drop constraint ac_check;
QUERY: alter table p1 add constraint inh_check_constraint1 check (f1 > 0);
QUERY: alter table p1 add constraint inh_check_constraint2 check (f1 < 10);
QUERY: ALTER TABLE errtst_child_fastdef ADD CONSTRAINT errtest_child_fastdef_data_check CHECK (data < 10);

tablespace

QUERY: ALTER TABLE testschema.test_tab ADD CONSTRAINT test_tab_unique UNIQUE (a);

Footnotes

  1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct.

Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

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

LGTM

@fulghum fulghum merged commit 3fcaa50 into main Oct 22, 2024
9 of 13 checks passed
@fulghum fulghum deleted the fulghum/alter-table branch October 22, 2024 01:34
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.

2 participants