diff --git a/src/backend/distributed/commands/statistics.c b/src/backend/distributed/commands/statistics.c index 9a1797fc8..c922a6f31 100644 --- a/src/backend/distributed/commands/statistics.c +++ b/src/backend/distributed/commands/statistics.c @@ -37,6 +37,7 @@ #include "distributed/relation_access_tracking.h" #include "distributed/resource_lock.h" #include "distributed/worker_transaction.h" +#include "miscadmin.h" #include "utils/builtins.h" #include "utils/fmgroids.h" #include "utils/lsyscache.h" @@ -45,6 +46,7 @@ static List * GetExplicitStatisticsIdList(Oid relationId); static Oid GetRelIdByStatsOid(Oid statsOid); +static char * CreateAlterCommandIfOwnerNotDefault(Oid statsOid); #if PG_VERSION_NUM >= PG_VERSION_13 static char * CreateAlterCommandIfTargetNotDefault(Oid statsOid); #endif @@ -420,20 +422,33 @@ GetExplicitStatisticsCommandList(Oid relationId) char *createStatisticsCommand = pg_get_statisticsobj_worker(statisticsId, false); - createStatisticsCommandList = lappend( - createStatisticsCommandList, - makeTableDDLCommandString(createStatisticsCommand)); + createStatisticsCommandList = + lappend(createStatisticsCommandList, + makeTableDDLCommandString(createStatisticsCommand)); #if PG_VERSION_NUM >= PG_VERSION_13 - char *alterStatisticsTargetCommand = CreateAlterCommandIfTargetNotDefault( - statisticsId); + + /* we need to alter stats' target if it's getting distributed after creation */ + char *alterStatisticsTargetCommand = + CreateAlterCommandIfTargetNotDefault(statisticsId); if (alterStatisticsTargetCommand != NULL) { - alterStatisticsCommandList = lappend(alterStatisticsCommandList, - makeTableDDLCommandString( - alterStatisticsTargetCommand)); + alterStatisticsCommandList = + lappend(alterStatisticsCommandList, + makeTableDDLCommandString(alterStatisticsTargetCommand)); } #endif + + /* we need to alter stats' owner if it's getting distributed after creation */ + char *alterStatisticsOwnerCommand = + CreateAlterCommandIfOwnerNotDefault(statisticsId); + + if (alterStatisticsOwnerCommand != NULL) + { + alterStatisticsCommandList = + lappend(alterStatisticsCommandList, + makeTableDDLCommandString(alterStatisticsOwnerCommand)); + } } /* revert back to original search_path */ @@ -559,6 +574,46 @@ GetRelIdByStatsOid(Oid statsOid) } +/* + * CreateAlterCommandIfOwnerNotDefault returns an ALTER STATISTICS .. OWNER TO + * command if the stats object with given id has an owner different than the default one. + * Returns NULL otherwise. + */ +static char * +CreateAlterCommandIfOwnerNotDefault(Oid statsOid) +{ + HeapTuple tup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(statsOid)); + + if (!HeapTupleIsValid(tup)) + { + ereport(WARNING, (errmsg("No stats object found with id: %u", statsOid))); + return NULL; + } + + Form_pg_statistic_ext statisticsForm = (Form_pg_statistic_ext) GETSTRUCT(tup); + ReleaseSysCache(tup); + + if (statisticsForm->stxowner == GetUserId()) + { + return NULL; + } + + char *schemaName = get_namespace_name(statisticsForm->stxnamespace); + char *statName = NameStr(statisticsForm->stxname); + char *ownerName = GetUserNameFromId(statisticsForm->stxowner, false); + + StringInfoData str; + initStringInfo(&str); + + appendStringInfo(&str, "ALTER STATISTICS %s OWNER TO %s", + NameListToQuotedString(list_make2(makeString(schemaName), + makeString(statName))), + quote_identifier(ownerName)); + + return str.data; +} + + #if PG_VERSION_NUM >= PG_VERSION_13 /* @@ -585,12 +640,11 @@ CreateAlterCommandIfTargetNotDefault(Oid statsOid) return NULL; } - AlterStatsStmt *alterStatsStmt = palloc0(sizeof(AlterStatsStmt)); + AlterStatsStmt *alterStatsStmt = makeNode(AlterStatsStmt); char *schemaName = get_namespace_name(statisticsForm->stxnamespace); char *statName = NameStr(statisticsForm->stxname); - alterStatsStmt->type = T_AlterStatsStmt; alterStatsStmt->stxstattarget = statisticsForm->stxstattarget; alterStatsStmt->defnames = list_make2(makeString(schemaName), makeString(statName)); diff --git a/src/backend/distributed/deparser/deparse_statistics_stmts.c b/src/backend/distributed/deparser/deparse_statistics_stmts.c index f4658b0d7..125d4494f 100644 --- a/src/backend/distributed/deparser/deparse_statistics_stmts.c +++ b/src/backend/distributed/deparser/deparse_statistics_stmts.c @@ -188,8 +188,8 @@ static void AppendAlterStatisticsOwnerStmt(StringInfo buf, AlterOwnerStmt *stmt) { List *names = (List *) stmt->object; - appendStringInfo(buf, "ALTER STATISTICS %s OWNER TO %s", NameListToQuotedString( - names), + appendStringInfo(buf, "ALTER STATISTICS %s OWNER TO %s", + NameListToQuotedString(names), RoleSpecString(stmt->newowner, true)); } diff --git a/src/backend/distributed/relay/relay_event_utility.c b/src/backend/distributed/relay/relay_event_utility.c index c1f114d79..9e1ca2865 100644 --- a/src/backend/distributed/relay/relay_event_utility.c +++ b/src/backend/distributed/relay/relay_event_utility.c @@ -89,8 +89,7 @@ RelayEventExtendNames(Node *parseTree, char *schemaName, uint64 shardId) RangeVar *stat = makeRangeVarFromNameList( (List *) alterObjectSchemaStmt->object); - /* set schema name and append shard id */ - SetSchemaNameIfNotExist(&stat->schemaname, schemaName); + /* append shard id */ AppendShardIdToName(&stat->relname, shardId); alterObjectSchemaStmt->object = (Node *) MakeNameListFromRangeVar(stat); @@ -118,7 +117,6 @@ RelayEventExtendNames(Node *parseTree, char *schemaName, uint64 shardId) RangeVar *stat = makeRangeVarFromNameList(alterStatsStmt->defnames); AppendShardIdToName(&stat->relname, shardId); - SetSchemaNameIfNotExist(&stat->schemaname, schemaName); alterStatsStmt->defnames = MakeNameListFromRangeVar(stat); diff --git a/src/test/regress/expected/pg13_propagate_statistics.out b/src/test/regress/expected/pg13_propagate_statistics.out new file mode 100644 index 000000000..c75eb333f --- /dev/null +++ b/src/test/regress/expected/pg13_propagate_statistics.out @@ -0,0 +1,112 @@ +SHOW server_version \gset +SELECT substring(:'server_version', '\d+')::int > 12 AS server_version_above_twelve +\gset +\if :server_version_above_twelve +\else +\q +\endif +CREATE SCHEMA "statistics'TestTarget"; +SET search_path TO "statistics'TestTarget"; +SET citus.next_shard_id TO 980000; +SET client_min_messages TO WARNING; +SET citus.shard_count TO 32; +SET citus.shard_replication_factor TO 1; +CREATE TABLE t1 (a int, b int); +CREATE STATISTICS s1 ON a,b FROM t1; +CREATE STATISTICS s2 ON a,b FROM t1; +CREATE STATISTICS s3 ON a,b FROM t1; +CREATE STATISTICS s4 ON a,b FROM t1; +-- test altering stats target +-- test alter target before distribution +ALTER STATISTICS s1 SET STATISTICS 3; +-- since max value for target is 10000, this will automatically be lowered +ALTER STATISTICS s2 SET STATISTICS 999999; +WARNING: lowering statistics target to 10000 +SELECT create_distributed_table('t1', 'b'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- test alter target before distribution +ALTER STATISTICS s3 SET STATISTICS 46; +\c - - - :worker_1_port +SELECT stxstattarget, stxrelid::regclass +FROM pg_statistic_ext +WHERE stxnamespace IN ( + SELECT oid + FROM pg_namespace + WHERE nspname IN ('statistics''TestTarget') +) +ORDER BY stxstattarget, stxrelid::regclass ASC; + stxstattarget | stxrelid +--------------------------------------------------------------------- + -1 | "statistics'TestTarget".t1_980000 + -1 | "statistics'TestTarget".t1_980002 + -1 | "statistics'TestTarget".t1_980004 + -1 | "statistics'TestTarget".t1_980006 + -1 | "statistics'TestTarget".t1_980008 + -1 | "statistics'TestTarget".t1_980010 + -1 | "statistics'TestTarget".t1_980012 + -1 | "statistics'TestTarget".t1_980014 + -1 | "statistics'TestTarget".t1_980016 + -1 | "statistics'TestTarget".t1_980018 + -1 | "statistics'TestTarget".t1_980020 + -1 | "statistics'TestTarget".t1_980022 + -1 | "statistics'TestTarget".t1_980024 + -1 | "statistics'TestTarget".t1_980026 + -1 | "statistics'TestTarget".t1_980028 + -1 | "statistics'TestTarget".t1_980030 + 3 | "statistics'TestTarget".t1_980000 + 3 | "statistics'TestTarget".t1_980002 + 3 | "statistics'TestTarget".t1_980004 + 3 | "statistics'TestTarget".t1_980006 + 3 | "statistics'TestTarget".t1_980008 + 3 | "statistics'TestTarget".t1_980010 + 3 | "statistics'TestTarget".t1_980012 + 3 | "statistics'TestTarget".t1_980014 + 3 | "statistics'TestTarget".t1_980016 + 3 | "statistics'TestTarget".t1_980018 + 3 | "statistics'TestTarget".t1_980020 + 3 | "statistics'TestTarget".t1_980022 + 3 | "statistics'TestTarget".t1_980024 + 3 | "statistics'TestTarget".t1_980026 + 3 | "statistics'TestTarget".t1_980028 + 3 | "statistics'TestTarget".t1_980030 + 46 | "statistics'TestTarget".t1_980000 + 46 | "statistics'TestTarget".t1_980002 + 46 | "statistics'TestTarget".t1_980004 + 46 | "statistics'TestTarget".t1_980006 + 46 | "statistics'TestTarget".t1_980008 + 46 | "statistics'TestTarget".t1_980010 + 46 | "statistics'TestTarget".t1_980012 + 46 | "statistics'TestTarget".t1_980014 + 46 | "statistics'TestTarget".t1_980016 + 46 | "statistics'TestTarget".t1_980018 + 46 | "statistics'TestTarget".t1_980020 + 46 | "statistics'TestTarget".t1_980022 + 46 | "statistics'TestTarget".t1_980024 + 46 | "statistics'TestTarget".t1_980026 + 46 | "statistics'TestTarget".t1_980028 + 46 | "statistics'TestTarget".t1_980030 + 10000 | "statistics'TestTarget".t1_980000 + 10000 | "statistics'TestTarget".t1_980002 + 10000 | "statistics'TestTarget".t1_980004 + 10000 | "statistics'TestTarget".t1_980006 + 10000 | "statistics'TestTarget".t1_980008 + 10000 | "statistics'TestTarget".t1_980010 + 10000 | "statistics'TestTarget".t1_980012 + 10000 | "statistics'TestTarget".t1_980014 + 10000 | "statistics'TestTarget".t1_980016 + 10000 | "statistics'TestTarget".t1_980018 + 10000 | "statistics'TestTarget".t1_980020 + 10000 | "statistics'TestTarget".t1_980022 + 10000 | "statistics'TestTarget".t1_980024 + 10000 | "statistics'TestTarget".t1_980026 + 10000 | "statistics'TestTarget".t1_980028 + 10000 | "statistics'TestTarget".t1_980030 +(64 rows) + +\c - - - :master_port +SET client_min_messages TO WARNING; +DROP SCHEMA "statistics'TestTarget" CASCADE; diff --git a/src/test/regress/expected/pg13_propagate_statistics_0.out b/src/test/regress/expected/pg13_propagate_statistics_0.out new file mode 100644 index 000000000..e25fbb82d --- /dev/null +++ b/src/test/regress/expected/pg13_propagate_statistics_0.out @@ -0,0 +1,6 @@ +SHOW server_version \gset +SELECT substring(:'server_version', '\d+')::int > 12 AS server_version_above_twelve +\gset +\if :server_version_above_twelve +\else +\q diff --git a/src/test/regress/expected/propagate_statistics.out b/src/test/regress/expected/propagate_statistics.out index 470a92826..80e501fff 100644 --- a/src/test/regress/expected/propagate_statistics.out +++ b/src/test/regress/expected/propagate_statistics.out @@ -79,23 +79,18 @@ ALTER STATISTICS sc1.st1 RENAME TO st1_new; -- test altering stats schema CREATE SCHEMA test_alter_schema; ALTER STATISTICS s7 SET SCHEMA test_alter_schema; --- test altering stats target -ALTER STATISTICS s1 SET STATISTICS 3; --- since max value for target is 10000, this will automatically be lowered -ALTER STATISTICS s2 SET STATISTICS 999999; -WARNING: lowering statistics target to 10000 --- test alter target before distribution -CREATE TABLE targettable(a int, b int); -CREATE STATISTICS s8 ON a,b FROM targettable; -ALTER STATISTICS s8 SET STATISTICS 46; -SELECT create_distributed_table('targettable', 'b'); +-- test alter owner +ALTER STATISTICS sc2."neW'Stat" OWNER TO pg_monitor; +-- test alter owner before distribution +CREATE TABLE ownertest(a int, b int); +CREATE STATISTICS sc1.s9 ON a,b FROM ownertest; +ALTER STATISTICS sc1.s9 OWNER TO pg_signal_backend; +SELECT create_distributed_table('ownertest','a'); create_distributed_table --------------------------------------------------------------------- (1 row) --- test alter owner -ALTER STATISTICS s8 OWNER TO pg_monitor; \c - - - :worker_1_port SELECT stxname FROM pg_statistic_ext @@ -155,22 +150,22 @@ ORDER BY stxname ASC; s2_980058 s2_980060 s2_980062 - s8_980129 - s8_980131 - s8_980133 - s8_980135 - s8_980137 - s8_980139 - s8_980141 - s8_980143 - s8_980145 - s8_980147 - s8_980149 - s8_980151 - s8_980153 - s8_980155 - s8_980157 - s8_980159 + s9_980129 + s9_980131 + s9_980133 + s9_980135 + s9_980137 + s9_980139 + s9_980141 + s9_980143 + s9_980145 + s9_980147 + s9_980149 + s9_980151 + s9_980153 + s9_980155 + s9_980157 + s9_980159 st1_new_980064 st1_new_980066 st1_new_980068 @@ -201,102 +196,16 @@ WHERE stxnamespace IN ( 3 (1 row) -SELECT stxstattarget +SELECT COUNT(DISTINCT stxowner) FROM pg_statistic_ext WHERE stxnamespace IN ( SELECT oid FROM pg_namespace WHERE nspname IN ('public', 'statistics''Test', 'sc1', 'sc2') -) -ORDER BY stxstattarget ASC; - stxstattarget ---------------------------------------------------------------------- - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - -1 - 3 - 3 - 3 - 3 - 3 - 3 - 3 - 3 - 3 - 3 - 3 - 3 - 3 - 3 - 3 - 3 - 46 - 46 - 46 - 46 - 46 - 46 - 46 - 46 - 46 - 46 - 46 - 46 - 46 - 46 - 46 - 46 - 10000 - 10000 - 10000 - 10000 - 10000 - 10000 - 10000 - 10000 - 10000 - 10000 - 10000 - 10000 - 10000 - 10000 - 10000 - 10000 -(80 rows) - -SELECT COUNT(DISTINCT stxowner) FROM pg_statistic_ext; +); count --------------------------------------------------------------------- - 2 + 3 (1 row) \c - - - :master_port diff --git a/src/test/regress/multi_schedule b/src/test/regress/multi_schedule index 5f862dc71..8a65fb475 100644 --- a/src/test/regress/multi_schedule +++ b/src/test/regress/multi_schedule @@ -94,6 +94,7 @@ test: tableam # Tests for statistics propagation # ---------- test: propagate_statistics +test: pg13_propagate_statistics # ---------- # Miscellaneous tests to check our query planning behavior diff --git a/src/test/regress/sql/pg13_propagate_statistics.sql b/src/test/regress/sql/pg13_propagate_statistics.sql new file mode 100644 index 000000000..9716d3289 --- /dev/null +++ b/src/test/regress/sql/pg13_propagate_statistics.sql @@ -0,0 +1,45 @@ +SHOW server_version \gset +SELECT substring(:'server_version', '\d+')::int > 12 AS server_version_above_twelve +\gset +\if :server_version_above_twelve +\else +\q +\endif + +CREATE SCHEMA "statistics'TestTarget"; +SET search_path TO "statistics'TestTarget"; +SET citus.next_shard_id TO 980000; +SET client_min_messages TO WARNING; +SET citus.shard_count TO 32; +SET citus.shard_replication_factor TO 1; + +CREATE TABLE t1 (a int, b int); +CREATE STATISTICS s1 ON a,b FROM t1; +CREATE STATISTICS s2 ON a,b FROM t1; +CREATE STATISTICS s3 ON a,b FROM t1; +CREATE STATISTICS s4 ON a,b FROM t1; + +-- test altering stats target +-- test alter target before distribution +ALTER STATISTICS s1 SET STATISTICS 3; +-- since max value for target is 10000, this will automatically be lowered +ALTER STATISTICS s2 SET STATISTICS 999999; + +SELECT create_distributed_table('t1', 'b'); + +-- test alter target before distribution +ALTER STATISTICS s3 SET STATISTICS 46; + +\c - - - :worker_1_port +SELECT stxstattarget, stxrelid::regclass +FROM pg_statistic_ext +WHERE stxnamespace IN ( + SELECT oid + FROM pg_namespace + WHERE nspname IN ('statistics''TestTarget') +) +ORDER BY stxstattarget, stxrelid::regclass ASC; + +\c - - - :master_port +SET client_min_messages TO WARNING; +DROP SCHEMA "statistics'TestTarget" CASCADE; diff --git a/src/test/regress/sql/propagate_statistics.sql b/src/test/regress/sql/propagate_statistics.sql index e9b78ee57..8a0372cf7 100644 --- a/src/test/regress/sql/propagate_statistics.sql +++ b/src/test/regress/sql/propagate_statistics.sql @@ -67,18 +67,13 @@ ALTER STATISTICS sc1.st1 RENAME TO st1_new; CREATE SCHEMA test_alter_schema; ALTER STATISTICS s7 SET SCHEMA test_alter_schema; --- test altering stats target -ALTER STATISTICS s1 SET STATISTICS 3; --- since max value for target is 10000, this will automatically be lowered -ALTER STATISTICS s2 SET STATISTICS 999999; --- test alter target before distribution -CREATE TABLE targettable(a int, b int); -CREATE STATISTICS s8 ON a,b FROM targettable; -ALTER STATISTICS s8 SET STATISTICS 46; -SELECT create_distributed_table('targettable', 'b'); - -- test alter owner -ALTER STATISTICS s8 OWNER TO pg_monitor; +ALTER STATISTICS sc2."neW'Stat" OWNER TO pg_monitor; +-- test alter owner before distribution +CREATE TABLE ownertest(a int, b int); +CREATE STATISTICS sc1.s9 ON a,b FROM ownertest; +ALTER STATISTICS sc1.s9 OWNER TO pg_signal_backend; +SELECT create_distributed_table('ownertest','a'); \c - - - :worker_1_port SELECT stxname @@ -98,16 +93,13 @@ WHERE stxnamespace IN ( WHERE nspname IN ('public', 'statistics''Test', 'sc1', 'sc2') ); -SELECT stxstattarget +SELECT COUNT(DISTINCT stxowner) FROM pg_statistic_ext WHERE stxnamespace IN ( SELECT oid FROM pg_namespace WHERE nspname IN ('public', 'statistics''Test', 'sc1', 'sc2') -) -ORDER BY stxstattarget ASC; - -SELECT COUNT(DISTINCT stxowner) FROM pg_statistic_ext; +); \c - - - :master_port SET client_min_messages TO WARNING;