diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c index 6010abeb20d..5f920e3a55c 100644 --- a/src/backend/commands/statscmds.c +++ b/src/backend/commands/statscmds.c @@ -57,9 +57,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; @@ -68,7 +74,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]; @@ -77,7 +83,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 */ @@ -95,24 +101,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 @@ -121,7 +120,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 && @@ -135,13 +134,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)); @@ -154,7 +151,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 2fa5d92da75..2a142b4ebfd 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -205,6 +205,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 */ @@ -588,7 +589,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, @@ -9244,7 +9245,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; } @@ -13816,11 +13821,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); } } @@ -13838,6 +13857,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 @@ -13910,7 +13930,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); } @@ -13929,7 +13949,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); @@ -13938,8 +13958,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; @@ -13959,7 +13980,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); @@ -14023,8 +14044,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; @@ -14064,10 +14086,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 2be41afc2d3..4c149f8a10c 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1875,7 +1875,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, @@ -1898,7 +1912,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 74ca50ac8e7..762f9bc1f47 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -81,7 +81,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 67c90a2bd32..a6599b95445 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3391,6 +3391,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 ec92d784680..25e0cc18b69 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; @@ -3459,3 +3463,28 @@ 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; diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql index 0c20415723b..9ba21c5fb3b 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; @@ -1783,3 +1785,21 @@ DROP TABLE stats_ext_tbl; 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;