do_analyze_rel() clobbers pg_statistic slot 4 (stakind5) on partition leaves, colliding with type-specific typanalyze (e.g. PostGIS geometry)

[Chong-cn]

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 (cloudberry-contrib/postgis#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:

/*
 * 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 selectivity 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)

--- 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:

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(
           11500000 + (random()*200000),
           3500000  + (random()*200000),
           11500000 + (random()*200000) + 300,
           3500000  + (random()*200000) + 300,
           3857
         ),
         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';

SET client_min_messages = notice;
SELECT count(*) FROM verify_fix_test
WHERE geom && ST_MakeEnvelope(11583573, 3588565, 11583878, 3588871, 3857)
AND ST_Intersects(ST_MakeEnvelope(11583573, 3588565, 11583878, 3588871, 3857), 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.