Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed issue of dissapearing comments after undistributing table. #7635

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fixed issue of dissapearing comments after undistributing table. #7635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4e525fc
CommentMigration
EPoikans Jun 14, 2024
45fca50
Code formatting cleanup
Jun 21, 2024
729478d
Merge branch 'main' of https://github.com/citusdata/citus
Jun 25, 2024
abc6869
Merge branch 'main' into main
EPoikans Jul 12, 2024
1e97fa7
Merge branch 'main' into main
EPoikans Jul 18, 2024
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
43 changes: 40 additions & 3 deletions src/backend/distributed/commands/alter_table.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "catalog/pg_am.h"
#include "catalog/pg_depend.h"
#include "catalog/pg_rewrite_d.h"
#include "commands/comment.h"
#include "commands/defrem.h"
#include "executor/spi.h"
#include "nodes/pg_list.h"
Expand Down Expand Up @@ -844,7 +845,6 @@ ConvertTableInternal(TableConversionState *con)
*/
.cascadeViaForeignKeys = false
};

TableConversionReturn *partitionReturn = con->function(&partitionParam);
if (cascadeOption == CASCADE_TO_COLOCATED_NO_ALREADY_CASCADED)
{
Expand Down Expand Up @@ -1032,7 +1032,6 @@ ConvertTableInternal(TableConversionState *con)

/* increment command counter so that next command can see the new table */
CommandCounterIncrement();

SetLocalEnableLocalReferenceForeignKeys(oldEnableLocalReferenceForeignKeys);

InTableTypeConversionFunctionCall = false;
Expand Down Expand Up @@ -1781,6 +1780,41 @@ CreateMaterializedViewDDLCommand(Oid matViewOid)
}


/*
* MigrateColumnComments migrates distributed table column comments to the target undistributed table columns.
*/
static void
MigrateColumnComments(Oid sourceId, Oid targetId)
{
Relation relation = relation_open(sourceId, AccessShareLock);
TupleDesc tupleDesc = RelationGetDescr(relation);
for (int attrNum = 0; attrNum < tupleDesc->natts; attrNum++)
{
Form_pg_attribute attr = TupleDescAttr(tupleDesc, attrNum);
if (!attr->attisdropped)
{
char *columnComment = GetComment(sourceId, RelationRelationId, attrNum + 1);
CreateComments(targetId, RelationRelationId, attrNum + 1, columnComment);
}
}
relation_close(relation, AccessShareLock);
}


/*
* MigrateTableComment migrates the comment of the source distributed table to the target undistributed table.
*/
static void
MigrateTableComment(Oid sourceId, Oid targetId)
{
char *comment = GetComment(sourceId, RelationRelationId, 0);
if (comment != NULL)
{
CreateComments(targetId, RelationRelationId, 0, comment);
}
}


/*
* ReplaceTable replaces the source table with the target table.
* It moves all the rows of the source table to target table with INSERT SELECT.
Expand All @@ -1796,7 +1830,6 @@ ReplaceTable(Oid sourceId, Oid targetId, List *justBeforeDropCommands,
char *sourceName = get_rel_name(sourceId);
char *qualifiedSourceName = generate_qualified_relation_name(sourceId);
char *qualifiedTargetName = generate_qualified_relation_name(targetId);

StringInfo query = makeStringInfo();

if (!PartitionedTable(sourceId) && !IsForeignTable(sourceId))
Expand All @@ -1814,6 +1847,7 @@ ReplaceTable(Oid sourceId, Oid targetId, List *justBeforeDropCommands,
*/
appendStringInfo(query, "INSERT INTO %s SELECT * FROM %s",
qualifiedTargetName, qualifiedSourceName);
MigrateColumnComments(sourceId, targetId);
}
else
{
Expand All @@ -1833,6 +1867,7 @@ ReplaceTable(Oid sourceId, Oid targetId, List *justBeforeDropCommands,
}

ExecuteQueryViaSPI(query->data, SPI_OK_INSERT);
MigrateColumnComments(sourceId, targetId);
}

/*
Expand Down Expand Up @@ -1883,6 +1918,8 @@ ReplaceTable(Oid sourceId, Oid targetId, List *justBeforeDropCommands,
ereport(NOTICE, (errmsg("dropping the old %s", qualifiedSourceName)));
}

MigrateTableComment(sourceId, targetId);

resetStringInfo(query);
appendStringInfo(query, "DROP %sTABLE %s CASCADE",
IsForeignTable(sourceId) ? "FOREIGN " : "",
Expand Down
3 changes: 3 additions & 0 deletions src/test/regress/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ check-isolation-custom-schedule-vg: all $(isolation_test_files)
--valgrind --pg_ctl-timeout=360 --connection-timeout=500000 --valgrind-path=valgrind --valgrind-log-file=$(CITUS_VALGRIND_LOG_FILE) \
-- $(MULTI_REGRESS_OPTS) --inputdir=$(citus_abs_srcdir)/build --schedule=$(citus_abs_srcdir)/$(SCHEDULE) $(EXTRA_TESTS)

check-comment-migration: all
$(pg_regress_multi_check) --load-extension=citus \
-- $(MULTI_REGRESS_OPTS) --schedule=$(citus_abs_srcdir)/comment_migration_schedule $(EXTRA_TESTS)

check-empty: all
$(pg_regress_multi_check) --load-extension=citus \
Expand Down
7 changes: 7 additions & 0 deletions src/test/regress/comment_migration_schedule
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Tests comment migration during undistribute_table
test: comment_migration_table
test: comment_migration_table_joined_tables
test: comment_migration_table_joined_tables_FK
test: comment_migration_column
test: comment_migration_column_joined_tables
test: comment_migration_column_joined_tables_FK
65 changes: 65 additions & 0 deletions src/test/regress/expected/comment_migration_column.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
CREATE SCHEMA comment_migration_column;
CREATE TABLE comment_migration_column.table_1
(
id bigserial,
code varchar(200) not null,
name varchar(200),
date_created timestamp with time zone default NOW() not null,
active boolean default true,
CONSTRAINT table_1_pkey PRIMARY KEY (id)
)
WITH (autovacuum_enabled = TRUE);
SET citus.shard_replication_factor = 1;
comment on column comment_migration_column.table_1.id is 'table id';
comment on column comment_migration_column.table_1.code is 'table code';
comment on column comment_migration_column.table_1.name is 'table name';
comment on column comment_migration_column.table_1.date_created is 'table date_created';
comment on column comment_migration_column.table_1.active is 'table active';
select col_description('comment_migration_column.table_1'::regclass,1), col_description('comment_migration_column.table_1'::regclass,2), col_description('comment_migration_column.table_1'::regclass,3), col_description('comment_migration_column.table_1'::regclass,4), col_description('comment_migration_column.table_1'::regclass,5);
col_description | col_description | col_description | col_description | col_description
---------------------------------------------------------------------
table id | table code | table name | table date_created | table active
(1 row)

SELECT create_distributed_table('comment_migration_column.table_1', 'id');
create_distributed_table
---------------------------------------------------------------------

(1 row)

select col_description('comment_migration_column.table_1'::regclass,1), col_description('comment_migration_column.table_1'::regclass,2), col_description('comment_migration_column.table_1'::regclass,3), col_description('comment_migration_column.table_1'::regclass,4), col_description('comment_migration_column.table_1'::regclass,5);
col_description | col_description | col_description | col_description | col_description
---------------------------------------------------------------------
table id | table code | table name | table date_created | table active
(1 row)

select undistribute_table('comment_migration_column.table_1');
NOTICE: creating a new table for comment_migration_column.table_1
NOTICE: moving the data of comment_migration_column.table_1
NOTICE: dropping the old comment_migration_column.table_1
NOTICE: renaming the new table to comment_migration_column.table_1
undistribute_table
---------------------------------------------------------------------

(1 row)

select col_description('comment_migration_column.table_1'::regclass,1), col_description('comment_migration_column.table_1'::regclass,2), col_description('comment_migration_column.table_1'::regclass,3), col_description('comment_migration_column.table_1'::regclass,4), col_description('comment_migration_column.table_1'::regclass,5);
col_description | col_description | col_description | col_description | col_description
---------------------------------------------------------------------
table id | table code | table name | table date_created | table active
(1 row)

SELECT create_distributed_table('comment_migration_column.table_1', 'id');
create_distributed_table
---------------------------------------------------------------------

(1 row)

select col_description('comment_migration_column.table_1'::regclass,1), col_description('comment_migration_column.table_1'::regclass,2), col_description('comment_migration_column.table_1'::regclass,3), col_description('comment_migration_column.table_1'::regclass,4), col_description('comment_migration_column.table_1'::regclass,5);
col_description | col_description | col_description | col_description | col_description
---------------------------------------------------------------------
table id | table code | table name | table date_created | table active
(1 row)

DROP TABLE comment_migration_column.table_1;
DROP SCHEMA comment_migration_column;
101 changes: 101 additions & 0 deletions src/test/regress/expected/comment_migration_column_joined_tables.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
CREATE SCHEMA comment_migration_column;
CREATE TABLE comment_migration_column.table_1
(
id bigserial,
code varchar(200) not null,
name varchar(200),
date_created timestamp with time zone default NOW() not null,
active boolean default true,
tenant_id bigint,
CONSTRAINT table_1_pkey PRIMARY KEY (id, tenant_id)
)
WITH (autovacuum_enabled = TRUE);
CREATE TABLE comment_migration_column.table_2
(
id bigserial,
table_1_id bigint ,
description varchar(200),
tenant_id bigint,
CONSTRAINT table_2_pkey PRIMARY KEY (id, tenant_id)
)
WITH (autovacuum_enabled = TRUE);
SET citus.shard_replication_factor = 1;
comment on column comment_migration_column.table_1.id is 'table 1 id';
comment on column comment_migration_column.table_1.code is 'table 1 code';
comment on column comment_migration_column.table_1.name is 'table 1 name';
comment on column comment_migration_column.table_1.date_created is 'table 1 date_created';
comment on column comment_migration_column.table_1.active is 'table 1 active';
comment on column comment_migration_column.table_2.id is 'table 2 id';
comment on column comment_migration_column.table_2.table_1_id is 'table 2 foreign key to table 1';
comment on column comment_migration_column.table_2.description is 'table 2 description';
select col_description('comment_migration_column.table_1'::regclass,1), col_description('comment_migration_column.table_1'::regclass,2), col_description('comment_migration_column.table_1'::regclass,3), col_description('comment_migration_column.table_1'::regclass,4), col_description('comment_migration_column.table_1'::regclass,5);
col_description | col_description | col_description | col_description | col_description
---------------------------------------------------------------------
table 1 id | table 1 code | table 1 name | table 1 date_created | table 1 active
(1 row)

select col_description('comment_migration_column.table_2'::regclass,1), col_description('comment_migration_column.table_2'::regclass,2), col_description('comment_migration_column.table_2'::regclass,3);
col_description | col_description | col_description
---------------------------------------------------------------------
table 2 id | table 2 foreign key to table 1 | table 2 description
(1 row)

SELECT create_distributed_table('comment_migration_column.table_1', 'tenant_id');
create_distributed_table
---------------------------------------------------------------------

(1 row)

SELECT create_distributed_table('comment_migration_column.table_2', 'tenant_id', colocate_with=>'comment_migration_column.table_1');
create_distributed_table
---------------------------------------------------------------------

(1 row)

SELECT undistribute_table('comment_migration_column.table_1');
NOTICE: creating a new table for comment_migration_column.table_1
NOTICE: moving the data of comment_migration_column.table_1
NOTICE: dropping the old comment_migration_column.table_1
NOTICE: renaming the new table to comment_migration_column.table_1
undistribute_table
---------------------------------------------------------------------

(1 row)

SELECT undistribute_table('comment_migration_column.table_2');
NOTICE: creating a new table for comment_migration_column.table_2
NOTICE: moving the data of comment_migration_column.table_2
NOTICE: dropping the old comment_migration_column.table_2
NOTICE: renaming the new table to comment_migration_column.table_2
undistribute_table
---------------------------------------------------------------------

(1 row)

SELECT create_distributed_table('comment_migration_column.table_1', 'tenant_id');
create_distributed_table
---------------------------------------------------------------------

(1 row)

SELECT create_distributed_table('comment_migration_column.table_2', 'tenant_id', colocate_with=>'comment_migration_column.table_1');
create_distributed_table
---------------------------------------------------------------------

(1 row)

select col_description('comment_migration_column.table_1'::regclass,1), col_description('comment_migration_column.table_1'::regclass,2), col_description('comment_migration_column.table_1'::regclass,3), col_description('comment_migration_column.table_1'::regclass,4), col_description('comment_migration_column.table_1'::regclass,5);
col_description | col_description | col_description | col_description | col_description
---------------------------------------------------------------------
table 1 id | table 1 code | table 1 name | table 1 date_created | table 1 active
(1 row)

select col_description('comment_migration_column.table_2'::regclass,1), col_description('comment_migration_column.table_2'::regclass,2), col_description('comment_migration_column.table_2'::regclass,3);
col_description | col_description | col_description
---------------------------------------------------------------------
table 2 id | table 2 foreign key to table 1 | table 2 description
(1 row)

DROP TABLE comment_migration_column.table_2;
DROP TABLE comment_migration_column.table_1;
DROP SCHEMA comment_migration_column;
98 changes: 98 additions & 0 deletions src/test/regress/expected/comment_migration_column_joined_tables_FK.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
CREATE SCHEMA comment_migration_column;
CREATE TABLE comment_migration_column.table_1
(
id bigserial,
code varchar(200) not null,
name varchar(200),
date_created timestamp with time zone default NOW() not null,
active boolean default true,
owner_id bigint,
CONSTRAINT table_1_pkey PRIMARY KEY (id)
)
WITH (autovacuum_enabled = TRUE);
CREATE TABLE comment_migration_column.table_2
(
id bigserial,
table_1_id bigint ,
description varchar(200),
owner_id bigint,
CONSTRAINT table_2_pkey PRIMARY KEY (id)
)
WITH (autovacuum_enabled = TRUE);
SET citus.shard_replication_factor = 1;
comment on column comment_migration_column.table_1.id is 'table 1 id';
comment on column comment_migration_column.table_1.code is 'table 1 code';
comment on column comment_migration_column.table_1.name is 'table 1 name';
comment on column comment_migration_column.table_1.date_created is 'table 1 date_created';
comment on column comment_migration_column.table_1.active is 'table 1 active';
comment on column comment_migration_column.table_2.id is 'table 2 id';
comment on column comment_migration_column.table_2.table_1_id is 'table 2 foreign key to table 1';
comment on column comment_migration_column.table_2.description is 'table 2 description';
select col_description('comment_migration_column.table_1'::regclass,1), col_description('comment_migration_column.table_1'::regclass,2), col_description('comment_migration_column.table_1'::regclass,3), col_description('comment_migration_column.table_1'::regclass,4), col_description('comment_migration_column.table_1'::regclass,5);
col_description | col_description | col_description | col_description | col_description
---------------------------------------------------------------------
table 1 id | table 1 code | table 1 name | table 1 date_created | table 1 active
(1 row)

select col_description('comment_migration_column.table_2'::regclass,1), col_description('comment_migration_column.table_2'::regclass,2), col_description('comment_migration_column.table_2'::regclass,3);
col_description | col_description | col_description
---------------------------------------------------------------------
table 2 id | table 2 foreign key to table 1 | table 2 description
(1 row)

SELECT create_reference_table('comment_migration_column.table_1');
create_reference_table
---------------------------------------------------------------------

(1 row)

SELECT create_distributed_table('comment_migration_column.table_2', 'id');
create_distributed_table
---------------------------------------------------------------------

(1 row)

ALTER TABLE comment_migration_column.table_2 ADD CONSTRAINT table2_table1_fk FOREIGN KEY (table_1_id) REFERENCES comment_migration_column.table_1(id);
SELECT undistribute_table('comment_migration_column.table_1', cascade_via_foreign_keys=>true);
NOTICE: creating a new table for comment_migration_column.table_1
NOTICE: moving the data of comment_migration_column.table_1
NOTICE: dropping the old comment_migration_column.table_1
NOTICE: renaming the new table to comment_migration_column.table_1
NOTICE: creating a new table for comment_migration_column.table_2
NOTICE: moving the data of comment_migration_column.table_2
NOTICE: dropping the old comment_migration_column.table_2
NOTICE: renaming the new table to comment_migration_column.table_2
undistribute_table
---------------------------------------------------------------------

(1 row)

ALTER TABLE comment_migration_column.table_2 DROP CONSTRAINT table2_table1_fk;
SELECT create_reference_table('comment_migration_column.table_1');
create_reference_table
---------------------------------------------------------------------

(1 row)

SELECT create_distributed_table('comment_migration_column.table_2', 'id');
create_distributed_table
---------------------------------------------------------------------

(1 row)

ALTER TABLE comment_migration_column.table_2 ADD CONSTRAINT table2_table1_fk FOREIGN KEY (table_1_id) REFERENCES comment_migration_column.table_1(id);
select col_description('comment_migration_column.table_1'::regclass,1), col_description('comment_migration_column.table_1'::regclass,2), col_description('comment_migration_column.table_1'::regclass,3), col_description('comment_migration_column.table_1'::regclass,4), col_description('comment_migration_column.table_1'::regclass,5);
col_description | col_description | col_description | col_description | col_description
---------------------------------------------------------------------
table 1 id | table 1 code | table 1 name | table 1 date_created | table 1 active
(1 row)

select col_description('comment_migration_column.table_2'::regclass,1), col_description('comment_migration_column.table_2'::regclass,2), col_description('comment_migration_column.table_2'::regclass,3);
col_description | col_description | col_description
---------------------------------------------------------------------
table 2 id | table 2 foreign key to table 1 | table 2 description
(1 row)

DROP TABLE comment_migration_column.table_2;
DROP TABLE comment_migration_column.table_1;
DROP SCHEMA comment_migration_column;
Loading