external
/
citus
mirror of https://github.com/citusdata/citus.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Implement ALTER TABLE ... RENAME TO ... 

The implementation was already mostly in place, but the code was protected
by a principled check against the operation. Turns out there's a nasty
concurrency bug though with long identifier names, much as in #1664.

To prevent deadlocks from happening, we could either review the DDL
transaction management in shards and placements, or we can simply reject
names with (NAMEDATALEN - 1) chars or more — that's because of the
PostgreSQL array types being created with a one-char prefix: '_'.
pull/1939/head
Dimitri Fontaine 2018-01-09 21:04:11 +01:00
parent ef3517a5dc
commit e010238280
7 changed files with 123 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/backend/distributed/executor/multi_utility.c
View File

@ -2740,12 +2740,7 @@ ErrorIfUnsupportedRenameStmt(RenameStmt *renameStmt)
{
Assert(IsAlterTableRenameStmt(renameStmt));
if (renameStmt->renameType == OBJECT_TABLE)
{
ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("renaming distributed tables is currently unsupported")));
}
else if (renameStmt->renameType == OBJECT_TABCONSTRAINT)
if (renameStmt->renameType == OBJECT_TABCONSTRAINT)
{
ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("renaming constraints belonging to distributed tables is "

25
src/backend/distributed/relay/relay_event_utility.c
View File

@ -388,12 +388,37 @@ RelayEventExtendNames(Node *parseTree, char *schemaName, uint64 shardId)
char **oldRelationName = &(renameStmt->relation->relname);
char **newRelationName = &(renameStmt->newname);
char **objectSchemaName = &(renameStmt->relation->schemaname);
int newRelationNameLength;
/* prefix with schema name if it is not added already */
SetSchemaNameIfNotExist(objectSchemaName, schemaName);
AppendShardIdToName(oldRelationName, shardId);
AppendShardIdToName(newRelationName, shardId);
/*
* PostgreSQL creates array types for each ordinary table, with
* the same name plus a prefix of '_'.
*
* ALTER TABLE ... RENAME TO ... also renames the underlying
* array type, and the DDL is run in parallel connections over
* all the placements and shards at once. Concurrent access
* here deadlocks.
*
* Let's provide an easier to understand error message here
* than the deadlock one.
*
* See also https://github.com/citusdata/citus/issues/1664
*/
newRelationNameLength = strlen(*newRelationName);
if (newRelationNameLength >= (NAMEDATALEN - 1))
{
ereport(ERROR,
(errcode(ERRCODE_NAME_TOO_LONG),
errmsg(
"shard name %s exceeds %d characters",
*newRelationName, NAMEDATALEN - 1)));
}
}
else if (objectType == OBJECT_COLUMN || objectType == OBJECT_TRIGGER)
{

3
src/test/regress/expected/multi_name_lengths.out
View File

@ -113,7 +113,8 @@ SELECT "Constraint", "Definition" FROM table_checks WHERE relid='public.name_len
\c - - - :master_port
-- Placeholders for RENAME operations
ALTER TABLE name_lengths RENAME TO name_len_12345678901234567890123456789012345678901234567890;
ERROR: renaming distributed tables is currently unsupported
ERROR: shard name name_len_12345678901234567890123456789012345678_fcd8ab6f_225002 exceeds 63 characters
CONTEXT: while executing command on localhost:57638
ALTER TABLE name_lengths RENAME CONSTRAINT unique_12345678901234567890123456789012345678901234567890 TO unique2_12345678901234567890123456789012345678901234567890;
ERROR: renaming constraints belonging to distributed tables is currently unsupported
-- Verify that CREATE INDEX on already distributed table has proper shard names.

7
src/test/regress/expected/multi_reference_table.out
View File

@ -1382,12 +1382,13 @@ SELECT "Column", "Type", "Modifiers" FROM table_desc WHERE relid='public.referen
(0 rows)
\c - - - :master_port
-- as we expect, renaming and setting WITH OIDS does not work for reference tables
ALTER TABLE reference_table_ddl RENAME TO reference_table_ddl_test;
ERROR: renaming distributed tables is currently unsupported
-- as we expect, setting WITH OIDS does not work for reference tables
ALTER TABLE reference_table_ddl SET WITH OIDS;
ERROR: alter table command is currently unsupported
DETAIL: Only ADD|DROP COLUMN, SET|DROP NOT NULL, SET|DROP DEFAULT, ADD|DROP CONSTRAINT, ATTACH|DETACH PARTITION and TYPE subcommands are supported.
-- now test the renaming of the table, and back to the expected name
ALTER TABLE reference_table_ddl RENAME TO reference_table_ddl_test;
ALTER TABLE reference_table_ddl_test RENAME TO reference_table_ddl;
-- now test reference tables against some helper UDFs that Citus provides
-- cannot delete / drop shards from a reference table
SELECT master_apply_delete_command('DELETE FROM reference_table_ddl');

25
src/test/regress/input/multi_alter_table_statements.source
View File

@ -148,9 +148,8 @@ ALTER TABLE lineitem_alter ADD COLUMN new_column non_existent_type;
ALTER TABLE lineitem_alter ALTER COLUMN null_column SET NOT NULL;
ALTER TABLE lineitem_alter ALTER COLUMN l_partkey SET DEFAULT 'a';
-- Verify that we error out on non-column RENAME statements
-- Verify that we error out on RENAME CONSTRAINT statement
ALTER TABLE lineitem_alter RENAME TO lineitem_renamed;
ALTER TABLE lineitem_alter RENAME CONSTRAINT constraint_a TO constraint_b;
-- Verify that IF EXISTS works as expected with RENAME statements
@ -356,10 +355,30 @@ FROM
ORDER BY attnum;
\c - - - :master_port
-- verify that we can rename distributed tables
SHOW citus.enable_ddl_propagation;
ALTER TABLE lineitem_alter RENAME TO lineitem_renamed;
-- verify rename is performed
SELECT relname FROM pg_class WHERE relname = 'lineitem_renamed';
-- show rename worked on one worker, too
\c - - - :worker_1_port
SELECT relname FROM pg_class WHERE relname LIKE 'lineitem_renamed%' ORDER BY relname;
\c - - - :master_port
-- revert it to original name
ALTER TABLE lineitem_renamed RENAME TO lineitem_alter;
-- show rename worked on one worker, too
\c - - - :worker_1_port
SELECT relname FROM pg_class WHERE relname LIKE 'lineitem_alter%' ORDER BY relname;
\c - - - :master_port
-- verify that we don't intercept DDL commands if propagation is turned off
SET citus.enable_ddl_propagation to false;
-- table rename statement can be performed now
-- table rename statement can be performed on the coordinator only now
ALTER TABLE lineitem_alter RENAME TO lineitem_renamed;
-- verify rename is performed
SELECT relname FROM pg_class WHERE relname = 'lineitem_alter' or relname = 'lineitem_renamed';

68
src/test/regress/output/multi_alter_table_statements.source
View File

@ -377,9 +377,7 @@ ERROR: column "null_column" contains null values
CONTEXT: while executing command on localhost:57638
ALTER TABLE lineitem_alter ALTER COLUMN l_partkey SET DEFAULT 'a';
ERROR: invalid input syntax for integer: "a"
-- Verify that we error out on non-column RENAME statements
ALTER TABLE lineitem_alter RENAME TO lineitem_renamed;
ERROR: renaming distributed tables is currently unsupported
-- Verify that we error out on RENAME CONSTRAINT statement
ALTER TABLE lineitem_alter RENAME CONSTRAINT constraint_a TO constraint_b;
ERROR: renaming constraints belonging to distributed tables is currently unsupported
-- Verify that IF EXISTS works as expected with RENAME statements
@ -783,10 +781,72 @@ ORDER BY attnum;
........pg.dropped.24........ | -
(30 rows)
\c - - - :master_port
-- verify that we can rename distributed tables
SHOW citus.enable_ddl_propagation;
citus.enable_ddl_propagation
------------------------------
on
(1 row)
ALTER TABLE lineitem_alter RENAME TO lineitem_renamed;
-- verify rename is performed
SELECT relname FROM pg_class WHERE relname = 'lineitem_renamed';
relname
------------------
lineitem_renamed
(1 row)
-- show rename worked on one worker, too
\c - - - :worker_1_port
SELECT relname FROM pg_class WHERE relname LIKE 'lineitem_renamed%' ORDER BY relname;
relname
-------------------------
lineitem_renamed_220000
lineitem_renamed_220001
lineitem_renamed_220002
lineitem_renamed_220003
lineitem_renamed_220004
lineitem_renamed_220005
lineitem_renamed_220006
lineitem_renamed_220007
lineitem_renamed_220008
lineitem_renamed_220010
lineitem_renamed_220011
lineitem_renamed_220012
lineitem_renamed_220013
lineitem_renamed_220014
(14 rows)
\c - - - :master_port
-- revert it to original name
ALTER TABLE lineitem_renamed RENAME TO lineitem_alter;
-- show rename worked on one worker, too
\c - - - :worker_1_port
SELECT relname FROM pg_class WHERE relname LIKE 'lineitem_alter%' ORDER BY relname;
relname
-----------------------
lineitem_alter_220000
lineitem_alter_220001
lineitem_alter_220002
lineitem_alter_220003
lineitem_alter_220004
lineitem_alter_220005
lineitem_alter_220006
lineitem_alter_220007
lineitem_alter_220008
lineitem_alter_220009
lineitem_alter_220010
lineitem_alter_220011
lineitem_alter_220012
lineitem_alter_220013
lineitem_alter_220014
(15 rows)
\c - - - :master_port
-- verify that we don't intercept DDL commands if propagation is turned off
SET citus.enable_ddl_propagation to false;
-- table rename statement can be performed now
-- table rename statement can be performed on the coordinator only now
ALTER TABLE lineitem_alter RENAME TO lineitem_renamed;
-- verify rename is performed
SELECT relname FROM pg_class WHERE relname = 'lineitem_alter' or relname = 'lineitem_renamed';

7
src/test/regress/sql/multi_reference_table.sql
View File

@ -872,10 +872,13 @@ SELECT "Column", "Type", "Modifiers" FROM table_desc WHERE relid='public.referen
\di reference_index_2*
\c - - - :master_port
-- as we expect, renaming and setting WITH OIDS does not work for reference tables
ALTER TABLE reference_table_ddl RENAME TO reference_table_ddl_test;
-- as we expect, setting WITH OIDS does not work for reference tables
ALTER TABLE reference_table_ddl SET WITH OIDS;
-- now test the renaming of the table, and back to the expected name
ALTER TABLE reference_table_ddl RENAME TO reference_table_ddl_test;
ALTER TABLE reference_table_ddl_test RENAME TO reference_table_ddl;
-- now test reference tables against some helper UDFs that Citus provides
-- cannot delete / drop shards from a reference table