From d7b47e515530520da9564b05991bd8a8c6f52b06 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Tue, 24 Jul 2012 15:49:54 -0400 Subject: [PATCH] Change syntax of new CHECK NO INHERIT constraints The initially implemented syntax, "CHECK NO INHERIT (expr)" was not deemed very good, so switch to "CHECK (expr) NO INHERIT" instead. This way it looks similar to SQL-standards compliant constraint attribute. Backport to 9.2 where the new syntax and feature was introduced. Per discussion. --- doc/src/sgml/ref/alter_table.sgml | 4 +- doc/src/sgml/ref/create_table.sgml | 6 +-- doc/src/sgml/release-9.2.sgml | 2 +- src/backend/commands/typecmds.c | 8 ++- src/backend/parser/gram.y | 58 ++++++++++++++-------- src/backend/utils/adt/ruleutils.c | 7 ++- src/test/regress/expected/alter_table.out | 10 ++-- src/test/regress/expected/inherit.out | 4 +- src/test/regress/input/constraints.source | 4 +- src/test/regress/output/constraints.source | 4 +- src/test/regress/sql/alter_table.sql | 6 +-- src/test/regress/sql/inherit.sql | 2 +- 12 files changed, 67 insertions(+), 48 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 3f61d7d45f..b7648142ee 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -495,7 +495,7 @@ ALTER TABLE [ IF EXISTS ] name There must also be matching child-table constraints for all CHECK constraints of the parent, except those - marked non-inheritable (that is, created with ALTER TABLE ONLY) + marked non-inheritable (that is, created with ALTER TABLE ... ADD CONSTRAINT ... NO INHERIT) in the parent, which are ignored; all child-table constraints matched must not be marked non-inheritable. Currently @@ -1013,7 +1013,7 @@ ALTER TABLE distributors ADD CONSTRAINT zipchk CHECK (char_length(zipcode) = 5); To add a check constraint only to a table and not to its children: -ALTER TABLE distributors ADD CONSTRAINT zipchk CHECK NO INHERIT (char_length(zipcode) = 5); +ALTER TABLE distributors ADD CONSTRAINT zipchk CHECK (char_length(zipcode) = 5) NO INHERIT; (The check constraint will not be inherited by future children, either.) diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 08632d3172..19e6f8ed61 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -47,7 +47,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI [ CONSTRAINT constraint_name ] { NOT NULL | NULL | - CHECK [ NO INHERIT ] ( expression ) | + CHECK ( expression ) [ NO INHERIT ] | DEFAULT default_expr | UNIQUE index_parameters | PRIMARY KEY index_parameters | @@ -58,7 +58,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI and table_constraint is: [ CONSTRAINT constraint_name ] -{ CHECK [ NO INHERIT ] ( expression ) | +{ CHECK ( expression ) [ NO INHERIT ] | UNIQUE ( column_name [, ... ] ) index_parameters | PRIMARY KEY ( column_name [, ... ] ) index_parameters | EXCLUDE [ USING index_method ] ( exclude_element WITH operator [, ... ] ) index_parameters [ WHERE ( predicate ) ] | @@ -417,7 +417,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI - CHECK [ NO INHERIT ] ( expression ) + CHECK ( expression ) [ NO INHERIT ] The CHECK clause specifies an expression producing a diff --git a/doc/src/sgml/release-9.2.sgml b/doc/src/sgml/release-9.2.sgml index e79b071500..3ce80c22c2 100644 --- a/doc/src/sgml/release-9.2.sgml +++ b/doc/src/sgml/release-9.2.sgml @@ -1271,7 +1271,7 @@ Allow CHECK constraints to be declared NO - INHERIT (Nikhil Sontakke, Alex Hunsaker) + INHERIT (Nikhil Sontakke, Alex Hunsaker, Álvaro Herrera) diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 30850b2b22..353043d581 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -921,8 +921,14 @@ DefineDomain(CreateDomainStmt *stmt) /* * Check constraints are handled after domain creation, as - * they require the Oid of the domain + * they require the Oid of the domain; at this point we can + * only check that they're not marked NO INHERIT, because + * that would be bogus. */ + if (constr->is_no_inherit) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("CHECK constraints for domains cannot be marked NO INHERIT"))); break; /* diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 777da1139c..6b6901197d 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -101,6 +101,7 @@ typedef struct PrivTarget #define CAS_INITIALLY_IMMEDIATE 0x04 #define CAS_INITIALLY_DEFERRED 0x08 #define CAS_NOT_VALID 0x10 +#define CAS_NO_INHERIT 0x20 #define parser_yyerror(msg) scanner_yyerror(msg, yyscanner) @@ -144,7 +145,7 @@ static void SplitColQualList(List *qualList, core_yyscan_t yyscanner); static void processCASbits(int cas_bits, int location, const char *constrType, bool *deferrable, bool *initdeferred, bool *not_valid, - core_yyscan_t yyscanner); + bool *no_inherit, core_yyscan_t yyscanner); %} @@ -2709,13 +2710,13 @@ ColConstraintElem: n->indexspace = $4; $$ = (Node *)n; } - | CHECK opt_no_inherit '(' a_expr ')' + | CHECK '(' a_expr ')' opt_no_inherit { Constraint *n = makeNode(Constraint); n->contype = CONSTR_CHECK; n->location = @1; - n->is_no_inherit = $2; - n->raw_expr = $4; + n->is_no_inherit = $5; + n->raw_expr = $3; n->cooked_expr = NULL; $$ = (Node *)n; } @@ -2755,10 +2756,10 @@ ColConstraintElem: * combinations. * * See also ConstraintAttributeSpec, which can be used in places where - * there is no parsing conflict. (Note: currently, NOT VALID is an allowed - * clause in ConstraintAttributeSpec, but not here. Someday we might need - * to allow it here too, but for the moment it doesn't seem useful in the - * statements that use ConstraintAttr.) + * there is no parsing conflict. (Note: currently, NOT VALID and NO INHERIT + * are allowed clauses in ConstraintAttributeSpec, but not here. Someday we + * might need to allow them here too, but for the moment it doesn't seem + * useful in the statements that use ConstraintAttr.) */ ConstraintAttr: DEFERRABLE @@ -2835,17 +2836,16 @@ TableConstraint: ; ConstraintElem: - CHECK opt_no_inherit '(' a_expr ')' ConstraintAttributeSpec + CHECK '(' a_expr ')' ConstraintAttributeSpec { Constraint *n = makeNode(Constraint); n->contype = CONSTR_CHECK; n->location = @1; - n->is_no_inherit = $2; - n->raw_expr = $4; + n->raw_expr = $3; n->cooked_expr = NULL; - processCASbits($6, @6, "CHECK", + processCASbits($5, @5, "CHECK", NULL, NULL, &n->skip_validation, - yyscanner); + &n->is_no_inherit, yyscanner); n->initially_valid = !n->skip_validation; $$ = (Node *)n; } @@ -2861,7 +2861,7 @@ ConstraintElem: n->indexspace = $6; processCASbits($7, @7, "UNIQUE", &n->deferrable, &n->initdeferred, NULL, - yyscanner); + NULL, yyscanner); $$ = (Node *)n; } | UNIQUE ExistingIndex ConstraintAttributeSpec @@ -2875,7 +2875,7 @@ ConstraintElem: n->indexspace = NULL; processCASbits($3, @3, "UNIQUE", &n->deferrable, &n->initdeferred, NULL, - yyscanner); + NULL, yyscanner); $$ = (Node *)n; } | PRIMARY KEY '(' columnList ')' opt_definition OptConsTableSpace @@ -2890,7 +2890,7 @@ ConstraintElem: n->indexspace = $7; processCASbits($8, @8, "PRIMARY KEY", &n->deferrable, &n->initdeferred, NULL, - yyscanner); + NULL, yyscanner); $$ = (Node *)n; } | PRIMARY KEY ExistingIndex ConstraintAttributeSpec @@ -2904,7 +2904,7 @@ ConstraintElem: n->indexspace = NULL; processCASbits($4, @4, "PRIMARY KEY", &n->deferrable, &n->initdeferred, NULL, - yyscanner); + NULL, yyscanner); $$ = (Node *)n; } | EXCLUDE access_method_clause '(' ExclusionConstraintList ')' @@ -2922,7 +2922,7 @@ ConstraintElem: n->where_clause = $8; processCASbits($9, @9, "EXCLUDE", &n->deferrable, &n->initdeferred, NULL, - yyscanner); + NULL, yyscanner); $$ = (Node *)n; } | FOREIGN KEY '(' columnList ')' REFERENCES qualified_name @@ -2939,7 +2939,7 @@ ConstraintElem: n->fk_del_action = (char) ($10 & 0xFF); processCASbits($11, @11, "FOREIGN KEY", &n->deferrable, &n->initdeferred, - &n->skip_validation, + &n->skip_validation, NULL, yyscanner); n->initially_valid = !n->skip_validation; $$ = (Node *)n; @@ -4133,7 +4133,7 @@ CreateTrigStmt: n->isconstraint = TRUE; processCASbits($10, @10, "TRIGGER", &n->deferrable, &n->initdeferred, NULL, - yyscanner); + NULL, yyscanner); n->constrrel = $9; $$ = (Node *)n; } @@ -4270,6 +4270,7 @@ ConstraintAttributeElem: | INITIALLY IMMEDIATE { $$ = CAS_INITIALLY_IMMEDIATE; } | INITIALLY DEFERRED { $$ = CAS_INITIALLY_DEFERRED; } | NOT VALID { $$ = CAS_NOT_VALID; } + | NO INHERIT { $$ = CAS_NO_INHERIT; } ; @@ -4386,7 +4387,7 @@ CreateAssertStmt: n->isconstraint = TRUE; processCASbits($8, @8, "ASSERTION", &n->deferrable, &n->initdeferred, NULL, - yyscanner); + NULL, yyscanner); ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -13380,7 +13381,7 @@ SplitColQualList(List *qualList, static void processCASbits(int cas_bits, int location, const char *constrType, bool *deferrable, bool *initdeferred, bool *not_valid, - core_yyscan_t yyscanner) + bool *no_inherit, core_yyscan_t yyscanner) { /* defaults */ if (deferrable) @@ -13428,6 +13429,19 @@ processCASbits(int cas_bits, int location, const char *constrType, constrType), parser_errposition(location))); } + + if (cas_bits & CAS_NO_INHERIT) + { + if (no_inherit) + *no_inherit = true; + else + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + /* translator: %s is CHECK, UNIQUE, or similar */ + errmsg("%s constraints cannot be marked NO INHERIT", + constrType), + parser_errposition(location))); + } } /* parser_init() diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index ec93149f3e..412dfe6f9a 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -1343,10 +1343,9 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand, * Note that simply checking for leading '(' and trailing ')' * would NOT be good enough, consider "(x > 0) AND (y > 0)". */ - appendStringInfo(&buf, "CHECK %s(%s)", - conForm->connoinherit ? "NO INHERIT " : "", - consrc); - + appendStringInfo(&buf, "CHECK (%s)%s", + consrc, + conForm->connoinherit ? " NO INHERIT" : ""); break; } case CONSTRAINT_TRIGGER: diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index c7dce415a6..453a3894b2 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -233,7 +233,7 @@ Check constraints: "con1foo" CHECK (a > 0) Inherits: constraint_rename_test -ALTER TABLE constraint_rename_test ADD CONSTRAINT con2 CHECK NO INHERIT (b > 0); +ALTER TABLE constraint_rename_test ADD CONSTRAINT con2 CHECK (b > 0) NO INHERIT; ALTER TABLE ONLY constraint_rename_test RENAME CONSTRAINT con2 TO con2foo; -- ok ALTER TABLE constraint_rename_test RENAME CONSTRAINT con2foo TO con2bar; -- ok \d constraint_rename_test @@ -245,7 +245,7 @@ Table "public.constraint_rename_test" c | integer | Check constraints: "con1foo" CHECK (a > 0) - "con2bar" CHECK NO INHERIT (b > 0) + "con2bar" CHECK (b > 0) NO INHERIT Number of child tables: 1 (Use \d+ to list them.) \d constraint_rename_test2 @@ -273,7 +273,7 @@ Indexes: "con3foo" PRIMARY KEY, btree (a) Check constraints: "con1foo" CHECK (a > 0) - "con2bar" CHECK NO INHERIT (b > 0) + "con2bar" CHECK (b > 0) NO INHERIT Number of child tables: 1 (Use \d+ to list them.) \d constraint_rename_test2 @@ -635,7 +635,7 @@ drop table atacc1; create table atacc1 (test int); create table atacc2 (test2 int) inherits (atacc1); -- ok: -alter table atacc1 add constraint foo check no inherit (test>0); +alter table atacc1 add constraint foo check (test>0) no inherit; -- check constraint is not there on child insert into atacc2 (test) values (-3); -- check constraint is there on parent @@ -644,7 +644,7 @@ ERROR: new row for relation "atacc1" violates check constraint "foo" DETAIL: Failing row contains (-3). insert into atacc1 (test) values (3); -- fail, violating row: -alter table atacc2 add constraint foo check no inherit (test>0); +alter table atacc2 add constraint foo check (test>0) no inherit; ERROR: check constraint "foo" is violated by some row drop table atacc2; drop table atacc1; diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index c178e25607..25adcd2346 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -597,7 +597,7 @@ select * from d; -- Test non-inheritable parent constraints create table p1(ff1 int); -alter table p1 add constraint p1chk check no inherit (ff1 > 0); +alter table p1 add constraint p1chk check (ff1 > 0) no inherit; alter table p1 add constraint p2chk check (ff1 > 10); -- connoinherit should be true for NO INHERIT constraint select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pgc.connoinherit from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname = 'p1' order by 1,2; @@ -615,7 +615,7 @@ create table c1 () inherits (p1); --------+---------+----------- ff1 | integer | Check constraints: - "p1chk" CHECK NO INHERIT (ff1 > 0) + "p1chk" CHECK (ff1 > 0) NO INHERIT "p2chk" CHECK (ff1 > 10) Number of child tables: 1 (Use \d+ to list them.) diff --git a/src/test/regress/input/constraints.source b/src/test/regress/input/constraints.source index 37d06b0127..16468b20af 100644 --- a/src/test/regress/input/constraints.source +++ b/src/test/regress/input/constraints.source @@ -148,7 +148,7 @@ DROP TABLE INSERT_CHILD; -- CREATE TABLE ATACC1 (TEST INT - CHECK NO INHERIT (TEST > 0)); + CHECK (TEST > 0) NO INHERIT); CREATE TABLE ATACC2 (TEST2 INT) INHERITS (ATACC1); -- check constraint is not there on child @@ -158,7 +158,7 @@ INSERT INTO ATACC1 (TEST) VALUES (-3); DROP TABLE ATACC1 CASCADE; CREATE TABLE ATACC1 (TEST INT, TEST2 INT - CHECK (TEST > 0), CHECK NO INHERIT (TEST2 > 10)); + CHECK (TEST > 0), CHECK (TEST2 > 10) NO INHERIT); CREATE TABLE ATACC2 () INHERITS (ATACC1); -- check constraint is there on child diff --git a/src/test/regress/output/constraints.source b/src/test/regress/output/constraints.source index e8237f4a69..ff6c1dc450 100644 --- a/src/test/regress/output/constraints.source +++ b/src/test/regress/output/constraints.source @@ -231,7 +231,7 @@ DROP TABLE INSERT_CHILD; -- Check NO INHERIT type of constraints and inheritance -- CREATE TABLE ATACC1 (TEST INT - CHECK NO INHERIT (TEST > 0)); + CHECK (TEST > 0) NO INHERIT); CREATE TABLE ATACC2 (TEST2 INT) INHERITS (ATACC1); -- check constraint is not there on child INSERT INTO ATACC2 (TEST) VALUES (-3); @@ -242,7 +242,7 @@ DETAIL: Failing row contains (-3). DROP TABLE ATACC1 CASCADE; NOTICE: drop cascades to table atacc2 CREATE TABLE ATACC1 (TEST INT, TEST2 INT - CHECK (TEST > 0), CHECK NO INHERIT (TEST2 > 10)); + CHECK (TEST > 0), CHECK (TEST2 > 10) NO INHERIT); CREATE TABLE ATACC2 () INHERITS (ATACC1); -- check constraint is there on child INSERT INTO ATACC2 (TEST) VALUES (-3); diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 5ca537e91b..0f9cb380e1 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -218,7 +218,7 @@ ALTER TABLE ONLY constraint_rename_test RENAME CONSTRAINT con1 TO con1foo; -- fa ALTER TABLE constraint_rename_test RENAME CONSTRAINT con1 TO con1foo; -- ok \d constraint_rename_test \d constraint_rename_test2 -ALTER TABLE constraint_rename_test ADD CONSTRAINT con2 CHECK NO INHERIT (b > 0); +ALTER TABLE constraint_rename_test ADD CONSTRAINT con2 CHECK (b > 0) NO INHERIT; ALTER TABLE ONLY constraint_rename_test RENAME CONSTRAINT con2 TO con2foo; -- ok ALTER TABLE constraint_rename_test RENAME CONSTRAINT con2foo TO con2bar; -- ok \d constraint_rename_test @@ -500,14 +500,14 @@ drop table atacc1; create table atacc1 (test int); create table atacc2 (test2 int) inherits (atacc1); -- ok: -alter table atacc1 add constraint foo check no inherit (test>0); +alter table atacc1 add constraint foo check (test>0) no inherit; -- check constraint is not there on child insert into atacc2 (test) values (-3); -- check constraint is there on parent insert into atacc1 (test) values (-3); insert into atacc1 (test) values (3); -- fail, violating row: -alter table atacc2 add constraint foo check no inherit (test>0); +alter table atacc2 add constraint foo check (test>0) no inherit; drop table atacc2; drop table atacc1; diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 74cb3f09e8..29c1e59fd0 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -140,7 +140,7 @@ select * from d; -- Test non-inheritable parent constraints create table p1(ff1 int); -alter table p1 add constraint p1chk check no inherit (ff1 > 0); +alter table p1 add constraint p1chk check (ff1 > 0) no inherit; alter table p1 add constraint p2chk check (ff1 > 10); -- connoinherit should be true for NO INHERIT constraint select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pgc.connoinherit from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname = 'p1' order by 1,2; -- 2.40.0