From b635ac7f4703599b50aa3a4c527356388c19828e Mon Sep 17 00:00:00 2001 From: jian he Date: Wed, 26 Nov 2025 11:24:18 +0800 Subject: [PATCH v5 1/2] Improve processCASbits API with a 'seen' struct This allows ALTER TABLE .. ALTER CONSTRAINT to be more precise about operations that are supported or not, as well as the reports from CREATE CONSTRAINT TRIGGER error messages making more sense. --- src/backend/parser/gram.y | 102 ++++++++++++++++++++++++++++---------- 1 file changed, 77 insertions(+), 25 deletions(-) diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index c3a0a354a9c..4e0aa41e2b6 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -147,6 +147,17 @@ typedef struct KeyActions #define CAS_NOT_ENFORCED 0x40 #define CAS_ENFORCED 0x80 +/* + * We represent whether each set of flags is seen on a command with CAS_flags. + */ +typedef struct CAS_flags +{ + bool seen_deferrability; + bool seen_enforced; + bool seen_valid; + bool seen_inherit; +} CAS_flags; + #define parser_yyerror(msg) scanner_yyerror(msg, yyscanner) #define parser_errposition(pos) scanner_errposition(pos, yyscanner) @@ -198,8 +209,9 @@ static void SplitColQualList(List *qualList, List **constraintList, CollateClause **collClause, core_yyscan_t yyscanner); static void processCASbits(int cas_bits, int location, const char *constrType, - bool *deferrable, bool *initdeferred, bool *is_enforced, - bool *not_valid, bool *no_inherit, core_yyscan_t yyscanner); + bool *deferrable, bool *initdeferred, bool *is_enforced, + bool *not_valid, bool *no_inherit, CAS_flags *seen, + core_yyscan_t yyscanner); static PartitionStrategy parsePartitionStrategy(char *strategy, int location, core_yyscan_t yyscanner); static void preprocess_pub_all_objtype_list(List *all_objects_list, @@ -2744,6 +2756,7 @@ alter_table_cmd: &c->is_enforced, NULL, &c->noinherit, + NULL, yyscanner); $$ = (Node *) n; } @@ -4275,7 +4288,7 @@ ConstraintElem: n->cooked_expr = NULL; processCASbits($5, @5, "CHECK", NULL, NULL, &n->is_enforced, &n->skip_validation, - &n->is_no_inherit, yyscanner); + &n->is_no_inherit, NULL, yyscanner); n->initially_valid = !n->skip_validation; $$ = (Node *) n; } @@ -4288,7 +4301,7 @@ ConstraintElem: n->keys = list_make1(makeString($3)); processCASbits($4, @4, "NOT NULL", NULL, NULL, NULL, &n->skip_validation, - &n->is_no_inherit, yyscanner); + &n->is_no_inherit, NULL, yyscanner); n->initially_valid = !n->skip_validation; $$ = (Node *) n; } @@ -4308,7 +4321,7 @@ ConstraintElem: n->indexspace = $9; processCASbits($10, @10, "UNIQUE", &n->deferrable, &n->initdeferred, NULL, - NULL, NULL, yyscanner); + NULL, NULL, NULL, yyscanner); $$ = (Node *) n; } | UNIQUE ExistingIndex ConstraintAttributeSpec @@ -4324,7 +4337,7 @@ ConstraintElem: n->indexspace = NULL; processCASbits($3, @3, "UNIQUE", &n->deferrable, &n->initdeferred, NULL, - NULL, NULL, yyscanner); + NULL, NULL, NULL, yyscanner); $$ = (Node *) n; } | PRIMARY KEY '(' columnList opt_without_overlaps ')' opt_c_include opt_definition OptConsTableSpace @@ -4342,7 +4355,7 @@ ConstraintElem: n->indexspace = $9; processCASbits($10, @10, "PRIMARY KEY", &n->deferrable, &n->initdeferred, NULL, - NULL, NULL, yyscanner); + NULL, NULL, NULL, yyscanner); $$ = (Node *) n; } | PRIMARY KEY ExistingIndex ConstraintAttributeSpec @@ -4358,7 +4371,7 @@ ConstraintElem: n->indexspace = NULL; processCASbits($4, @4, "PRIMARY KEY", &n->deferrable, &n->initdeferred, NULL, - NULL, NULL, yyscanner); + NULL, NULL, NULL, yyscanner); $$ = (Node *) n; } | EXCLUDE access_method_clause '(' ExclusionConstraintList ')' @@ -4378,7 +4391,7 @@ ConstraintElem: n->where_clause = $9; processCASbits($10, @10, "EXCLUDE", &n->deferrable, &n->initdeferred, NULL, - NULL, NULL, yyscanner); + NULL, NULL, NULL, yyscanner); $$ = (Node *) n; } | FOREIGN KEY '(' columnList optionalPeriodName ')' REFERENCES qualified_name @@ -4408,7 +4421,7 @@ ConstraintElem: processCASbits($12, @12, "FOREIGN KEY", &n->deferrable, &n->initdeferred, &n->is_enforced, &n->skip_validation, NULL, - yyscanner); + NULL, yyscanner); n->initially_valid = !n->skip_validation; $$ = (Node *) n; } @@ -4446,9 +4459,9 @@ DomainConstraintElem: n->location = @1; n->raw_expr = $3; n->cooked_expr = NULL; - processCASbits($5, @5, "CHECK", + processCASbits($5, @5, "CHECK", /* FIXME */ NULL, NULL, NULL, &n->skip_validation, - &n->is_no_inherit, yyscanner); + &n->is_no_inherit, NULL, yyscanner); n->is_enforced = true; n->initially_valid = !n->skip_validation; $$ = (Node *) n; @@ -4461,9 +4474,9 @@ DomainConstraintElem: n->location = @1; n->keys = list_make1(makeString("value")); /* no NOT VALID, NO INHERIT support */ - processCASbits($3, @3, "NOT NULL", + processCASbits($3, @3, "NOT NULL", /* FIXME */ NULL, NULL, NULL, - NULL, NULL, yyscanner); + NULL, NULL, NULL, yyscanner); n->initially_valid = true; $$ = (Node *) n; } @@ -6146,7 +6159,7 @@ CreateTrigStmt: n->transitionRels = NIL; processCASbits($11, @11, "TRIGGER", &n->deferrable, &n->initdeferred, &dummy, - NULL, NULL, yyscanner); + NULL, NULL, NULL, yyscanner); n->constrrel = $10; $$ = (Node *) n; } @@ -19599,14 +19612,31 @@ SplitColQualList(List *qualList, } /* - * Process result of ConstraintAttributeSpec, and set appropriate bool flags - * in the output command node. Pass NULL for any flags the particular - * command doesn't support. + * Process result of ConstraintAttributeSpec, and set appropriate bool flags. + * Any of those flags can be given as NULL pointers, for options that are + * unsupported by the particular production being parsed. If 'seen' is given + * as a non NULL pointer, the corresponding boolean there is set for every + * option in the command being parsed. + * + * Unsupported flags for a particular command can be handled in one of two + * ways. Productions that require ad-hoc error reporting (those that don't + * know which type of constraint is being parsed or simply require a + * different phrasing than what this routine provides) can pass a valid + * 'seen' pointer; when that is given, each flag in that struct is set when + * a particular type of option appears in the command. The production- + * specific case can inspect the 'seen' flags and complain appropriately if + * one option was seen that the command doesn't support. In this case, + * 'constrType' must be given as NULL. + * + * The other option is to give a NULL 'seen' pointer. In this case, an + * unsuported flag will give rise to an error report using the 'constrType', + * which must be given as not NULL. */ static void processCASbits(int cas_bits, int location, const char *constrType, bool *deferrable, bool *initdeferred, bool *is_enforced, - bool *not_valid, bool *no_inherit, core_yyscan_t yyscanner) + bool *not_valid, bool *no_inherit, + CAS_flags *seen, core_yyscan_t yyscanner) { /* defaults */ if (deferrable) @@ -19617,70 +19647,90 @@ processCASbits(int cas_bits, int location, const char *constrType, *not_valid = false; if (is_enforced) *is_enforced = true; + if (no_inherit) + *no_inherit = false; + + /* Assert((constrType == NULL) ^ (seen == NULL)); */ + if (seen) + memset(seen, 0, sizeof(CAS_flags)); if (cas_bits & (CAS_DEFERRABLE | CAS_INITIALLY_DEFERRED)) { if (deferrable) *deferrable = true; - else + else if (!seen) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), /* translator: %s is CHECK, UNIQUE, or similar */ errmsg("%s constraints cannot be marked DEFERRABLE", constrType), parser_errposition(location))); + if (seen) + seen->seen_deferrability = true; } if (cas_bits & CAS_INITIALLY_DEFERRED) { if (initdeferred) *initdeferred = true; - else + else if (!seen) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), /* translator: %s is CHECK, UNIQUE, or similar */ errmsg("%s constraints cannot be marked DEFERRABLE", constrType), parser_errposition(location))); + if (seen) + seen->seen_deferrability = true; } + /* not deferrable is the default; just report that we saw it */ + if (cas_bits & (CAS_NOT_DEFERRABLE) && seen) + seen->seen_deferrability = true; + if (cas_bits & CAS_NOT_VALID) { if (not_valid) *not_valid = true; - else + else if (!seen) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), /* translator: %s is CHECK, UNIQUE, or similar */ errmsg("%s constraints cannot be marked NOT VALID", constrType), parser_errposition(location))); + if (seen) + seen->seen_valid = true; } if (cas_bits & CAS_NO_INHERIT) { if (no_inherit) *no_inherit = true; - else + else if (!seen) 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))); + if (seen) + seen->seen_inherit = true; } if (cas_bits & CAS_NOT_ENFORCED) { if (is_enforced) *is_enforced = false; - else + else if (!seen) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), /* translator: %s is CHECK, UNIQUE, or similar */ errmsg("%s constraints cannot be marked NOT ENFORCED", constrType), parser_errposition(location))); + if (seen) + seen->seen_enforced = true; /* * NB: The validated status is irrelevant when the constraint is set to @@ -19696,13 +19746,15 @@ processCASbits(int cas_bits, int location, const char *constrType, { if (is_enforced) *is_enforced = true; - else + else if (!seen) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), /* translator: %s is CHECK, UNIQUE, or similar */ errmsg("%s constraints cannot be marked ENFORCED", constrType), parser_errposition(location))); + if (seen) + seen->seen_enforced = true; } } -- 2.34.1