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

diff --git a/src/backend/statistics/dependencies.c b/src/backend/statistics/dependencies.c
index d7305b7ab393c7581d9345127617451dbc8bd83c..3b4a09b1a7e729970f80b3551ecf3c75e8ad5ed7 100644 (file)
--- 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 24e74d7a1b22a2cc0e3e550a4840ec992019d4ae..593c21983960501f8a1b93fbd97b76dd55f9a91a 100644 (file)
--- 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;
 }
 
 /*