Rework the pg_statistic_ext catalog

Since extended statistic got introduced in PostgreSQL 10, there was a
single catalog pg_statistic_ext storing both the definitions and built
statistic.  That's however problematic when a user is supposed to have
access only to the definitions, but not to user data.

Consider for example pg_dump on a database with RLS enabled - if the
pg_statistic_ext catalog respects RLS (which it should, if it contains
user data), pg_dump would not see any records and the result would not
define any extended statistics.  That would be a surprising behavior.

Until now this was not a pressing issue, because the existing types of
extended statistic (functional dependencies and ndistinct coefficients)
do not include any user data directly.  This changed with introduction
of MCV lists, which do include most common combinations of values.

The easiest way to fix this is to split the pg_statistic_ext catalog
into two - one for definitions, one for the built statistic values.
The new catalog is called pg_statistic_ext_data, and we're maintaining
a 1:1 relationship with the old catalog - either there are matching
records in both catalogs, or neither of them.

Bumped CATVERSION due to changing system catalog definitions.

Author: Dean Rasheed, with improvements by me
Reviewed-by: Dean Rasheed, John Naylor
Discussion: https://postgr.es/m/CAEZATCUhT9rt7Ui%3DVdx4N%3D%3DVV5XOK5dsXfnGgVOz_JhAicB%3DZA%40mail.gmail.com

Author: tvondra

doc/src/sgml/catalogs.sgml

@@ -297,7 +297,12 @@
 
      <row>
       <entry><link linkend="catalog-pg-statistic-ext"><structname>pg_statistic_ext</structname></link></entry>
-      <entry>extended planner statistics</entry>
+      <entry>extended planner statistics (definition)</entry>
+     </row>
+
+     <row>
+      <entry><link linkend="catalog-pg-statistic-ext-data"><structname>pg_statistic_ext_data</structname></link></entry>
+      <entry>extended planner statistics (built statistics)</entry>
      </row>
 
      <row>
@@ -6506,7 +6511,7 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
 
   <para>
    The catalog <structname>pg_statistic_ext</structname>
-   holds extended planner statistics.
+   holds definitions of extended planner statistics.
    Each row in this catalog corresponds to a <firstterm>statistics object</firstterm>
    created with <xref linkend="sql-createstatistics"/>.
   </para>
@@ -6581,8 +6586,57 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
       </entry>
      </row>
 
+    </tbody>
+   </tgroup>
+  </table>
+
+  <para>
+   The <structfield>stxkind</structfield> field is filled at creation of the
+   statistics object, indicating which statistic type(s) are desired. The
+   statistics (once computed by <command>ANALYZE</command>) are stored in
+   <link linkend="catalog-pg-statistic-ext-data"><structname>pg_statistic_ext_data</structname></link>
+   catalog.
+  </para>
+ </sect1>
+
+ <sect1 id="catalog-pg-statistic-ext-data">
+  <title><structname>pg_statistic_ext_data</structname></title>
+
+  <indexterm zone="catalog-pg-statistic-ext">
+   <primary>pg_statistic_ext_data</primary>
+  </indexterm>
+
+  <para>
+   The catalog <structname>pg_statistic_ext_data</structname>
+   holds data for extended planner statistics defined in <structname>pg_statistic_ext</structname>.
+   Each row in this catalog corresponds to a <firstterm>statistics object</firstterm>
+   created with <xref linkend="sql-createstatistics"/>.
+  </para>
+
+  <table>
+   <title><structname>pg_statistic_ext_data</structname> Columns</title>
+
+   <tgroup cols="4">
+    <thead>
+     <row>
+      <entry>Name</entry>
+      <entry>Type</entry>
+      <entry>References</entry>
+      <entry>Description</entry>
+     </row>
+    </thead>
+
+    <tbody>
+
+     <row>
+      <entry><structfield>stxoid</structfield></entry>
+      <entry><type>oid</type></entry>
+      <entry><literal><link linkend="catalog-pg-statistic-ext"><structname>pg_statistic_ext</structname></link>.oid</literal></entry>
+      <entry>Extended statistic containing the definition for this data.</entry>
+     </row>
+
      <row>
-      <entry><structfield>stxndistinct</structfield></entry>
+      <entry><structfield>stxdndistinct</structfield></entry>
       <entry><type>pg_ndistinct</type></entry>
       <entry></entry>
       <entry>
@@ -6591,7 +6645,7 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
      </row>
 
      <row>
-      <entry><structfield>stxdependencies</structfield></entry>
+      <entry><structfield>stxddependencies</structfield></entry>
       <entry><type>pg_dependencies</type></entry>
       <entry></entry>
       <entry>
@@ -6601,7 +6655,7 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
      </row>
 
      <row>
-      <entry><structfield>stxmcv</structfield></entry>
+      <entry><structfield>stxdmcv</structfield></entry>
       <entry><type>pg_mcv_list</type></entry>
       <entry></entry>
       <entry>
@@ -6614,12 +6668,6 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
    </tgroup>
   </table>
 
-  <para>
-   The <structfield>stxkind</structfield> field is filled at creation of the
-   statistics object, indicating which statistic type(s) are desired.
-   The fields after it are initially NULL and are filled only when the
-   corresponding statistic has been computed by <command>ANALYZE</command>.
-  </para>
  </sect1>
 
  <sect1 id="catalog-pg-subscription">

doc/src/sgml/func.sgml

@@ -22427,12 +22427,12 @@ CREATE EVENT TRIGGER test_table_rewrite_oid
     The <function>pg_mcv_list_items</function> function can be used like this:
 
 <programlisting>
-SELECT m.* FROM pg_statistic_ext,
-                pg_mcv_list_items(stxmcv) m WHERE stxname = 'stts';
+SELECT m.* FROM pg_statistic_ext join pg_statistic_ext_data on (oid = stxoid),
+                pg_mcv_list_items(stxdmcv) m WHERE stxname = 'stts';
 </programlisting>
 
      Values of the <type>pg_mcv_list</type> can be obtained only from the
-     <literal>pg_statistic_ext.stxmcv</literal> column.
+     <literal>pg_statistic_ext_data.stxdmcv</literal> column.
    </para>
   </sect2>
 

doc/src/sgml/perform.sgml

@@ -1076,6 +1076,10 @@ WHERE tablename = 'road';
     <primary>pg_statistic_ext</primary>
    </indexterm>
 
+   <indexterm>
+    <primary>pg_statistic_ext_data</primary>
+   </indexterm>
+
    <para>
     It is common to see slow queries running bad execution plans because
     multiple columns used in the query clauses are correlated.
@@ -1104,7 +1108,7 @@ WHERE tablename = 'road';
     interest in the statistics.  Actual data collection is performed
     by <command>ANALYZE</command> (either a manual command, or background
     auto-analyze).  The collected values can be examined in the
-    <link linkend="catalog-pg-statistic-ext"><structname>pg_statistic_ext</structname></link>
+    <link linkend="catalog-pg-statistic-ext-data"><structname>pg_statistic_ext_data</structname></link>
     catalog.
    </para>
 
@@ -1172,10 +1176,10 @@ CREATE STATISTICS stts (dependencies) ON zip, city FROM zipcodes;
 
 ANALYZE zipcodes;
 
-SELECT stxname, stxkeys, stxdependencies
-  FROM pg_statistic_ext
+SELECT stxname, stxkeys, stxddependencies
+  FROM pg_statistic_ext join pg_statistic_ext_data on (oid = stxoid)
   WHERE stxname = 'stts';
- stxname | stxkeys |             stxdependencies               
+ stxname | stxkeys |             stxddependencies             
 ---------+---------+------------------------------------------
  stts    | 1 5     | {"1 => 5": 1.000000, "5 => 1": 0.423130}
 (1 row)
@@ -1262,8 +1266,8 @@ CREATE STATISTICS stts2 (ndistinct) ON zip, state, city FROM zipcodes;
 
 ANALYZE zipcodes;
 
-SELECT stxkeys AS k, stxndistinct AS nd
-  FROM pg_statistic_ext
+SELECT stxkeys AS k, stxdndistinct AS nd
+  FROM pg_statistic_ext join pg_statistic_ext_data on (oid = stxoid)
   WHERE stxname = 'stts2';
 -[ RECORD 1 ]--------------------------------------------------------
 k  | 1 2 5
@@ -1317,8 +1321,8 @@ CREATE STATISTICS stts3 (mcv) ON state, city FROM zipcodes;
 
 ANALYZE zipcodes;
 
-SELECT m.* FROM pg_statistic_ext,
-                pg_mcv_list_items(stxmcv) m WHERE stxname = 'stts3';
+SELECT m.* FROM pg_statistic_ext join pg_statistic_ext_data on (oid = stxoid),
+                pg_mcv_list_items(stxdmcv) m WHERE stxname = 'stts3';
 
  index |         values         | nulls | frequency | base_frequency 
 -------+------------------------+-------+-----------+----------------

doc/src/sgml/planstats.sgml

@@ -635,8 +635,8 @@ EXPLAIN (ANALYZE, TIMING OFF) SELECT * FROM t WHERE a = 1 AND b = 1;
     <function>pg_mcv_list_items</function> set-returning function.
 
 <programlisting>
-SELECT m.* FROM pg_statistic_ext,
-                pg_mcv_list_items(stxmcv) m WHERE stxname = 'stts2';
+SELECT m.* FROM pg_statistic_ext join pg_statistic_ext_data on (oid = stxoid),
+                pg_mcv_list_items(stxdmcv) m WHERE stxname = 'stts2';
  index |  values  | nulls | frequency | base_frequency 
 -------+----------+-------+-----------+----------------
      0 | {0, 0}   | {f,f} |      0.01 |         0.0001

src/backend/catalog/Makefile

@@ -34,7 +34,7 @@ CATALOG_HEADERS := \
 	pg_attrdef.h pg_constraint.h pg_inherits.h pg_index.h pg_operator.h \
 	pg_opfamily.h pg_opclass.h pg_am.h pg_amop.h pg_amproc.h \
 	pg_language.h pg_largeobject_metadata.h pg_largeobject.h pg_aggregate.h \
-	pg_statistic_ext.h \
+	pg_statistic_ext.h pg_statistic_ext_data.h \
 	pg_statistic.h pg_rewrite.h pg_trigger.h pg_event_trigger.h pg_description.h \
 	pg_cast.h pg_enum.h pg_namespace.h pg_conversion.h pg_depend.h \
 	pg_database.h pg_db_role_setting.h pg_tablespace.h pg_pltemplate.h \

src/backend/commands/statscmds.c

@@ -23,6 +23,7 @@
 #include "catalog/namespace.h"
 #include "catalog/pg_namespace.h"
 #include "catalog/pg_statistic_ext.h"
+#include "catalog/pg_statistic_ext_data.h"
 #include "commands/comment.h"
 #include "commands/defrem.h"
 #include "miscadmin.h"
@@ -67,8 +68,11 @@ CreateStatistics(CreateStatsStmt *stmt)
 	HeapTuple	htup;
 	Datum		values[Natts_pg_statistic_ext];
 	bool		nulls[Natts_pg_statistic_ext];
+	Datum		datavalues[Natts_pg_statistic_ext_data];
+	bool		datanulls[Natts_pg_statistic_ext_data];
 	int2vector *stxkeys;
 	Relation	statrel;
+	Relation	datarel;
 	Relation	rel = NULL;
 	Oid			relid;
 	ObjectAddress parentobject,
@@ -336,18 +340,36 @@ CreateStatistics(CreateStatsStmt *stmt)
 	values[Anum_pg_statistic_ext_stxkeys - 1] = PointerGetDatum(stxkeys);
 	values[Anum_pg_statistic_ext_stxkind - 1] = PointerGetDatum(stxkind);
 
-	/* no statistics built yet */
-	nulls[Anum_pg_statistic_ext_stxndistinct - 1] = true;
-	nulls[Anum_pg_statistic_ext_stxdependencies - 1] = true;
-	nulls[Anum_pg_statistic_ext_stxmcv - 1] = true;
-
 	/* insert it into pg_statistic_ext */
 	htup = heap_form_tuple(statrel->rd_att, values, nulls);
 	CatalogTupleInsert(statrel, htup);
 	heap_freetuple(htup);
 
 	relation_close(statrel, RowExclusiveLock);
 
+	/*
+	 * Also build the pg_statistic_ext_data tuple, to hold the actual
+	 * statistics data.
+	 */
+	datarel = table_open(StatisticExtDataRelationId, RowExclusiveLock);
+
+	memset(datavalues, 0, sizeof(datavalues));
+	memset(datanulls, false, sizeof(datanulls));
+
+	datavalues[Anum_pg_statistic_ext_data_stxoid - 1] = ObjectIdGetDatum(statoid);
+
+	/* no statistics built yet */
+	datanulls[Anum_pg_statistic_ext_data_stxdndistinct - 1] = true;
+	datanulls[Anum_pg_statistic_ext_data_stxddependencies - 1] = true;
+	datanulls[Anum_pg_statistic_ext_data_stxdmcv - 1] = true;
+
+	/* insert it into pg_statistic_ext_data */
+	htup = heap_form_tuple(datarel->rd_att, datavalues, datanulls);
+	CatalogTupleInsert(datarel, htup);
+	heap_freetuple(htup);
+
+	relation_close(datarel, RowExclusiveLock);
+
 	/*
 	 * Invalidate relcache so that others see the new statistics object.
 	 */
@@ -403,6 +425,23 @@ RemoveStatisticsById(Oid statsOid)
 	Form_pg_statistic_ext statext;
 	Oid			relid;
 
+	/*
+	 * First delete the pg_statistic_ext_data tuple holding the actual
+	 * statistical data.
+	 */
+	relation = table_open(StatisticExtDataRelationId, RowExclusiveLock);
+
+	tup = SearchSysCache1(STATEXTDATASTXOID, ObjectIdGetDatum(statsOid));
+
+	if (!HeapTupleIsValid(tup)) /* should not happen */
+		elog(ERROR, "cache lookup failed for statistics data %u", statsOid);
+
+	CatalogTupleDelete(relation, &tup->t_self);
+
+	ReleaseSysCache(tup);
+
+	table_close(relation, RowExclusiveLock);
+
 	/*
 	 * Delete the pg_statistic_ext tuple.  Also send out a cache inval on the
 	 * associated table, so that dependent plans will be rebuilt.
@@ -431,8 +470,8 @@ RemoveStatisticsById(Oid statsOid)
  *
  * This could throw an error if the type change can't be supported.
  * If it can be supported, but the stats must be recomputed, a likely choice
- * would be to set the relevant column(s) of the pg_statistic_ext tuple to
- * null until the next ANALYZE.  (Note that the type change hasn't actually
+ * would be to set the relevant column(s) of the pg_statistic_ext_data tuple
+ * to null until the next ANALYZE.  (Note that the type change hasn't actually
  * happened yet, so one option that's *not* on the table is to recompute
  * immediately.)
  *
@@ -456,11 +495,11 @@ UpdateStatisticsForTypeChange(Oid statsOid, Oid relationOid, int attnum,
 
 	Relation	rel;
 
-	Datum		values[Natts_pg_statistic_ext];
-	bool		nulls[Natts_pg_statistic_ext];
-	bool		replaces[Natts_pg_statistic_ext];
+	Datum		values[Natts_pg_statistic_ext_data];
+	bool		nulls[Natts_pg_statistic_ext_data];
+	bool		replaces[Natts_pg_statistic_ext_data];
 
-	oldtup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(statsOid));
+	oldtup = SearchSysCache1(STATEXTDATASTXOID, ObjectIdGetDatum(statsOid));
 	if (!HeapTupleIsValid(oldtup))
 		elog(ERROR, "cache lookup failed for statistics object %u", statsOid);
 
@@ -479,14 +518,14 @@ UpdateStatisticsForTypeChange(Oid statsOid, Oid relationOid, int attnum,
 	 * OK, we need to reset some statistics. So let's build the new tuple,
 	 * replacing the affected statistics types with NULL.
 	 */
-	memset(nulls, 0, Natts_pg_statistic_ext * sizeof(bool));
-	memset(replaces, 0, Natts_pg_statistic_ext * sizeof(bool));
-	memset(values, 0, Natts_pg_statistic_ext * sizeof(Datum));
+	memset(nulls, 0, Natts_pg_statistic_ext_data * sizeof(bool));
+	memset(replaces, 0, Natts_pg_statistic_ext_data * sizeof(bool));
+	memset(values, 0, Natts_pg_statistic_ext_data * sizeof(Datum));
 
-	replaces[Anum_pg_statistic_ext_stxmcv - 1] = true;
-	nulls[Anum_pg_statistic_ext_stxmcv - 1] = true;
+	replaces[Anum_pg_statistic_ext_data_stxdmcv - 1] = true;
+	nulls[Anum_pg_statistic_ext_data_stxdmcv - 1] = true;
 
-	rel = heap_open(StatisticExtRelationId, RowExclusiveLock);
+	rel = heap_open(StatisticExtDataRelationId, RowExclusiveLock);
 
 	/* replace the old tuple */
 	stup = heap_modify_tuple(oldtup,

src/backend/optimizer/util/plancat.c

@@ -1308,6 +1308,7 @@ get_relation_statistics(RelOptInfo *rel, Relation relation)
 		Oid			statOid = lfirst_oid(l);
 		Form_pg_statistic_ext staForm;
 		HeapTuple	htup;
+		HeapTuple	dtup;
 		Bitmapset  *keys = NULL;
 		int			i;
 
@@ -1316,6 +1317,10 @@ get_relation_statistics(RelOptInfo *rel, Relation relation)
 			elog(ERROR, "cache lookup failed for statistics object %u", statOid);
 		staForm = (Form_pg_statistic_ext) GETSTRUCT(htup);
 
+		dtup = SearchSysCache1(STATEXTDATASTXOID, ObjectIdGetDatum(statOid));
+		if (!HeapTupleIsValid(dtup))
+			elog(ERROR, "cache lookup failed for statistics object %u", statOid);
+
 		/*
 		 * First, build the array of columns covered.  This is ultimately
 		 * wasted if no stats within the object have actually been built, but
@@ -1325,7 +1330,7 @@ get_relation_statistics(RelOptInfo *rel, Relation relation)
 			keys = bms_add_member(keys, staForm->stxkeys.values[i]);
 
 		/* add one StatisticExtInfo for each kind built */
-		if (statext_is_kind_built(htup, STATS_EXT_NDISTINCT))
+		if (statext_is_kind_built(dtup, STATS_EXT_NDISTINCT))
 		{
 			StatisticExtInfo *info = makeNode(StatisticExtInfo);
 
@@ -1337,7 +1342,7 @@ get_relation_statistics(RelOptInfo *rel, Relation relation)
 			stainfos = lcons(info, stainfos);
 		}
 
-		if (statext_is_kind_built(htup, STATS_EXT_DEPENDENCIES))
+		if (statext_is_kind_built(dtup, STATS_EXT_DEPENDENCIES))
 		{
 			StatisticExtInfo *info = makeNode(StatisticExtInfo);
 
@@ -1349,7 +1354,7 @@ get_relation_statistics(RelOptInfo *rel, Relation relation)
 			stainfos = lcons(info, stainfos);
 		}
 
-		if (statext_is_kind_built(htup, STATS_EXT_MCV))
+		if (statext_is_kind_built(dtup, STATS_EXT_MCV))
 		{
 			StatisticExtInfo *info = makeNode(StatisticExtInfo);
 
@@ -1362,6 +1367,7 @@ get_relation_statistics(RelOptInfo *rel, Relation relation)
 		}
 
 		ReleaseSysCache(htup);
+		ReleaseSysCache(dtup);
 		bms_free(keys);
 	}