diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c index b354723be44..8e377ca445d 100644 --- a/src/backend/commands/statscmds.c +++ b/src/backend/commands/statscmds.c @@ -59,9 +59,15 @@ compare_int16(const void *a, const void *b) /* * CREATE STATISTICS + * + * relids is a list of OIDs of relations specified in the FROM clause, on which + * the statistics object is defined. We identify the target by the passed-in + * OID rather than re-resolving stmt->relations by name, so that we operate + * on exactly the relation the caller looked up. Only a single relation is + * supported for now. */ ObjectAddress -CreateStatistics(CreateStatsStmt *stmt, bool check_rights) +CreateStatistics(List *relids, CreateStatsStmt *stmt, bool check_rights) { int16 attnums[STATS_MAX_DIMENSIONS]; int nattnums = 0; @@ -70,7 +76,7 @@ CreateStatistics(CreateStatsStmt *stmt, bool check_rights) NameData stxname; Oid statoid; Oid namespaceId; - Oid stxowner = GetUserId(); + Oid stxowner = OidIsValid(stmt->owner) ? stmt->owner : GetUserId(); HeapTuple htup; Datum values[Natts_pg_statistic_ext]; bool nulls[Natts_pg_statistic_ext]; @@ -79,7 +85,7 @@ CreateStatistics(CreateStatsStmt *stmt, bool check_rights) Datum exprsDatum; Relation statrel; Relation rel = NULL; - Oid relid; + Oid relid = InvalidOid; ObjectAddress parentobject, myself; Datum types[4]; /* one for each possible type of statistic */ @@ -97,24 +103,17 @@ CreateStatistics(CreateStatsStmt *stmt, bool check_rights) Assert(IsA(stmt, CreateStatsStmt)); /* - * Examine the FROM clause. Currently, we only allow it to be a single - * simple table, but later we'll probably allow multiple tables and JOIN - * syntax. The grammar is already prepared for that, so we have to check - * here that what we got is what we can support. + * Currently, we only allow the FROM clause to be a single simple table, + * but later we'll probably allow multiple tables and JOIN syntax. The + * grammar and the loop below are already prepared for that, but examining + * the FROM clause is the caller's job, so all we do here is assert that + * the caller rejected what we can't support. */ - if (list_length(stmt->relations) != 1) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("only a single relation is allowed in CREATE STATISTICS"))); + Assert(list_length(relids) == 1); - foreach(cell, stmt->relations) + foreach(cell, relids) { - Node *rln = (Node *) lfirst(cell); - - if (!IsA(rln, RangeVar)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("only a single relation is allowed in CREATE STATISTICS"))); + relid = lfirst_oid(cell); /* * CREATE STATISTICS will influence future execution plans but does @@ -123,7 +122,7 @@ CreateStatistics(CreateStatsStmt *stmt, bool check_rights) * conflicting with ANALYZE and other DDL that sets statistical * information, but not with normal queries. */ - rel = relation_openrv((RangeVar *) rln, ShareUpdateExclusiveLock); + rel = relation_open(relid, ShareUpdateExclusiveLock); /* Restrict to allowed relation types */ if (rel->rd_rel->relkind != RELKIND_RELATION && @@ -137,13 +136,11 @@ CreateStatistics(CreateStatsStmt *stmt, bool check_rights) errdetail_relkind_not_supported(rel->rd_rel->relkind))); /* - * You must own the relation to create stats on it. - * - * NB: Concurrent changes could cause this function's lookup to find a - * different relation than a previous lookup by the caller, so we must - * perform this check even when check_rights == false. + * You must own the relation to create stats on it. Skip check if + * caller doesn't want it. */ - if (!object_ownercheck(RelationRelationId, RelationGetRelid(rel), stxowner)) + if (check_rights && + !object_ownercheck(RelationRelationId, RelationGetRelid(rel), stxowner)) aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(rel->rd_rel->relkind), RelationGetRelationName(rel)); @@ -156,7 +153,6 @@ CreateStatistics(CreateStatsStmt *stmt, bool check_rights) } Assert(rel); - relid = RelationGetRelid(rel); /* * If the node has a name, split it up and determine creation namespace. diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index a2003c75331..f8fedc2b249 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -211,6 +211,7 @@ typedef struct AlteredTableInfo char *clusterOnIndex; /* index to use for CLUSTER */ List *changedStatisticsOids; /* OIDs of statistics to rebuild */ List *changedStatisticsDefs; /* string definitions of same */ + List *changedStatisticsOwners; /* owners of same */ } AlteredTableInfo; /* Struct describing one new constraint to check in Phase 3 scan */ @@ -699,7 +700,7 @@ static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab); static void RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab); static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode); -static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, +static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, Oid ownerId, char *cmd, List **wqueue, LOCKMODE lockmode, bool rewrite); static void RebuildConstraintComment(AlteredTableInfo *tab, AlterTablePass pass, @@ -9774,7 +9775,11 @@ ATExecAddStatistics(AlteredTableInfo *tab, Relation rel, /* The CreateStatsStmt has already been through transformStatsStmt */ Assert(stmt->transformed); - address = CreateStatistics(stmt, !is_rebuild); + /* The owner must be set to the original statistics owner */ + Assert(OidIsValid(stmt->owner)); + + address = CreateStatistics(list_make1_oid(RelationGetRelid(rel)), + stmt, !is_rebuild); return address; } @@ -16057,11 +16062,25 @@ RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab) { /* OK, capture the statistics object's existing definition string */ char *defstring = pg_get_statisticsobjdef_string(stxoid); + HeapTuple tup; + Form_pg_statistic_ext statext; + + tup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(stxoid)); + + if (!HeapTupleIsValid(tup)) /* should not happen */ + elog(ERROR, "cache lookup failed for statistics object %u", stxoid); + + statext = (Form_pg_statistic_ext) GETSTRUCT(tup); tab->changedStatisticsOids = lappend_oid(tab->changedStatisticsOids, stxoid); tab->changedStatisticsDefs = lappend(tab->changedStatisticsDefs, defstring); + + tab->changedStatisticsOwners = lappend_oid(tab->changedStatisticsOwners, + statext->stxowner); + + ReleaseSysCache(tup); } } @@ -16079,6 +16098,7 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) ObjectAddresses *objects; ListCell *def_item; ListCell *oid_item; + ListCell *owner_item; /* * Collect all the constraints and indexes to drop so we can process them @@ -16152,7 +16172,7 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) if (relid != tab->relid) LockRelationOid(relid, AccessExclusiveLock); - ATPostAlterTypeParse(oldId, relid, confrelid, + ATPostAlterTypeParse(oldId, relid, confrelid, InvalidOid, (char *) lfirst(def_item), wqueue, lockmode, tab->rewrite); } @@ -16171,7 +16191,7 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) if (relid != tab->relid) LockRelationOid(relid, AccessExclusiveLock); - ATPostAlterTypeParse(oldId, relid, InvalidOid, + ATPostAlterTypeParse(oldId, relid, InvalidOid, InvalidOid, (char *) lfirst(def_item), wqueue, lockmode, tab->rewrite); @@ -16180,8 +16200,9 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) } /* add dependencies for new statistics */ - forboth(oid_item, tab->changedStatisticsOids, - def_item, tab->changedStatisticsDefs) + forthree(oid_item, tab->changedStatisticsOids, + def_item, tab->changedStatisticsDefs, + owner_item, tab->changedStatisticsOwners) { Oid oldId = lfirst_oid(oid_item); Oid relid; @@ -16201,7 +16222,7 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) if (relid != tab->relid) LockRelationOid(relid, ShareUpdateExclusiveLock); - ATPostAlterTypeParse(oldId, relid, InvalidOid, + ATPostAlterTypeParse(oldId, relid, InvalidOid, lfirst_oid(owner_item), (char *) lfirst(def_item), wqueue, lockmode, tab->rewrite); @@ -16265,8 +16286,9 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) * operator that's not available for the new column type. */ static void -ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, - List **wqueue, LOCKMODE lockmode, bool rewrite) +ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, Oid ownerId, + char *cmd, List **wqueue, LOCKMODE lockmode, + bool rewrite) { List *raw_parsetree_list; List *querytree_list; @@ -16306,10 +16328,14 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, querytree_list = list_concat(querytree_list, afterStmts); } else if (IsA(stmt, CreateStatsStmt)) - querytree_list = lappend(querytree_list, - transformStatsStmt(oldRelId, - (CreateStatsStmt *) stmt, - cmd)); + { + CreateStatsStmt *csstmt; + + csstmt = transformStatsStmt(oldRelId, (CreateStatsStmt *) stmt, cmd); + csstmt->owner = ownerId; + + querytree_list = lappend(querytree_list, csstmt); + } else querytree_list = lappend(querytree_list, stmt); } diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 73a56f1df1d..5f204addcd4 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1890,7 +1890,21 @@ ProcessUtilitySlow(ParseState *pstate, { Oid relid; CreateStatsStmt *stmt = (CreateStatsStmt *) parsetree; - RangeVar *rel = (RangeVar *) linitial(stmt->relations); + RangeVar *rel; + + /* + * Examine the FROM clause. Currently, we only allow it + * to be a single simple table, but later we'll probably + * allow multiple tables and JOIN syntax. The grammar is + * already prepared for that, so we have to check here + * that what we got is what we can support. + */ + if (list_length(stmt->relations) != 1) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("only a single relation is allowed in CREATE STATISTICS"))); + + rel = (RangeVar *) linitial(stmt->relations); if (!IsA(rel, RangeVar)) ereport(ERROR, @@ -1913,7 +1927,8 @@ ProcessUtilitySlow(ParseState *pstate, /* Run parse analysis ... */ stmt = transformStatsStmt(relid, stmt, queryString); - address = CreateStatistics(stmt, true); + address = CreateStatistics(list_make1_oid(relid), stmt, + true); } break; diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index d080ad59b71..574f860bdd2 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -86,7 +86,7 @@ extern void RemoveOperatorById(Oid operOid); extern ObjectAddress AlterOperator(AlterOperatorStmt *stmt); /* commands/statscmds.c */ -extern ObjectAddress CreateStatistics(CreateStatsStmt *stmt, bool check_rights); +extern ObjectAddress CreateStatistics(List *relids, CreateStatsStmt *stmt, bool check_rights); extern ObjectAddress AlterStatistics(AlterStatsStmt *stmt); extern void RemoveStatisticsById(Oid statsOid); extern void RemoveStatisticsDataById(Oid statsOid, bool inh); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index ad31a9c059b..2fcea826003 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3630,6 +3630,7 @@ typedef struct CreateStatsStmt char *stxcomment; /* comment to apply to stats, or NULL */ bool transformed; /* true when transformStatsStmt is finished */ bool if_not_exists; /* do nothing if stats name already exists */ + Oid owner; /* OID of owner, or InvalidOid for default */ } CreateStatsStmt; /* diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out index 37070c1a896..4512670d525 100644 --- a/src/test/regress/expected/stats_ext.out +++ b/src/test/regress/expected/stats_ext.out @@ -55,6 +55,10 @@ ERROR: duplicate expression in statistics definition CREATE STATISTICS tst (unrecognized) ON x, y FROM ext_stats_test; ERROR: unrecognized statistics kind "unrecognized" -- unsupported targets +CREATE STATISTICS tst ON x, y FROM ext_stats_test, ext_stats_test; +ERROR: only a single relation is allowed in CREATE STATISTICS +CREATE STATISTICS tst ON x, y FROM ext_stats_test, (SELECT * FROM ext_stats_test) AS foo; +ERROR: only a single relation is allowed in CREATE STATISTICS CREATE STATISTICS tst ON a FROM (VALUES (x)) AS foo; ERROR: CREATE STATISTICS only supports relation names in the FROM clause CREATE STATISTICS tst ON a FROM foo NATURAL JOIN bar; @@ -3578,6 +3582,31 @@ drop cascades to view tststats.priv_test_view DROP SCHEMA sts_sch1, sts_sch2 CASCADE; NOTICE: drop cascades to table sts_sch1.tbl DROP USER regress_stats_user1; +-- CREATE STATISTICS checks for the owner +CREATE ROLE regress_relowner; +CREATE ROLE regress_stxowner; +CREATE TABLE stats_ext_tbl (a int, b int); +ALTER TABLE stats_ext_tbl OWNER TO regress_relowner; +CREATE STATISTICS tst ON a, b FROM stats_ext_tbl; +ALTER STATISTICS tst OWNER TO regress_stxowner; +SELECT stxowner::regrole FROM pg_statistic_ext WHERE stxname = 'tst'; + stxowner +------------------ + regress_stxowner +(1 row) + +-- re-creating statistics via ALTER TABLE preserve the statistics owner. +ALTER TABLE stats_ext_tbl ALTER COLUMN a TYPE bigint; +SELECT stxowner::regrole FROM pg_statistic_ext WHERE stxname = 'tst'; + stxowner +------------------ + regress_stxowner +(1 row) + +-- Tidy up +DROP TABLE stats_ext_tbl; +DROP ROLE regress_relowner; +DROP ROLE regress_stxowner; CREATE TABLE grouping_unique (x integer); INSERT INTO grouping_unique (x) SELECT gs FROM generate_series(1,1000) AS gs; ANALYZE grouping_unique; diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql index 3cc6012b822..a9176e74f3c 100644 --- a/src/test/regress/sql/stats_ext.sql +++ b/src/test/regress/sql/stats_ext.sql @@ -41,6 +41,8 @@ CREATE STATISTICS tst ON (x || 'x'), (x || 'x'), (y + 1), (x || 'x'), (x || 'x') CREATE STATISTICS tst ON (x || 'x'), (x || 'x'), y FROM ext_stats_test; CREATE STATISTICS tst (unrecognized) ON x, y FROM ext_stats_test; -- unsupported targets +CREATE STATISTICS tst ON x, y FROM ext_stats_test, ext_stats_test; +CREATE STATISTICS tst ON x, y FROM ext_stats_test, (SELECT * FROM ext_stats_test) AS foo; CREATE STATISTICS tst ON a FROM (VALUES (x)) AS foo; CREATE STATISTICS tst ON a FROM foo NATURAL JOIN bar; CREATE STATISTICS tst ON a FROM (SELECT * FROM ext_stats_test) AS foo; @@ -1830,6 +1832,24 @@ DROP SCHEMA tststats CASCADE; DROP SCHEMA sts_sch1, sts_sch2 CASCADE; DROP USER regress_stats_user1; +-- CREATE STATISTICS checks for the owner +CREATE ROLE regress_relowner; +CREATE ROLE regress_stxowner; +CREATE TABLE stats_ext_tbl (a int, b int); +ALTER TABLE stats_ext_tbl OWNER TO regress_relowner; +CREATE STATISTICS tst ON a, b FROM stats_ext_tbl; +ALTER STATISTICS tst OWNER TO regress_stxowner; +SELECT stxowner::regrole FROM pg_statistic_ext WHERE stxname = 'tst'; + +-- re-creating statistics via ALTER TABLE preserve the statistics owner. +ALTER TABLE stats_ext_tbl ALTER COLUMN a TYPE bigint; +SELECT stxowner::regrole FROM pg_statistic_ext WHERE stxname = 'tst'; + +-- Tidy up +DROP TABLE stats_ext_tbl; +DROP ROLE regress_relowner; +DROP ROLE regress_stxowner; + CREATE TABLE grouping_unique (x integer); INSERT INTO grouping_unique (x) SELECT gs FROM generate_series(1,1000) AS gs; ANALYZE grouping_unique;