Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/backend/distributed/commands/table.c
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,28 @@ SwitchToSequentialAndLocalExecutionIfConstraintNameTooLong(Oid relationId,
return;
}

/*
* For non-index constraints (CHECK, FOREIGN KEY), PostgreSQL propagates
* the exact same constraint name from the parent to all partition children
* (see ATAddCheckConstraint and addFkConstraint in tablecmds.c). Since
* user-provided identifiers are already bounded by NAMEDATALEN at parse
* time, the inherited name can never be too long on partition shards.
*
* This also correctly handles unnamed non-index constraints whose
* conname was assigned by PrepareAlterTableStmtForConstraint(), since
* those generated names are also bounded by NAMEDATALEN.
*
* Only index-backed constraints (PRIMARY KEY, UNIQUE, EXCLUDE) need this
* check because PostgreSQL auto-generates new names for partition child
* indexes based on the partition table name (see generateClonedIndexStmt
* in parse_utilcmd.c which sets idxname = NULL), and those generated
* names can exceed NAMEDATALEN when the partition name is long.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: the check also (correctly) applies to handling unnamed non-index constraints (e.g., unnamed CHECK) whose conname was assigned by PrepareAlterTableStmtForConstraint() earlier, right ? Those generated names are also bounded by NAMEDATALEN, so this early return is correct for them too. How about extending the comment to describe that also, with something like:

 * This also correctly handles unnamed non-index constraints whose 
 * conname was assigned by PrepareAlterTableStmtForConstraint(), since
 * those generated names are also bounded by NAMEDATALEN.

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.

Good catch! I've extended the comment to document this.

*/
if (constraint->conname != NULL && !ConstrTypeUsesIndex(constraint->contype))
{
return;
}

char *longestPartitionShardName = get_rel_name(longestNamePartitionId);
ShardInterval *shardInterval = LoadShardIntervalWithLongestShardName(
longestNamePartitionId);
Expand Down Expand Up @@ -1375,6 +1397,9 @@ PreprocessAlterTableStmt(Node *node, const char *alterTableCommand,
leftRelationId,
constraint);
}

SwitchToSequentialAndLocalExecutionIfConstraintNameTooLong(
leftRelationId, constraint);
}

/*
Expand All @@ -1397,6 +1422,17 @@ PreprocessAlterTableStmt(Node *node, const char *alterTableCommand,
constraint);
}
}
else if (constraint->conname != NULL)
{
/*
* When the user provides an explicit constraint name, we still
* need to check if the resulting constraint name on the shards
* of partitions would be too long, and if so, switch to
* sequential execution to prevent self-deadlocks.
*/
SwitchToSequentialAndLocalExecutionIfConstraintNameTooLong(
leftRelationId, constraint);
}
}
else if (alterTableType == AT_DropConstraint)
{
Expand Down
173 changes: 173 additions & 0 deletions src/test/regress/expected/multi_alter_table_add_constraints.out
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,179 @@ DROP SCHEMA sc3 CASCADE;
NOTICE: drop cascades to 2 other objects
DETAIL: drop cascades to table sc3.alter_add_prim_key
drop cascades to table sc3.alter_add_unique
-- Test that named constraints on partitioned tables with long partition names
-- switch to sequential execution to prevent self-deadlocks (issue #7799)
CREATE SCHEMA test_named_constraint_long_partition;
SET search_path TO 'test_named_constraint_long_partition';
CREATE TABLE dist_partitioned_table (dist_col int, another_col int, partition_col timestamp)
PARTITION BY RANGE (partition_col);
CREATE TABLE p1 PARTITION OF dist_partitioned_table
FOR VALUES FROM ('2021-01-01') TO ('2022-01-01');
CREATE TABLE longlonglonglonglonglonglonglonglonglonglonglonglonglonglongabc
PARTITION OF dist_partitioned_table
FOR VALUES FROM ('2020-01-01') TO ('2021-01-01');
SELECT create_distributed_table('dist_partitioned_table', 'partition_col');
create_distributed_table
---------------------------------------------------------------------

(1 row)

-- Check "ADD CONSTRAINT ... PRIMARY KEY" with explicit name switches to sequential
SET client_min_messages TO DEBUG1;
ALTER TABLE dist_partitioned_table ADD CONSTRAINT my_pk PRIMARY KEY(partition_col);
DEBUG: the constraint name on the shards of the partition is too long, switching to sequential and local execution mode to prevent self deadlocks: longlonglonglonglonglonglonglonglonglonglonglo_537570f5_14_pkey
DEBUG: ALTER TABLE / ADD PRIMARY KEY will create implicit index "my_pk" for table "dist_partitioned_table"
DEBUG: ALTER TABLE / ADD PRIMARY KEY will create implicit index "longlonglonglonglonglonglonglonglonglonglonglonglonglonglo_pkey" for table "longlonglonglonglonglonglonglonglonglonglonglonglonglonglongabc"
DEBUG: ALTER TABLE / ADD PRIMARY KEY will create implicit index "p1_pkey" for table "p1"
DEBUG: verifying table "p1"
DEBUG: verifying table "longlonglonglonglonglonglonglonglonglonglonglonglonglonglongabc"
RESET client_min_messages;
-- Verify constraint is created on the coordinator
SELECT con.conname
FROM pg_catalog.pg_constraint con
INNER JOIN pg_catalog.pg_class rel ON rel.oid = con.conrelid
INNER JOIN pg_catalog.pg_namespace nsp ON nsp.oid = rel.relnamespace
WHERE rel.relname = 'dist_partitioned_table'
AND con.contype = 'p';
conname
---------------------------------------------------------------------
my_pk
(1 row)

ALTER TABLE dist_partitioned_table DROP CONSTRAINT my_pk;
-- Check "ADD CONSTRAINT ... UNIQUE" with explicit name switches to sequential
SET client_min_messages TO DEBUG1;
ALTER TABLE dist_partitioned_table ADD CONSTRAINT my_uq UNIQUE(partition_col);
DEBUG: the constraint name on the shards of the partition is too long, switching to sequential and local execution mode to prevent self deadlocks: longlonglonglonglonglonglonglonglonglonglongl_partition_col_key
DEBUG: ALTER TABLE / ADD UNIQUE will create implicit index "my_uq" for table "dist_partitioned_table"
DEBUG: ALTER TABLE / ADD UNIQUE will create implicit index "longlonglonglonglonglonglonglonglonglonglongl_partition_col_key" for table "longlonglonglonglonglonglonglonglonglonglonglonglonglonglongabc"
DEBUG: ALTER TABLE / ADD UNIQUE will create implicit index "p1_partition_col_key" for table "p1"
RESET client_min_messages;
SELECT con.conname
FROM pg_catalog.pg_constraint con
INNER JOIN pg_catalog.pg_class rel ON rel.oid = con.conrelid
INNER JOIN pg_catalog.pg_namespace nsp ON nsp.oid = rel.relnamespace
WHERE rel.relname = 'dist_partitioned_table'
AND con.contype = 'u';
conname
---------------------------------------------------------------------
my_uq
(1 row)

ALTER TABLE dist_partitioned_table DROP CONSTRAINT my_uq;
-- Check "ADD CONSTRAINT ... CHECK" with explicit name does NOT switch to sequential
-- because PG propagates the user-provided name to partitions as-is (no auto-generation)
SET client_min_messages TO DEBUG1;
ALTER TABLE dist_partitioned_table ADD CONSTRAINT my_chk CHECK(dist_col > another_col);
DEBUG: verifying table "p1"
DEBUG: verifying table "longlonglonglonglonglonglonglonglonglonglonglonglonglonglongabc"
RESET client_min_messages;
SELECT con.conname
FROM pg_catalog.pg_constraint con
INNER JOIN pg_catalog.pg_class rel ON rel.oid = con.conrelid
INNER JOIN pg_catalog.pg_namespace nsp ON nsp.oid = rel.relnamespace
WHERE rel.relname = 'dist_partitioned_table'
AND con.contype = 'c';
conname
---------------------------------------------------------------------
my_chk
(1 row)

ALTER TABLE dist_partitioned_table DROP CONSTRAINT my_chk;
-- Check "ADD CONSTRAINT ... EXCLUDE" with explicit name switches to sequential
-- because PG auto-generates new index names on partitions (same as PK/UNIQUE)
-- EXCLUDE on partitioned tables is only supported in PG17+
SHOW server_version \gset
SELECT substring(:'server_version', '\d+')::int >= 17 AS server_version_ge_17
\gset
\if :server_version_ge_17
SET client_min_messages TO DEBUG1;
ALTER TABLE dist_partitioned_table ADD CONSTRAINT my_excl EXCLUDE USING btree (partition_col WITH =);
DEBUG: the constraint name on the shards of the partition is too long, switching to sequential and local execution mode to prevent self deadlocks: longlonglonglonglonglonglonglonglonglonglong_partition_col_excl
DEBUG: ALTER TABLE / ADD EXCLUDE will create implicit index "my_excl" for table "dist_partitioned_table"
DEBUG: ALTER TABLE / ADD EXCLUDE will create implicit index "longlonglonglonglonglonglonglonglonglonglong_partition_col_excl" for table "longlonglonglonglonglonglonglonglonglonglonglonglonglonglongabc"
DEBUG: ALTER TABLE / ADD EXCLUDE will create implicit index "p1_partition_col_excl" for table "p1"
RESET client_min_messages;
SELECT con.conname
FROM pg_catalog.pg_constraint con
INNER JOIN pg_catalog.pg_class rel ON rel.oid = con.conrelid
INNER JOIN pg_catalog.pg_namespace nsp ON nsp.oid = rel.relnamespace
WHERE rel.relname = 'dist_partitioned_table'
AND con.contype = 'x';
conname
---------------------------------------------------------------------
my_excl
(1 row)

ALTER TABLE dist_partitioned_table DROP CONSTRAINT my_excl;
\endif
-- Check "ADD CONSTRAINT ... FOREIGN KEY" with explicit name does NOT switch to
-- sequential because PG propagates the user-provided name to partitions as-is
-- FK requires shard_replication_factor = 1, so create a separate table for this test
SET citus.shard_replication_factor TO 1;
CREATE TABLE fk_dist_partitioned_table (dist_col int, another_col int, partition_col timestamp)
PARTITION BY RANGE (partition_col);
CREATE TABLE fk_p1 PARTITION OF fk_dist_partitioned_table
FOR VALUES FROM ('2021-01-01') TO ('2022-01-01');
CREATE TABLE fk_longlonglonglonglonglonglonglonglonglonglonglonglonglongabc
PARTITION OF fk_dist_partitioned_table
FOR VALUES FROM ('2020-01-01') TO ('2021-01-01');
SELECT create_distributed_table('fk_dist_partitioned_table', 'partition_col');
create_distributed_table
---------------------------------------------------------------------

(1 row)

CREATE TABLE ref_table (ref_col timestamp PRIMARY KEY);
SELECT create_reference_table('ref_table');
create_reference_table
---------------------------------------------------------------------

(1 row)

SET client_min_messages TO DEBUG1;
ALTER TABLE fk_dist_partitioned_table ADD CONSTRAINT my_fk FOREIGN KEY (partition_col) REFERENCES ref_table(ref_col);
RESET client_min_messages;
SELECT con.conname
FROM pg_catalog.pg_constraint con
INNER JOIN pg_catalog.pg_class rel ON rel.oid = con.conrelid
INNER JOIN pg_catalog.pg_namespace nsp ON nsp.oid = rel.relnamespace
WHERE rel.relname = 'fk_dist_partitioned_table'
AND con.contype = 'f';
conname
---------------------------------------------------------------------
my_fk
(1 row)

ALTER TABLE fk_dist_partitioned_table DROP CONSTRAINT my_fk;
RESET citus.shard_replication_factor;
-- Check that we error out when adding a named constraint in a transaction block
-- after a parallel query has already been executed
BEGIN;
SELECT count(*) FROM dist_partitioned_table;
count
---------------------------------------------------------------------
0
(1 row)

ALTER TABLE dist_partitioned_table ADD CONSTRAINT my_pk PRIMARY KEY(partition_col);
ERROR: The constraint name (longlonglonglonglonglonglonglonglonglonglonglo_537570f5_14_pkey) on a shard is too long and could lead to deadlocks when executed in a transaction block after a parallel query
HINT: Try re-running the transaction with "SET LOCAL citus.multi_shard_modify_mode TO 'sequential';"
ROLLBACK;
-- try inside a sequential block -- should succeed
BEGIN;
SET LOCAL citus.multi_shard_modify_mode TO 'sequential';
SELECT count(*) FROM dist_partitioned_table;
count
---------------------------------------------------------------------
0
(1 row)

ALTER TABLE dist_partitioned_table ADD CONSTRAINT my_pk PRIMARY KEY(partition_col);
ROLLBACK;
SET client_min_messages TO ERROR;
DROP SCHEMA test_named_constraint_long_partition CASCADE;
SET search_path TO 'public';
CREATE SCHEMA test_auto_explain;
SET search_path TO 'test_auto_explain';
-- Test ALTER TABLE ... ADD CONSTRAINT ... does not cause a crash when auto_explain module is loaded
Expand Down
Loading
Loading