diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index ab3d83f29b..0f1547b567 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -52,7 +52,6 @@ #include "catalog/pg_opclass.h" #include "catalog/pg_partitioned_table.h" #include "catalog/pg_statistic.h" -#include "catalog/pg_statistic_ext.h" #include "catalog/pg_subscription_rel.h" #include "catalog/pg_tablespace.h" #include "catalog/pg_type.h" @@ -1615,10 +1614,7 @@ RemoveAttributeById(Oid relid, AttrNumber attnum) heap_close(attr_rel, RowExclusiveLock); if (attnum > 0) - { RemoveStatistics(relid, attnum); - RemoveStatisticsExt(relid, attnum); - } relation_close(rel, NoLock); } @@ -1873,7 +1869,6 @@ heap_drop_with_catalog(Oid relid) * delete statistics */ RemoveStatistics(relid, 0); - RemoveStatisticsExt(relid, 0); /* * delete attribute tuples @@ -2785,75 +2780,6 @@ RemoveStatistics(Oid relid, AttrNumber attnum) } -/* - * RemoveStatisticsExt --- remove entries in pg_statistic_ext for a relation - * - * If attnum is zero, remove all entries for rel; else remove only the - * one(s) involving that column. - */ -void -RemoveStatisticsExt(Oid relid, AttrNumber attnum) -{ - Relation pgstatisticext; - SysScanDesc scan; - ScanKeyData key; - HeapTuple tuple; - - /* - * Scan pg_statistic_ext to delete relevant tuples - */ - pgstatisticext = heap_open(StatisticExtRelationId, RowExclusiveLock); - - ScanKeyInit(&key, - Anum_pg_statistic_ext_stxrelid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(relid)); - - scan = systable_beginscan(pgstatisticext, - StatisticExtRelidIndexId, - true, NULL, 1, &key); - - while (HeapTupleIsValid(tuple = systable_getnext(scan))) - { - bool delete = false; - - if (attnum == 0) - delete = true; - else if (attnum != 0) - { - Form_pg_statistic_ext staForm; - int i; - - /* - * Decode the stxkeys array and delete any stats that involve the - * specified column. - */ - staForm = (Form_pg_statistic_ext) GETSTRUCT(tuple); - for (i = 0; i < staForm->stxkeys.dim1; i++) - { - if (staForm->stxkeys.values[i] == attnum) - { - delete = true; - break; - } - } - } - - if (delete) - { - CatalogTupleDelete(pgstatisticext, &tuple->t_self); - deleteDependencyRecordsFor(StatisticExtRelationId, - HeapTupleGetOid(tuple), - false); - } - } - - systable_endscan(scan); - - heap_close(pgstatisticext, RowExclusiveLock); -} - - /* * RelationTruncateIndexes - truncate all indexes associated * with the heap relation to zero tuples. diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c index 662b4fa15d..db5f4278d5 100644 --- a/src/backend/commands/statscmds.c +++ b/src/backend/commands/statscmds.c @@ -50,11 +50,11 @@ CreateStatistics(CreateStatsStmt *stmt) { int16 attnums[STATS_MAX_DIMENSIONS]; int numcols = 0; - ObjectAddress address = InvalidObjectAddress; char *namestr; NameData stxname; Oid statoid; Oid namespaceId; + Oid stxowner = GetUserId(); HeapTuple htup; Datum values[Natts_pg_statistic_ext]; bool nulls[Natts_pg_statistic_ext]; @@ -63,7 +63,7 @@ CreateStatistics(CreateStatsStmt *stmt) Relation rel = NULL; Oid relid; ObjectAddress parentobject, - childobject; + myself; Datum types[2]; /* one for each possible type of statistics */ int ntypes; ArrayType *stxkind; @@ -140,7 +140,7 @@ CreateStatistics(CreateStatsStmt *stmt) RelationGetRelationName(rel)))); /* You must own the relation to create stats on it */ - if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId())) + if (!pg_class_ownercheck(RelationGetRelid(rel), stxowner)) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, RelationGetRelationName(rel)); } @@ -185,7 +185,7 @@ CreateStatistics(CreateStatsStmt *stmt) attForm = (Form_pg_attribute) GETSTRUCT(atttuple); /* Disallow use of system attributes in extended stats */ - if (attForm->attnum < 0) + if (attForm->attnum <= 0) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("statistics creation on system columns is not supported"))); @@ -205,7 +205,7 @@ CreateStatistics(CreateStatsStmt *stmt) errmsg("cannot have more than %d columns in statistics", STATS_MAX_DIMENSIONS))); - attnums[numcols] = ((Form_pg_attribute) GETSTRUCT(atttuple))->attnum; + attnums[numcols] = attForm->attnum; numcols++; ReleaseSysCache(atttuple); } @@ -231,10 +231,12 @@ CreateStatistics(CreateStatsStmt *stmt) * just check consecutive elements. */ for (i = 1; i < numcols; i++) + { if (attnums[i] == attnums[i - 1]) ereport(ERROR, (errcode(ERRCODE_DUPLICATE_COLUMN), errmsg("duplicate column name in statistics definition"))); + } /* Form an int2vector representation of the sorted column list */ stxkeys = buildint2vector(attnums, numcols); @@ -288,7 +290,7 @@ CreateStatistics(CreateStatsStmt *stmt) values[Anum_pg_statistic_ext_stxrelid - 1] = ObjectIdGetDatum(relid); values[Anum_pg_statistic_ext_stxname - 1] = NameGetDatum(&stxname); values[Anum_pg_statistic_ext_stxnamespace - 1] = ObjectIdGetDatum(namespaceId); - values[Anum_pg_statistic_ext_stxowner - 1] = ObjectIdGetDatum(GetUserId()); + values[Anum_pg_statistic_ext_stxowner - 1] = ObjectIdGetDatum(stxowner); values[Anum_pg_statistic_ext_stxkeys - 1] = PointerGetDatum(stxkeys); values[Anum_pg_statistic_ext_stxkind - 1] = PointerGetDatum(stxkind); @@ -312,29 +314,35 @@ CreateStatistics(CreateStatsStmt *stmt) relation_close(rel, NoLock); /* - * Add a dependency on the table, so that stats get dropped on DROP TABLE. - * - * XXX don't we need dependencies on the specific columns, instead? + * Add an AUTO dependency on each column used in the stats, so that the + * stats object goes away if any or all of them get dropped. */ - ObjectAddressSet(parentobject, RelationRelationId, relid); - ObjectAddressSet(childobject, StatisticExtRelationId, statoid); - recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO); + ObjectAddressSet(myself, StatisticExtRelationId, statoid); + + for (i = 0; i < numcols; i++) + { + ObjectAddressSubSet(parentobject, RelationRelationId, relid, attnums[i]); + recordDependencyOn(&myself, &parentobject, DEPENDENCY_AUTO); + } /* - * Also add dependency on the schema. This is required to ensure that we - * drop the statistics on DROP SCHEMA. This is not handled automatically - * by DROP TABLE because the statistics might be in a different schema - * from the table itself. (This definition is a bit bizarre for the - * single-table case, but it will make more sense if/when we support - * extended stats across multiple tables.) + * Also add dependencies on namespace and owner. These are required + * because the stats object might have a different namespace and/or owner + * than the underlying table(s). */ ObjectAddressSet(parentobject, NamespaceRelationId, namespaceId); - recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO); + recordDependencyOn(&myself, &parentobject, DEPENDENCY_NORMAL); + + recordDependencyOnOwner(StatisticExtRelationId, statoid, stxowner); + + /* + * XXX probably there should be a recordDependencyOnCurrentExtension call + * here too, but we'd have to add support for ALTER EXTENSION ADD/DROP + * STATISTICS, which is more work than it seems worth. + */ /* Return stats object's address */ - ObjectAddressSet(address, StatisticExtRelationId, statoid); - - return address; + return myself; } /* diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index e91479e089..85ee2f6bde 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 201705111 +#define CATALOG_VERSION_NO 201705121 #endif diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h index 473fe177ba..1187797fd9 100644 --- a/src/include/catalog/heap.h +++ b/src/include/catalog/heap.h @@ -119,7 +119,6 @@ extern void RemoveAttrDefault(Oid relid, AttrNumber attnum, DropBehavior behavior, bool complain, bool internal); extern void RemoveAttrDefaultById(Oid attrdefId); extern void RemoveStatistics(Oid relid, AttrNumber attnum); -extern void RemoveStatisticsExt(Oid relid, AttrNumber attnum); extern Form_pg_attribute SystemAttributeDefinition(AttrNumber attno, bool relhasoids); diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out index 4ccdf21a01..13780897c5 100644 --- a/src/test/regress/expected/stats_ext.out +++ b/src/test/regress/expected/stats_ext.out @@ -47,7 +47,7 @@ DROP STATISTICS regress_schema_2.ab1_a_b_stats; -- Ensure statistics are dropped when columns are CREATE STATISTICS ab1_b_c_stats ON b, c FROM ab1; CREATE STATISTICS ab1_a_b_c_stats ON a, b, c FROM ab1; -CREATE STATISTICS ab1_a_b_stats ON a, b FROM ab1; +CREATE STATISTICS ab1_b_a_stats ON b, a FROM ab1; ALTER TABLE ab1 DROP COLUMN a; \d ab1 Table "public.ab1" @@ -58,7 +58,19 @@ ALTER TABLE ab1 DROP COLUMN a; Statistics: "public"."ab1_b_c_stats" (ndistinct, dependencies) ON b, c FROM ab1 +-- Ensure statistics are dropped when table is +SELECT stxname FROM pg_statistic_ext WHERE stxname LIKE 'ab1%'; + stxname +--------------- + ab1_b_c_stats +(1 row) + DROP TABLE ab1; +SELECT stxname FROM pg_statistic_ext WHERE stxname LIKE 'ab1%'; + stxname +--------- +(0 rows) + -- Ensure things work sanely with SET STATISTICS 0 CREATE TABLE ab1 (a INTEGER, b INTEGER); ALTER TABLE ab1 ALTER a SET STATISTICS 0; diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql index 4050f33c08..09b7d8e102 100644 --- a/src/test/regress/sql/stats_ext.sql +++ b/src/test/regress/sql/stats_ext.sql @@ -34,10 +34,13 @@ DROP STATISTICS regress_schema_2.ab1_a_b_stats; -- Ensure statistics are dropped when columns are CREATE STATISTICS ab1_b_c_stats ON b, c FROM ab1; CREATE STATISTICS ab1_a_b_c_stats ON a, b, c FROM ab1; -CREATE STATISTICS ab1_a_b_stats ON a, b FROM ab1; +CREATE STATISTICS ab1_b_a_stats ON b, a FROM ab1; ALTER TABLE ab1 DROP COLUMN a; \d ab1 +-- Ensure statistics are dropped when table is +SELECT stxname FROM pg_statistic_ext WHERE stxname LIKE 'ab1%'; DROP TABLE ab1; +SELECT stxname FROM pg_statistic_ext WHERE stxname LIKE 'ab1%'; -- Ensure things work sanely with SET STATISTICS 0 CREATE TABLE ab1 (a INTEGER, b INTEGER);