Skip to content

MDEV-24602: Cannot specify a name for a column check constraint - #2773

Open
an3l wants to merge 5 commits into
MariaDB:10.4from
an3l:bb-10.4-anel-MDEV-24602-field-constraint
Open

MDEV-24602: Cannot specify a name for a column check constraint#2773
an3l wants to merge 5 commits into
MariaDB:10.4from
an3l:bb-10.4-anel-MDEV-24602-field-constraint

Conversation

@an3l

@an3l an3l commented Sep 28, 2023

Copy link
Copy Markdown
Contributor
  • Adding parser rule for named constraint
  • Add checks of names for field check constraints and between field and
    table check constraints (table check constraints name checks are done in
    mysql_prepare_create)
  • When altering/modifying table with field/table constraints check field/table
    constraints for adding constraint make it possible by changes done in
    parser, what is inline with documentation
alter_specification:
MODIFY [COLUMN] [IF EXISTS] col_name column_definition
[FIRST | AFTER col_name]
  • Fixes: MDEV-24598: duplicate CHECK constraint name
  • Includes reverting MDEV-16630
  • Reviewer: <>

@an3l an3l added the MariaDB Foundation Pull requests created by MariaDB Foundation label Sep 28, 2023
@an3l an3l added this to the 10.4 milestone Sep 28, 2023
@an3l
an3l requested a review from vuvova September 28, 2023 14:38
@an3l an3l changed the title MDEV-24602: Cannot specify a name for a column check constraint WIP: MDEV-24602: Cannot specify a name for a column check constraint Sep 28, 2023
@an3l
an3l force-pushed the bb-10.4-anel-MDEV-24602-field-constraint branch from d7c8188 to 6ed55d0 Compare September 29, 2023 09:33
Comment thread mysql-test/main/alter_table.result Outdated
`b` char(1) DEFAULT NULL,
PRIMARY KEY (`a`),
KEY `b` (`new_b`),
CONSTRAINT `CONSTRAINT_1` CHECK (`a` + `new_b` > 0),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand. Why did you change how constraints are printed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because of mysqldump that uses show create table and added constraint for json_valid that for the test mysqldump.test returned duplicate error constraint (duplicating when dumping mysql.global_priv table).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

for what table/test there was a duplicate error constraint? why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Have updated with the new commit as well PR description with questions and problems for the patch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The patch should be functional - no problems for the patch :).

@an3l
an3l force-pushed the bb-10.4-anel-MDEV-24602-field-constraint branch 2 times, most recently from 1fb93f6 to efeb4b6 Compare October 25, 2023 15:01
@an3l an3l changed the title WIP: MDEV-24602: Cannot specify a name for a column check constraint MDEV-24602: Cannot specify a name for a column check constraint Oct 25, 2023
@an3l
an3l force-pushed the bb-10.4-anel-MDEV-24602-field-constraint branch 2 times, most recently from ffb2f87 to 73b7130 Compare October 26, 2023 17:53

@vuvova vuvova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See inline comments. Global comments:

  • please put MDEV-24602 and MDEV-24598 in two different commits. They could still be in the same PR, just different commits.
  • in MDEV-24598 you need to handle the case when an existing table (created before your fix) has duplicate constraint names. What will happen in this case? Will ALTER TABLE work?

Comment thread sql/sql_show.cc Outdated
packet->append(STRING_WITH_LEN(" CONSTRAINT "));
packet->append(field->check_constraint->name.str);
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is that?

@an3l an3l Jan 29, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In case of create or alter table with new constraint name, there is no visible constraint name in show create.
Like:

   `t0` int(11) DEFAULT NULL,
   `t1` int(11) DEFAULT NULL CHECK (`t1` > 0),
   `t2` int(11) DEFAULT NULL CHECK (`t2` > 0),
-  `t3` int(11) DEFAULT NULL CHECK (`t3` > 0)
+  `t3` int(11) DEFAULT NULL CONSTRAINT t_field CHECK (`t3` > 0)
 ) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci
 SELECT CONSTRAINT_NAME, TABLE_NAME, CONSTRAINT_TYPE
 FROM information_schema.TABLE_CONSTRAINTS WHERE
@@ -408,7 +408,7 @@
 show create table t;
 Table	Create Table
 t	CREATE TABLE `t` (
-  `t1` int(11) DEFAULT NULL CHECK (`t1` > 3.14)
+  `t1` int(11) DEFAULT NULL CONSTRAINT t1_greater_pi_check CHECK (`t1` > 3.14)
 ) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci
 SELECT * from INFORMATION_SCHEMA.CHECK_CONSTRAINTS WHERE
 CONSTRAINT_SCHEMA = 'test';

Comment thread sql/sql_yacc.yy
$$= $1;
$$->check_constraint= $3;
$$->check_constraint->name= $2;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please, rebase your PR to 10.5. I suspect this hunk might be a problem there.

@an3l an3l Jan 29, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, seems there are some problems with 10.5. ⚒️
Update: Here is the rebased version of this PR for 10.5 https://github.com/an3l/server/commits/bb-10.5-anel-MDEV-24602-field-constraint-v5/

Comment thread sql/sql_yacc.yy Outdated
Comment thread sql/unireg.cc Outdated
an3l added 5 commits January 29, 2024 09:04
- Name of the field check constraint is the same as the name of the
field.
- Add check of field name and table constraint names,
  that is done during `mysql_prepare_create_table()`
- Closes PR MariaDB#2773

- Reviewer: <>
- `ALTER TABLE MODIFY_SYM` should use `column_def` instead of
`field_spec` what is inline with documentation
```
alter_specification:
MODIFY [COLUMN] [IF EXISTS] col_name column_definition
[FIRST | AFTER col_name]
```
- Closes PR MariaDB#2773
-Reviewer: <>
- Adding parser rule for named constraint
- Closes PR MariaDB#2773
- Reviewer: <>
@an3l
an3l force-pushed the bb-10.4-anel-MDEV-24602-field-constraint branch from 73b7130 to 6bfbba8 Compare January 29, 2024 12:41
an3l added a commit to an3l/server that referenced this pull request Jan 29, 2024
- Name of the field check constraint is the same as the name of the
field.
- Add check of field name and table constraint names,
  that is done during `mysql_prepare_create_table()`
- Closes PR MariaDB#2773

- Reviewer: <>
an3l added a commit to an3l/server that referenced this pull request Jan 29, 2024
- `ALTER TABLE MODIFY_SYM` should use `column_def` instead of
`field_spec` what is inline with documentation
```
alter_specification:
MODIFY [COLUMN] [IF EXISTS] col_name column_definition
[FIRST | AFTER col_name]
```
- Closes PR MariaDB#2773
-Reviewer: <>
an3l added a commit to an3l/server that referenced this pull request Jan 29, 2024
- Adding parser rule for named constraint
- Closes PR MariaDB#2773
- Reviewer: <>
an3l added a commit to an3l/server that referenced this pull request Jan 29, 2024
- Name of the field check constraint is the same as the name of the
field.
- Add check of field name and table constraint names,
  that is done during `mysql_prepare_create_table()`
- Closes PR MariaDB#2773

- Reviewer: <>
an3l added a commit to an3l/server that referenced this pull request Jan 29, 2024
- `ALTER TABLE MODIFY_SYM` should use `column_def` instead of
  `field_spec` what is inline with documentation
  ```
    alter_specification:
    MODIFY [COLUMN] [IF EXISTS] col_name column_definition
    [FIRST | AFTER col_name]
  ```
- Closes PR MariaDB#2773
-Reviewer: <>
an3l added a commit to an3l/server that referenced this pull request Jan 29, 2024
- Adding parser rule for named constraint
- Closes PR MariaDB#2773
- Reviewer: <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

MariaDB Foundation Pull requests created by MariaDB Foundation

Development

Successfully merging this pull request may close these issues.

2 participants