postgres.git / summary / log / commit / refs

commit    447dbf7aa7909bca76048042d6734ee8f5249d0f
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Wed May 02 16:22:48 2018 +0000
First released in: 11.0, 10.4

    Fix bogus code for extracting extended-statistics data from syscache.
    
    statext_dependencies_load and statext_ndistinct_load were not up to snuff,
    in addition to being randomly different from each other.  In detail:
    
    * Deserialize the fetched bytea value before releasing the syscache
    entry, not after.  This mistake causes visible regression test failures
    when running with -DCATCACHE_FORCE_RELEASE.  Since it's not exposed by
    -DCLOBBER_CACHE_ALWAYS, I think there may be no production hazard here
    at present, but it's at least a latent bug.
    
    * Use DatumGetByteaPP not DatumGetByteaP to save a detoasting cycle
    for short stats values; the deserialize function has to be, and is,
    prepared for short-header values since its other caller uses PP.
    
    * Use a test-and-elog for null stats values in both functions, rather
    than a test-and-elog in one case and an Assert in the other.  Perhaps
    Asserts would be sufficient in both cases, but I don't see a good
    argument for them being different.
    
    * Minor cosmetic changes to make these functions more visibly alike.
    
    Backpatch to v10 where this code came in.
    
    Amit Langote, minor additional hacking by me
    
    Discussion: https://postgr.es/m/1349aabb-3a1f-6675-9fc0-65e2ce7491dd@lab.ntt.co.jp


 src/backend/statistics/dependencies.c | 13 ++++++++++---
 src/backend/statistics/mvdistinct.c   | 11 +++++++----
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/src/backend/statistics/dependencies.c b/src/backend/statistics/dependencies.c
index d7305b7ab39..3b4a09b1a7e 100644
--- a/src/backend/statistics/dependencies.c
+++ b/src/backend/statistics/dependencies.c
@@ -631,20 +631,27 @@ dependency_implies_attribute(MVDependency *dependency, AttrNumber attnum)
 MVDependencies *
 statext_dependencies_load(Oid mvoid)
 {
+	MVDependencies *result;
 	bool		isnull;
 	Datum		deps;
-	HeapTuple	htup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(mvoid));
+	HeapTuple	htup;
 
+	htup = SearchSysCache1(STATEXTOID, 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);
-	Assert(!isnull);
+	if (isnull)
+		elog(ERROR,
+			 "requested statistic kind \"%c\" is not yet built for statistics object %u",
+			 STATS_EXT_DEPENDENCIES, mvoid);
+
+	result = statext_dependencies_deserialize(DatumGetByteaPP(deps));
 
 	ReleaseSysCache(htup);
 
-	return statext_dependencies_deserialize(DatumGetByteaP(deps));
+	return result;
 }
 
 /*
diff --git a/src/backend/statistics/mvdistinct.c b/src/backend/statistics/mvdistinct.c
index 24e74d7a1b2..593c2198396 100644
--- a/src/backend/statistics/mvdistinct.c
+++ b/src/backend/statistics/mvdistinct.c
@@ -126,24 +126,27 @@ statext_ndistinct_build(double totalrows, int numrows, HeapTuple *rows,
 MVNDistinct *
 statext_ndistinct_load(Oid mvoid)
 {
-	bool		isnull = false;
+	MVNDistinct *result;
+	bool		isnull;
 	Datum		ndist;
 	HeapTuple	htup;
 
 	htup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(mvoid));
-	if (!htup)
+	if (!HeapTupleIsValid(htup))
 		elog(ERROR, "cache lookup failed for statistics object %u", mvoid);
 
 	ndist = SysCacheGetAttr(STATEXTOID, htup,
 							Anum_pg_statistic_ext_stxndistinct, &isnull);
 	if (isnull)
 		elog(ERROR,
-			 "requested statistic kind %c is not yet built for statistics object %u",
+			 "requested statistic kind \"%c\" is not yet built for statistics object %u",
 			 STATS_EXT_NDISTINCT, mvoid);
 
+	result = statext_ndistinct_deserialize(DatumGetByteaPP(ndist));
+
 	ReleaseSysCache(htup);
 
-	return statext_ndistinct_deserialize(DatumGetByteaP(ndist));
+	return result;
 }
 
 /*


[parent: bcded2609ade]