Commit 447dbf7a authored by Tom Lane's avatar Tom Lane

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
parent bcded260
...@@ -631,20 +631,27 @@ dependency_implies_attribute(MVDependency *dependency, AttrNumber attnum) ...@@ -631,20 +631,27 @@ dependency_implies_attribute(MVDependency *dependency, AttrNumber attnum)
MVDependencies * MVDependencies *
statext_dependencies_load(Oid mvoid) statext_dependencies_load(Oid mvoid)
{ {
MVDependencies *result;
bool isnull; bool isnull;
Datum deps; Datum deps;
HeapTuple htup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(mvoid)); HeapTuple htup;
htup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(mvoid));
if (!HeapTupleIsValid(htup)) if (!HeapTupleIsValid(htup))
elog(ERROR, "cache lookup failed for statistics object %u", mvoid); elog(ERROR, "cache lookup failed for statistics object %u", mvoid);
deps = SysCacheGetAttr(STATEXTOID, htup, deps = SysCacheGetAttr(STATEXTOID, htup,
Anum_pg_statistic_ext_stxdependencies, &isnull); 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); ReleaseSysCache(htup);
return statext_dependencies_deserialize(DatumGetByteaP(deps)); return result;
} }
/* /*
......
...@@ -126,24 +126,27 @@ statext_ndistinct_build(double totalrows, int numrows, HeapTuple *rows, ...@@ -126,24 +126,27 @@ statext_ndistinct_build(double totalrows, int numrows, HeapTuple *rows,
MVNDistinct * MVNDistinct *
statext_ndistinct_load(Oid mvoid) statext_ndistinct_load(Oid mvoid)
{ {
bool isnull = false; MVNDistinct *result;
bool isnull;
Datum ndist; Datum ndist;
HeapTuple htup; HeapTuple htup;
htup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(mvoid)); htup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(mvoid));
if (!htup) if (!HeapTupleIsValid(htup))
elog(ERROR, "cache lookup failed for statistics object %u", mvoid); elog(ERROR, "cache lookup failed for statistics object %u", mvoid);
ndist = SysCacheGetAttr(STATEXTOID, htup, ndist = SysCacheGetAttr(STATEXTOID, htup,
Anum_pg_statistic_ext_stxndistinct, &isnull); Anum_pg_statistic_ext_stxndistinct, &isnull);
if (isnull) if (isnull)
elog(ERROR, 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); STATS_EXT_NDISTINCT, mvoid);
result = statext_ndistinct_deserialize(DatumGetByteaPP(ndist));
ReleaseSysCache(htup); ReleaseSysCache(htup);
return statext_ndistinct_deserialize(DatumGetByteaP(ndist)); return result;
} }
/* /*
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment