diff --git a/src/backend/distributed/relay/relay_event_utility.c b/src/backend/distributed/relay/relay_event_utility.c index 2bd597575..3748e63d9 100644 --- a/src/backend/distributed/relay/relay_event_utility.c +++ b/src/backend/distributed/relay/relay_event_utility.c @@ -51,7 +51,6 @@ #include "utils/relcache.h" /* Local functions forward declarations */ -static void AppendShardIdToConstraintName(AlterTableCmd *command, uint64 shardId); static bool UpdateWholeRowColumnReferencesWalker(Node *node, uint64 *shardId); /* exports for SQL callable functions */ @@ -85,6 +84,7 @@ RelayEventExtendNames(Node *parseTree, char *schemaName, uint64 shardId) */ AlterTableStmt *alterTableStmt = (AlterTableStmt *) parseTree; + Oid relationId = InvalidOid; char **relationName = &(alterTableStmt->relation->relname); char **relationSchemaName = &(alterTableStmt->relation->schemaname); @@ -104,6 +104,7 @@ RelayEventExtendNames(Node *parseTree, char *schemaName, uint64 shardId) if (command->subtype == AT_AddConstraint) { Constraint *constraint = (Constraint *) command->def; + char **constraintName = &(constraint->conname); if (constraint->contype == CONSTR_PRIMARY && constraint->indexname) { @@ -111,15 +112,30 @@ RelayEventExtendNames(Node *parseTree, char *schemaName, uint64 shardId) AppendShardIdToName(indexName, shardId); } - AppendShardIdToConstraintName(command, shardId); + AppendShardIdToName(constraintName, shardId); } - else if (command->subtype == AT_DropConstraint) + else if (command->subtype == AT_DropConstraint || + command->subtype == AT_ValidateConstraint) { - AppendShardIdToConstraintName(command, shardId); - } - else if (command->subtype == AT_ValidateConstraint) - { - AppendShardIdToConstraintName(command, shardId); + char **constraintName = &(command->name); + Oid constraintOid = InvalidOid; + const bool constraintMissingOk = true; + + if (!OidIsValid(relationId)) + { + const bool rvMissingOk = false; + relationId = RangeVarGetRelid(alterTableStmt->relation, + AccessShareLock, + rvMissingOk); + } + + constraintOid = get_relation_constraint_oid(relationId, + command->name, + constraintMissingOk); + if (!OidIsValid(constraintOid)) + { + AppendShardIdToName(constraintName, shardId); + } } else if (command->subtype == AT_ClusterOn) { @@ -585,30 +601,6 @@ RelayEventExtendNamesForInterShardCommands(Node *parseTree, uint64 leftShardId, } -/* - * AppendShardIdToConstraintName extends given constraint name with given - * shardId. Note that we only extend constraint names if they correspond to - * indexes, and the caller should verify that index correspondence before - * calling this function. - */ -static void -AppendShardIdToConstraintName(AlterTableCmd *command, uint64 shardId) -{ - if (command->subtype == AT_AddConstraint) - { - Constraint *constraint = (Constraint *) command->def; - char **constraintName = &(constraint->conname); - AppendShardIdToName(constraintName, shardId); - } - else if (command->subtype == AT_DropConstraint || - command->subtype == AT_ValidateConstraint) - { - char **constraintName = &(command->name); - AppendShardIdToName(constraintName, shardId); - } -} - - /* * UpdateWholeRowColumnReferencesWalker extends ColumnRef nodes that end with A_Star * with the given shardId. diff --git a/src/test/regress/expected/multi_alter_table_add_constraints.out b/src/test/regress/expected/multi_alter_table_add_constraints.out index bb06efa92..3458427a7 100644 --- a/src/test/regress/expected/multi_alter_table_add_constraints.out +++ b/src/test/regress/expected/multi_alter_table_add_constraints.out @@ -424,6 +424,11 @@ ALTER TABLE products ADD CHECK(product_no <> 0); ERROR: cannot create constraint without a name on a distributed table ALTER TABLE products ADD EXCLUDE USING btree (product_no with =); ERROR: cannot create constraint without a name on a distributed table +-- ... with names, we can add/drop the constraints just fine +ALTER TABLE products ADD CONSTRAINT nonzero_product_no CHECK(product_no <> 0); +ALTER TABLE products ADD CONSTRAINT uniq_product_no EXCLUDE USING btree (product_no with =); +ALTER TABLE products DROP CONSTRAINT nonzero_product_no; +ALTER TABLE products DROP CONSTRAINT uniq_product_no; DROP TABLE products; -- Tests with transactions CREATE TABLE products ( diff --git a/src/test/regress/expected/multi_create_table_constraints.out b/src/test/regress/expected/multi_create_table_constraints.out index 878b79085..90fa5b4a8 100644 --- a/src/test/regress/expected/multi_create_table_constraints.out +++ b/src/test/regress/expected/multi_create_table_constraints.out @@ -350,6 +350,20 @@ SELECT "Constraint", "Definition" FROM table_checks WHERE relid='public.check_ex (2 rows) \c - - - :master_port +-- Index-based constraints are created with shard-extended names, but others +-- (e.g. expression-based table CHECK constraints) do _not_ have shardids in +-- their object names, _at least originally as designed_. At some point, we +-- mistakenly started extending _all_ constraint names, but _only_ for ALTER +-- TABLE ... ADD CONSTRAINT commands (yes, even non-index constraints). So now +-- the _same_ constraint definition could result in a non-extended name if made +-- using CREATE TABLE and another name if made using AT ... ADD CONSTRAINT. So +-- DROP CONSTRAINT started erroring because _it_ was also changed to always do +-- shard-id extension. We've fixed that by looking for the non-extended name +-- first and using it for DROP or VALIDATE commands that could be targeting it. +-- As for the actual test: drop a constraint created by CREATE TABLE ... CHECK, +-- which per the above description would have been created with a non-extended +-- object name, but previously would have failed DROP as DROP does extension. +ALTER TABLE check_example DROP CONSTRAINT check_example_other_col_check; -- drop unnecessary tables DROP TABLE pk_on_non_part_col, uq_on_non_part_col CASCADE; DROP TABLE pk_on_part_col, uq_part_col, uq_two_columns CASCADE;