Chong-cn opened a new issue, #1851: URL: https://github.com/apache/cloudberry/issues/1851
## Summary For a partition-child (leaf) relation, `ANALYZE` silently corrupts the last `pg_statistic` slot (`stakind5`) whenever the column type's own `typanalyze` callback also uses that same slot for its own custom statistics — because Cloudberry's per-leaf HyperLogLog-counter bookkeeping in `do_analyze_rel()` unconditionally overwrites it with no collision check. This can lead to crashes downstream in extension code that relies on that slot (we hit this via PostGIS's geometry type), since the "kind" label gets corrupted while the underlying stats payload is left intact — producing a data-inconsistent `pg_statistic` row. Originally investigated and reported against `cloudberry-contrib/postgis#13` (https://github.com/cloudberry-contrib/postgis/issues/13), but the actual defect lives here in Cloudberry core, not in that extension packaging repo. ## Root cause `src/backend/commands/analyze.c`, function `do_analyze_rel()`, **lines 945-981**: ```c /* * Store HLL/HLL fullscan information for leaf partitions in * the stats object */ if (onerel->rd_rel->relkind == RELKIND_RELATION && onerel->rd_rel->relispartition) { MemoryContext old_context; Datum *hll_values; old_context = MemoryContextSwitchTo(stats->anl_context); hll_values = (Datum *) palloc(sizeof(Datum)); int16 hll_length = 0; int16 stakind = 0; if(stats->stahll_full != NULL) { hll_length = datumGetSize(PointerGetDatum(stats->stahll_full), false, -1); hll_values[0] = datumCopy(PointerGetDatum(stats->stahll_full), false, hll_length); stakind = STATISTIC_KIND_FULLHLL; } else if(stats->stahll != NULL) { ((GpHLLCounter) (stats->stahll))->relPages = relpages; ((GpHLLCounter) (stats->stahll))->relTuples = totalrows; hll_length = gp_hyperloglog_len((GpHLLCounter)stats->stahll); hll_values[0] = datumCopy(PointerGetDatum(stats->stahll), false, hll_length); stakind = STATISTIC_KIND_HLL; } MemoryContextSwitchTo(old_context); if (stakind > 0) { stats->stakind[STATISTIC_NUM_SLOTS-1] = stakind; stats->stavalues[STATISTIC_NUM_SLOTS-1] = hll_values; stats->numvalues[STATISTIC_NUM_SLOTS-1] = 1; stats->statyplen[STATISTIC_NUM_SLOTS-1] = hll_length; } } ``` This runs immediately after `stats->compute_stats(...)` (line 941), i.e. right after whatever the column's own `typanalyze` callback populated. It unconditionally claims **the last pg_statistic slot** (`STATISTIC_NUM_SLOTS-1`, index 4, catalog column `stakind5`) for a per-leaf HLL counter whenever the relation is a partition leaf (`relispartition == true`) — with no check for whether `compute_stats()` already wrote something there. `examine_attribute()` (same file, ~lines 1580-1601) documents this as an assumed invariant ("the last slot is reserved for the hyperloglog counter") and hard-codes `statypid[STATISTIC_NUM_SLOTS-1] = BYTEAOID` accordingly — but this is set up *before* the type's own `typanalyze` runs, so there's no actual enforcement, just an assumption that no type-specific `typanalyze` will ever use that slot. That assumption breaks for any extension type providing a 5-slot-using custom `typanalyze` — PostGIS's `geometry` type is the concrete case we hit (`STATISTIC_SLOT_2D = 4` in `postgis/gserialized_estimate.c`, kind `103`), colliding with `STATISTIC_KIND_HLL = 99`. Since the HLL write always runs after `compute_stats()`, it always wins, silently changing `stakind5` from `103` to `99` while leaving `stanumbers[4]`/`stavalues[4]`'s actual data untouched — the payload survives, only the kind label is corrupted. Downstream consumers that look up statistics by kind (e.g. PostGIS's selectivit y estimator) then get an unrecognized/absent match and, in our case, crash instead of degrading gracefully. This only affects partition-child relations (`relispartition == true`); standalone tables are never affected, and it reproduces identically for single-level and multi-level partitioned tables, at any row count (we reproduced it from 100 rows up through tens of millions). ## Fix (implemented and verified) ```diff --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ do_analyze_rel() MemoryContextSwitchTo(old_context); if (stakind > 0) { - stats->stakind[STATISTIC_NUM_SLOTS-1] = stakind; - stats->stavalues[STATISTIC_NUM_SLOTS-1] = hll_values; - stats->numvalues[STATISTIC_NUM_SLOTS-1] = 1; - stats->statyplen[STATISTIC_NUM_SLOTS-1] = hll_length; + /* + * The last pg_statistic slot is normally reserved for this + * per-leaf HLL counter, but a type-specific typanalyze (e.g. + * PostGIS's geometry ND/2D stats, which use catalog slots + * stakind4/stakind5) may already have claimed it. Never + * overwrite a slot that compute_stats() already populated -- + * doing so silently corrupts the type's own statistics kind + * while leaving its stanumbers/stavalues payload behind, which + * is worse than just not storing the HLL counter for this + * column on this leaf. + */ + if (stats->stakind[STATISTIC_NUM_SLOTS - 1] == 0) + { + stats->stakind[STATISTIC_NUM_SLOTS-1] = stakind; + stats->stavalues[STATISTIC_NUM_SLOTS-1] = hll_values; + stats->numvalues[STATISTIC_NUM_SLOTS-1] = 1; + stats->statyplen[STATISTIC_NUM_SLOTS-1] = hll_length; + } + else + { + ereport(elevel, + (errmsg("skipping per-leaf HLL counter for column \"%s\" of relation \"%s\": last statistics slot already used by kind %d", + NameStr(stats->attr->attname), + RelationGetRelationName(onerel), + stats->stakind[STATISTIC_NUM_SLOTS - 1]))); + } } ``` Why this is safe: `merge_leaf_stats()` (same file, ~lines 4473-4845), which rolls up per-leaf HLL data into root-level statistics, already reads it back **by kind** via `get_attstatsslot(..., STATISTIC_KIND_FULLHLL, ...)` / `get_attstatsslot(..., STATISTIC_KIND_HLL, ...)`, and already has a graceful fallback for a leaf contributing no HLL data at all (`if (!HeapTupleIsValid(heaptupleStats[i])) { ...; continue; }`). Skipping the write on collision just becomes another instance of that same "no HLL data for this leaf" case — the root-level NDV estimate degrades slightly for that column instead of the column's own statistics kind getting corrupted. For the overwhelming majority of columns (any type using the standard `compute_scalar_stats`/`compute_distinct_stats`/`compute_trivial_stats`, which only ever populate slots 0-3), `stats->stakind[STATISTIC_NUM_SLOTS-1]` is still `0` when this code runs, so the guard is a no-op and behavior is unchanged. ## Verification Reproduced and verified against a from-source build of Cloudberry (`--disable-orca --disable-pxf` to keep the build light — unrelated to the fix), with a locally-built PostGIS 3.3.2 to exercise a real 5-slot-using `typanalyze`: ```sql CREATE TABLE verify_fix_test ( id serial, city varchar(50), geom geometry(Polygon,3857) ) PARTITION BY LIST (city) ( PARTITION p_chengdu VALUES ('chengdu'), DEFAULT PARTITION p_other ); INSERT INTO verify_fix_test (city, geom) SELECT 'chengdu', ST_SetSRID(ST_MakeEnvelope(...), 3857) FROM generate_series(1, 1000); ANALYZE verify_fix_test; SELECT stakind4, stakind5 FROM pg_statistic s JOIN pg_attribute a ON a.attrelid = s.starelid AND a.attnum = s.staattnum WHERE a.attrelid = 'verify_fix_test_1_prt_p_chengdu'::regclass AND a.attname = 'geom'; ``` - **Before the patch:** `stakind4=102, stakind5=99` (should be `103`); a subsequent query using `geom && ...` / `ST_Intersects(...)` crashes the backend with `SIGSEGV` (`NOTICE: estimate_selectivity called with null input` immediately precedes the crash). - **After the patch:** `stakind4=102, stakind5=103` (correct) on both the leaf and the parent; the same query returns results cleanly with no crash. Happy to open a PR with this patch if useful — let us know your preferred process (CLA/contribution guidelines) for an Apache Incubator project. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
