From 1d6c81245cdb5579de1a2d86d8180e89aeac22e0 Mon Sep 17 00:00:00 2001 From: aykutbozkurt Date: Fri, 10 Jun 2022 20:30:01 +0300 Subject: [PATCH] fix bug, which is column mismatch of shard tasks when specifying column names for citus tables in vacuum and analyze commands --- src/backend/distributed/commands/vacuum.c | 39 +++++++++++++------ src/test/regress/expected/multi_utilities.out | 12 ++++++ src/test/regress/sql/multi_utilities.sql | 7 ++++ 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/src/backend/distributed/commands/vacuum.c b/src/backend/distributed/commands/vacuum.c index 42eb00467..31300f353 100644 --- a/src/backend/distributed/commands/vacuum.c +++ b/src/backend/distributed/commands/vacuum.c @@ -48,6 +48,7 @@ typedef struct CitusVacuumParams } CitusVacuumParams; /* Local functions forward declarations for processing distributed table commands */ +static bool IsDistributedVacuumStmt(List *vacuumRelationIdList); static List * VacuumTaskList(Oid relationId, CitusVacuumParams vacuumParams, List *vacuumColumnList); static char * DeparseVacuumStmtPrefix(CitusVacuumParams vacuumParams); @@ -59,8 +60,8 @@ static void ExecuteVacuumOnDistributedTables(VacuumStmt *vacuumStmt, List *relat static void ExecuteUnqualifiedVacuumTasks(VacuumStmt *vacuumStmt, CitusVacuumParams vacuumParams); static CitusVacuumParams VacuumStmtParams(VacuumStmt *vacstmt); -static List * VacuumCitusRelationIdList(VacuumStmt *vacuumStmt, CitusVacuumParams - vacuumParams); +static List * VacuumRelationIdList(VacuumStmt *vacuumStmt, CitusVacuumParams + vacuumParams); /* * PostprocessVacuumStmt processes vacuum statements that may need propagation to @@ -95,7 +96,7 @@ PostprocessVacuumStmt(Node *node, const char *vacuumCommand) * when no table is specified propagate the command as it is; * otherwise, only propagate when there is at least 1 citus table */ - List *citusRelationIdList = VacuumCitusRelationIdList(vacuumStmt, vacuumParams); + List *relationIdList = VacuumRelationIdList(vacuumStmt, vacuumParams); if (list_length(vacuumStmt->rels) == 0) { @@ -103,11 +104,11 @@ PostprocessVacuumStmt(Node *node, const char *vacuumCommand) ExecuteUnqualifiedVacuumTasks(vacuumStmt, vacuumParams); } - else if (list_length(citusRelationIdList) > 0) + else if (IsDistributedVacuumStmt(relationIdList)) { /* there is at least 1 citus table specified */ - ExecuteVacuumOnDistributedTables(vacuumStmt, citusRelationIdList, + ExecuteVacuumOnDistributedTables(vacuumStmt, relationIdList, vacuumParams); } @@ -118,10 +119,10 @@ PostprocessVacuumStmt(Node *node, const char *vacuumCommand) /* - * VacuumCitusRelationIdList returns the oid of the citus relations in the given vacuum statement. + * VacuumRelationIdList returns the oid of the relations in the given vacuum statement. */ static List * -VacuumCitusRelationIdList(VacuumStmt *vacuumStmt, CitusVacuumParams vacuumParams) +VacuumRelationIdList(VacuumStmt *vacuumStmt, CitusVacuumParams vacuumParams) { LOCKMODE lockMode = (vacuumParams.options & VACOPT_FULL) ? AccessExclusiveLock : ShareUpdateExclusiveLock; @@ -134,10 +135,6 @@ VacuumCitusRelationIdList(VacuumStmt *vacuumStmt, CitusVacuumParams vacuumParams foreach_ptr(vacuumRelation, vacuumRelationList) { Oid relationId = RangeVarGetRelid(vacuumRelation, lockMode, false); - if (!IsCitusTable(relationId)) - { - continue; - } relationIdList = lappend_oid(relationIdList, relationId); } @@ -145,6 +142,26 @@ VacuumCitusRelationIdList(VacuumStmt *vacuumStmt, CitusVacuumParams vacuumParams } +/* + * IsDistributedVacuumStmt returns true if there is any citus table in the relation id list; + * otherwise, it returns false. + */ +static bool +IsDistributedVacuumStmt(List *vacuumRelationIdList) +{ + Oid relationId = InvalidOid; + foreach_oid(relationId, vacuumRelationIdList) + { + if (OidIsValid(relationId) && IsCitusTable(relationId)) + { + return true; + } + } + + return false; +} + + /* * ExecuteVacuumOnDistributedTables executes the vacuum for the shard placements of given tables * if they are citus tables. diff --git a/src/test/regress/expected/multi_utilities.out b/src/test/regress/expected/multi_utilities.out index 41e426508..ce03e3c18 100644 --- a/src/test/regress/expected/multi_utilities.out +++ b/src/test/regress/expected/multi_utilities.out @@ -387,6 +387,14 @@ SELECT create_distributed_table('distributed_analyze_table', 'id'); (1 row) +CREATE TABLE loc (a INT, b INT); +CREATE TABLE dist (a INT); +SELECT create_distributed_table ('dist', 'a'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + SET citus.log_remote_commands TO ON; -- should propagate to all workers because no table is specified ANALYZE; @@ -480,3 +488,7 @@ DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx SET citus.enable_ddl_propagation TO OFF; ANALYZE distributed_analyze_table; SET citus.enable_ddl_propagation TO ON; +-- analyze only specified columns for corresponding tables +ANALYZE loc(b), dist(a); +NOTICE: issuing ANALYZE public.dist_970004 (a) +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx diff --git a/src/test/regress/sql/multi_utilities.sql b/src/test/regress/sql/multi_utilities.sql index ba11960a1..c84796181 100644 --- a/src/test/regress/sql/multi_utilities.sql +++ b/src/test/regress/sql/multi_utilities.sql @@ -246,6 +246,10 @@ SELECT create_reference_table('reference_analyze_table'); CREATE TABLE distributed_analyze_table(id int); SELECT create_distributed_table('distributed_analyze_table', 'id'); +CREATE TABLE loc (a INT, b INT); +CREATE TABLE dist (a INT); +SELECT create_distributed_table ('dist', 'a'); + SET citus.log_remote_commands TO ON; -- should propagate to all workers because no table is specified @@ -270,3 +274,6 @@ ANALYZE local_analyze_table, reference_analyze_table; SET citus.enable_ddl_propagation TO OFF; ANALYZE distributed_analyze_table; SET citus.enable_ddl_propagation TO ON; + +-- analyze only specified columns for corresponding tables +ANALYZE loc(b), dist(a);