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

5 files changed, 52 insertions, 39 deletions

diff --git a/src/backend/statistics/README.mcv b/src/backend/statistics/README.mcv
index c18878f5d2b..8455b0d13f6 100644
--- a/src/backend/statistics/README.mcv
+++ b/src/backend/statistics/README.mcv
@@ -86,11 +86,14 @@ So instead the MCV lists are stored in a custom data type (pg_mcv_list),
 which however makes it more difficult to inspect the contents. To make that
 easier, there's a SRF returning detailed information about the MCV lists.
 
-    SELECT m.* FROM pg_statistic_ext,
-                    pg_mcv_list_items(stxmcv) m WHERE stxname = 'stts2';
+    SELECT m.* FROM pg_statistic_ext s,
+                    pg_statistic_ext_data d,
+                    pg_mcv_list_items(stxdmcv) m
+              WHERE s.stxname = 'stts2'
+                AND d.stxoid = s.oid;
 
 It accepts one parameter - a pg_mcv_list value (which can only be obtained
-from pg_statistic_ext catalog, to defend against malicious input), and
+from pg_statistic_ext_data catalog, to defend against malicious input), and
 returns these columns:
 
     - item index (0, ..., (nitems-1))
diff --git a/src/backend/statistics/dependencies.c b/src/backend/statistics/dependencies.c
index cd318faf3b9..66c38ce2bc9 100644
--- a/src/backend/statistics/dependencies.c
+++ b/src/backend/statistics/dependencies.c
@@ -17,6 +17,7 @@
 #include "access/sysattr.h"
 #include "catalog/pg_operator.h"
 #include "catalog/pg_statistic_ext.h"
+#include "catalog/pg_statistic_ext_data.h"
 #include "lib/stringinfo.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/clauses.h"
@@ -637,12 +638,12 @@ statext_dependencies_load(Oid mvoid)
 	Datum		deps;
 	HeapTuple	htup;
 
-	htup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(mvoid));
+	htup = SearchSysCache1(STATEXTDATASTXOID, ObjectIdGetDatum(mvoid));
 	if (!HeapTupleIsValid(htup))
 		elog(ERROR, "cache lookup failed for statistics object %u", mvoid);
 
-	deps = SysCacheGetAttr(STATEXTOID, htup,
-						   Anum_pg_statistic_ext_stxdependencies, &isnull);
+	deps = SysCacheGetAttr(STATEXTDATASTXOID, htup,
+						   Anum_pg_statistic_ext_data_stxddependencies, &isnull);
 	if (isnull)
 		elog(ERROR,
 			 "requested statistic kind \"%c\" is not yet built for statistics object %u",
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index ab187915c1d..96db32f0a0a 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -23,6 +23,7 @@
 #include "catalog/indexing.h"
 #include "catalog/pg_collation.h"
 #include "catalog/pg_statistic_ext.h"
+#include "catalog/pg_statistic_ext_data.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/clauses.h"
 #include "optimizer/optimizer.h"
@@ -65,9 +66,9 @@ typedef struct StatExtEntry
 static List *fetch_statentries_for_relation(Relation pg_statext, Oid relid);
 static VacAttrStats **lookup_var_attr_stats(Relation rel, Bitmapset *attrs,
 											int nvacatts, VacAttrStats **vacatts);
-static void statext_store(Relation pg_stext, Oid relid,
+static void statext_store(Oid relid,
 						  MVNDistinct *ndistinct, MVDependencies *dependencies,
-						  MCVList *mcvlist, VacAttrStats **stats);
+						  MCVList *mcv, VacAttrStats **stats);
 
 
 /*
@@ -145,7 +146,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
 		}
 
 		/* store the statistics in the catalog */
-		statext_store(pg_stext, stat->statOid, ndistinct, dependencies, mcv, stats);
+		statext_store(stat->statOid, ndistinct, dependencies, mcv, stats);
 	}
 
 	table_close(pg_stext, RowExclusiveLock);
@@ -156,7 +157,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
 
 /*
  * statext_is_kind_built
- *		Is this stat kind built in the given pg_statistic_ext tuple?
+ *		Is this stat kind built in the given pg_statistic_ext_data tuple?
  */
 bool
 statext_is_kind_built(HeapTuple htup, char type)
@@ -166,15 +167,15 @@ statext_is_kind_built(HeapTuple htup, char type)
 	switch (type)
 	{
 		case STATS_EXT_NDISTINCT:
-			attnum = Anum_pg_statistic_ext_stxndistinct;
+			attnum = Anum_pg_statistic_ext_data_stxdndistinct;
 			break;
 
 		case STATS_EXT_DEPENDENCIES:
-			attnum = Anum_pg_statistic_ext_stxdependencies;
+			attnum = Anum_pg_statistic_ext_data_stxddependencies;
 			break;
 
 		case STATS_EXT_MCV:
-			attnum = Anum_pg_statistic_ext_stxmcv;
+			attnum = Anum_pg_statistic_ext_data_stxdmcv;
 			break;
 
 		default:
@@ -312,70 +313,76 @@ lookup_var_attr_stats(Relation rel, Bitmapset *attrs,
 
 /*
  * statext_store
- *	Serializes the statistics and stores them into the pg_statistic_ext tuple.
+ *	Serializes the statistics and stores them into the pg_statistic_ext_data
+ *	tuple.
  */
 static void
-statext_store(Relation pg_stext, Oid statOid,
+statext_store(Oid statOid,
 			  MVNDistinct *ndistinct, MVDependencies *dependencies,
 			  MCVList *mcv, VacAttrStats **stats)
 {
 	HeapTuple	stup,
 				oldtup;
-	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];
+	Relation	pg_stextdata;
 
 	memset(nulls, true, sizeof(nulls));
 	memset(replaces, false, sizeof(replaces));
 	memset(values, 0, sizeof(values));
 
 	/*
-	 * Construct a new pg_statistic_ext tuple, replacing the calculated stats.
+	 * Construct a new pg_statistic_ext_data tuple, replacing the calculated
+	 * stats.
 	 */
 	if (ndistinct != NULL)
 	{
 		bytea	   *data = statext_ndistinct_serialize(ndistinct);
 
-		nulls[Anum_pg_statistic_ext_stxndistinct - 1] = (data == NULL);
-		values[Anum_pg_statistic_ext_stxndistinct - 1] = PointerGetDatum(data);
+		nulls[Anum_pg_statistic_ext_data_stxdndistinct - 1] = (data == NULL);
+		values[Anum_pg_statistic_ext_data_stxdndistinct - 1] = PointerGetDatum(data);
 	}
 
 	if (dependencies != NULL)
 	{
 		bytea	   *data = statext_dependencies_serialize(dependencies);
 
-		nulls[Anum_pg_statistic_ext_stxdependencies - 1] = (data == NULL);
-		values[Anum_pg_statistic_ext_stxdependencies - 1] = PointerGetDatum(data);
+		nulls[Anum_pg_statistic_ext_data_stxddependencies - 1] = (data == NULL);
+		values[Anum_pg_statistic_ext_data_stxddependencies - 1] = PointerGetDatum(data);
 	}
-
 	if (mcv != NULL)
 	{
 		bytea	   *data = statext_mcv_serialize(mcv, stats);
 
-		nulls[Anum_pg_statistic_ext_stxmcv - 1] = (data == NULL);
-		values[Anum_pg_statistic_ext_stxmcv - 1] = PointerGetDatum(data);
+		nulls[Anum_pg_statistic_ext_data_stxdmcv - 1] = (data == NULL);
+		values[Anum_pg_statistic_ext_data_stxdmcv - 1] = PointerGetDatum(data);
 	}
 
 	/* always replace the value (either by bytea or NULL) */
-	replaces[Anum_pg_statistic_ext_stxndistinct - 1] = true;
-	replaces[Anum_pg_statistic_ext_stxdependencies - 1] = true;
-	replaces[Anum_pg_statistic_ext_stxmcv - 1] = true;
+	replaces[Anum_pg_statistic_ext_data_stxdndistinct - 1] = true;
+	replaces[Anum_pg_statistic_ext_data_stxddependencies - 1] = true;
+	replaces[Anum_pg_statistic_ext_data_stxdmcv - 1] = true;
 
-	/* there should already be a pg_statistic_ext tuple */
-	oldtup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(statOid));
+	/* there should already be a pg_statistic_ext_data tuple */
+	oldtup = SearchSysCache1(STATEXTDATASTXOID, ObjectIdGetDatum(statOid));
 	if (!HeapTupleIsValid(oldtup))
 		elog(ERROR, "cache lookup failed for statistics object %u", statOid);
 
 	/* replace it */
+	pg_stextdata = table_open(StatisticExtDataRelationId, RowExclusiveLock);
+
 	stup = heap_modify_tuple(oldtup,
-							 RelationGetDescr(pg_stext),
+							 RelationGetDescr(pg_stextdata),
 							 values,
 							 nulls,
 							 replaces);
 	ReleaseSysCache(oldtup);
-	CatalogTupleUpdate(pg_stext, &stup->t_self, stup);
+	CatalogTupleUpdate(pg_stextdata, &stup->t_self, stup);
 
 	heap_freetuple(stup);
+
+	table_close(pg_stextdata, RowExclusiveLock);
 }
 
 /* initialize multi-dimensional sort */
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index d1f0fd55e83..2feb17ed447 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -19,6 +19,7 @@
 #include "access/htup_details.h"
 #include "catalog/pg_collation.h"
 #include "catalog/pg_statistic_ext.h"
+#include "catalog/pg_statistic_ext_data.h"
 #include "fmgr.h"
 #include "funcapi.h"
 #include "nodes/nodeFuncs.h"
@@ -429,13 +430,13 @@ statext_mcv_load(Oid mvoid)
 	MCVList    *result;
 	bool		isnull;
 	Datum		mcvlist;
-	HeapTuple	htup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(mvoid));
+	HeapTuple	htup = SearchSysCache1(STATEXTDATASTXOID, ObjectIdGetDatum(mvoid));
 
 	if (!HeapTupleIsValid(htup))
 		elog(ERROR, "cache lookup failed for statistics object %u", mvoid);
 
-	mcvlist = SysCacheGetAttr(STATEXTOID, htup,
-							  Anum_pg_statistic_ext_stxmcv, &isnull);
+	mcvlist = SysCacheGetAttr(STATEXTDATASTXOID, htup,
+							  Anum_pg_statistic_ext_data_stxdmcv, &isnull);
 
 	if (isnull)
 		elog(ERROR,
diff --git a/src/backend/statistics/mvdistinct.c b/src/backend/statistics/mvdistinct.c
index 7432a6a3969..9ebf183d909 100644
--- a/src/backend/statistics/mvdistinct.c
+++ b/src/backend/statistics/mvdistinct.c
@@ -27,6 +27,7 @@
 
 #include "access/htup_details.h"
 #include "catalog/pg_statistic_ext.h"
+#include "catalog/pg_statistic_ext_data.h"
 #include "utils/fmgrprotos.h"
 #include "utils/lsyscache.h"
 #include "lib/stringinfo.h"
@@ -145,12 +146,12 @@ statext_ndistinct_load(Oid mvoid)
 	Datum		ndist;
 	HeapTuple	htup;
 
-	htup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(mvoid));
+	htup = SearchSysCache1(STATEXTDATASTXOID, ObjectIdGetDatum(mvoid));
 	if (!HeapTupleIsValid(htup))
 		elog(ERROR, "cache lookup failed for statistics object %u", mvoid);
 
-	ndist = SysCacheGetAttr(STATEXTOID, htup,
-							Anum_pg_statistic_ext_stxndistinct, &isnull);
+	ndist = SysCacheGetAttr(STATEXTDATASTXOID, htup,
+							Anum_pg_statistic_ext_data_stxdndistinct, &isnull);
 	if (isnull)
 		elog(ERROR,
 			 "requested statistic kind \"%c\" is not yet built for statistics object %u",