From cec9d03d9165fd62544b63b7f236b9039f1adbe5 Mon Sep 17 00:00:00 2001 From: Tom Lane 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 ae0f304dba..99975817de 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 913829233e..74ab62d91a 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