From 447dbf7aa7909bca76048042d6734ee8f5249d0f Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 2 May 2018 12:22:48 -0400
Subject: [PATCH] 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 d7305b7ab3..3b4a09b1a7 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 24e74d7a1b..593c219839 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;
}
/*
--
2.40.0