Fix dependencies for extended statistics objects.

A stats object ought to have a dependency on each individual column
it reads, not the entire table.  Doing this honestly lets us get rid
of the hard-wired logic in RemoveStatisticsExt, which seems to have
been misguidedly modeled on RemoveStatistics; and it will be far easier
to extend to multiple tables later.

Also, add overlooked dependency on owner, and make the dependency on
schema be NORMAL like every other such dependency.

There remains some unfinished work here, which is to allow statistics
objects to be extension members.  That takes more effort than just
adding the dependency call, though, so I left it out for now.

initdb forced because this changes the set of pg_depend records that
should exist for a statistics object.

Discussion: https://postgr.es/m/22676.1494557205@sss.pgh.pa.us

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index ab3d83f29bba7a81a8e77dfbb6a9a2f9e5904274..0f1547b5671869511a68e50a296919b73e223c32 100644 (file)
--- 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 662b4fa15d2ed1fe190c5ac17421ab5f529718be..db5f4278d559db7efc62f1095b6313f64be9deab 100644 (file)
--- 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);
 
-   /* Return stats object's address */
-   ObjectAddressSet(address, StatisticExtRelationId, statoid);
+   recordDependencyOnOwner(StatisticExtRelationId, statoid, stxowner);
 
-   return address;
+   /*
+    * 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 */
+   return myself;
 }
 
 /*

diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index e91479e0893efcf9f1664cebf88b78e408e9c0dd..85ee2f6bdee985c945230fad8dff086e5a814074 100644 (file)
--- 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 473fe177ba4778998bc76a2a61869c7e316c74b7..1187797fd9ef3a90e951432ee1475053b9496a46 100644 (file)
--- 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 4ccdf21a015f14419b28c138bb357e7b84c63a16..13780897c5a65e29a77f873fba2d30e220898a6b 100644 (file)
--- 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 4050f33c088baefa4802c1cb4c48cf7931f77e0f..09b7d8e102b0fd14f4e9351993c537ccaf2556fe 100644 (file)
--- 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);