diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c index 763e662eab6..ead7a3ac5ba 100644 --- a/src/backend/commands/statscmds.c +++ b/src/backend/commands/statscmds.c @@ -60,9 +60,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; @@ -71,7 +77,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]; @@ -80,7 +86,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 */ @@ -98,24 +104,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 @@ -124,7 +123,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 && @@ -138,13 +137,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 (!pg_class_ownercheck(RelationGetRelid(rel), stxowner)) + if (check_rights && + !pg_class_ownercheck(RelationGetRelid(rel), stxowner)) aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(rel->rd_rel->relkind), RelationGetRelationName(rel)); @@ -157,7 +154,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 f04ab5fc1e6..a14f72f1c4d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -195,6 +195,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 */ @@ -571,7 +572,7 @@ static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab); static void RememberStatisticsForRebuilding(Oid indoid, 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, int pass, @@ -8827,7 +8828,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; } @@ -13423,11 +13428,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); } } @@ -13445,6 +13464,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 @@ -13517,7 +13537,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); } @@ -13536,7 +13556,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); @@ -13545,8 +13565,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; @@ -13566,7 +13587,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); @@ -13630,8 +13651,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; @@ -13671,10 +13693,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 5f95515324e..0fc65420753 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1868,7 +1868,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, @@ -1891,7 +1905,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 2d68fc338f0..b5cbb3e560e 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -80,7 +80,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 a77b5ba1b14..4b09aaa8e07 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2991,6 +2991,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 228f9f25cbb..5c04b6bcf4b 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; @@ -3424,3 +3428,28 @@ drop cascades to view tststats.priv_test_view DROP SCHEMA sts_sch1 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 dc1786af5c5..1894c777aaa 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; @@ -1759,3 +1761,21 @@ DROP TABLE stats_ext_tbl; DROP SCHEMA tststats CASCADE; DROP SCHEMA sts_sch1 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;