PG19: contain eager aggregation in the Citus join restriction hook

[ihalatci]

Part of #8597 (Phase 3 — later feature integration).

Problem

PG19 adds eager aggregation, and enable_eager_aggregate defaults to on. The planner now builds a second, grouped RelOptInfo for join relations. Citus's join restriction hook fires for those grouped rels with no guard, so Citus records duplicate join restrictions that carry the same relids as the plain join rel.

Call chain (traced in 19beta2 source)

• optimizer/path/joinrels.c:920-1005 — builds grouped_rel for a join rel.
• optimizer/path/joinrels.c:1055 — calls populate_joinrel_with_paths(..., grouped_rel, ...).
• populate_joinrel_with_paths calls add_paths_to_joinrel in every jointype branch.
• optimizer/path/joinpath.c:379-381 — set_join_pathlist_hook fires here, with no grouped-rel guard.

Citus side:

• src/backend/distributed/planner/distributed_planner.c:2101-2141 — multi_join_restriction_hook unconditionally appends a JoinRestriction to the context and ORs hasSemiJoin / hasOuterJoin.

The relation-side hook is safe — verified, not assumed

• optimizer/path/allpaths.c:589-590 — set_rel_pathlist_hook fires.
• allpaths.c:620 — set_grouped_rel_pathlist() runs after it.
• allpaths.c:1384-1403 — grouped paths are built onto rel->grouped_rel, a separate RelOptInfo.

So multi_relation_restriction_hook (distributed_planner.c:2150) needs no change. Only the join hook is exposed.

Impact

• Certain: planning work that Citus collects and then discards, on every distributed join, by default.
• Plausible but unproven: duplicate restrictions with identical relids altering a resulting plan. This has not been demonstrated and should not be described as a correctness bug until it is.

Why it is still worth treating as a risk class rather than a curiosity: src/backend/distributed/planner/recursive_planning.c:512-532 documents the PG16 695f5deb7 precedent, where an upstream change to when join hooks are invoked produced silently incorrect Citus plans.

Options

1. Force enable_eager_aggregate = off for the duration of Citus restriction collection (narrow, reversible, matches the existing outer-join GUC handling around shared_library_init.c:1591-1606).
2. Use the new PG19 levers instead — join_path_setup_hook (joinpath.c:177-179, optimizer/paths.h:32-38) and pgs_mask / PGS_JOIN_ANY (pathnodes.h:71,97,263,1039,3616-3626) — which allow suppressing the grouped variant directly rather than filtering after the fact.

Option 1 is the smaller change; option 2 is the more precise one.

First step

Reproduce on a live PG19 cluster: a distributed join with grouping, enable_eager_aggregate on vs off, comparing both the collected restriction count and the final plan. That determines whether this is a performance-only fix or a correctness fix.

[ihalatci]

Investigation complete: not a correctness bug today, but the containment premise needs correcting

Reopened after an accidental close. Summary of a full investigation on PG19 beta2 with a live 2-worker cluster. No repo changes were made and none are proposed at this time.

Verdict

The duplicate JoinRestriction entries this issue predicts are real, but confined to worker nodes, and they are provably never consumed. The leading correctness hypothesis in the original body is dead.

Tier	Grouped joinrels observed
Vanilla local tables (positive control)	exact 2x, both entries logging identical relids
Coordinator, distributed tables	zero, across a 12-case battery
Workers (shard-query planning)	682 at scale

Worker-side duplicates land in a context nothing reads: distributed_planner.c:300-303 calls PlanDistributedStmt only when needsDistributedPlanning. Shard tables are not in pg_dist_partition, so the restriction context is popped unused.

The decisive experiment

Rather than rely on the coordinator never building grouped rels, duplication was injected directly: a second identical JoinRestriction (same jointype, same copyObject(extra->restrictlist), same relids) appended on every hook call, env-gated. Injection was verified by max joinRestrictionList length going 12 -> 24, exactly 2x.

• 16-query battery: 16/16 RESULT_SAME and PLAN_SAME
• 14 adversarial queries (recurring-tuples reference outer joins, non-partition-column outer/subquery joins, IN/NOT EXISTS against reference tables, 4-way, CTE, UNION ALL, local-table joins, transitive-equivalence + outer): 14/14 RESULT_SAME and PLAN_SAME, including two queries whose baseline is an ERROR, where the error text was byte-identical

30/30 identical with duplication forced to 100%. Citus is empirically duplicate-insensitive. This kills the prime suspect GenerateAttributeEquivalencesForJoinRestrictions (relation_restriction_equivalence.c:1583), the only consumer that accumulates rather than tests.

Independently, joinrels.c:768-774 passes the same sjinfo to both invocations, so the hasSemiJoin/hasOuterJoin OR at distributed_planner.c:2135-2138 is idempotent (a || a == a). That hypothesis fails on the source alone.

Why the coordinator is immune, and why that matters

Immunity is incidental, not structural. It rides on a cost heuristic:

1. plancat.c:1327-1337 sets curpages = RelationGetNumberOfBlocks(rel) and returns *tuples = 0 when curpages == 0, before pg_class.relpages/reltuples are read at :1340-1341. Coordinator shells for distributed tables have 0 physical blocks, so the row estimate is 0, clamped to 1.
2. relnode.c:2809-2810: agg_useful = (rel->rows / grouped_rows) >= min_eager_agg_group_size, boot value 8.0 (postgresql.conf.sample:464).
3. With rows ~1 and grouped_rows ~1, the ratio is ~1 < 8, so agg_useful is false.
4. joinrels.c:961-963: if (!agg_info->agg_useful && (rel1_empty == rel2_empty)) return; The grouped rel is never built and the hook fires once.

Triangulated on local tables with parallelism disabled: 500k rows / 50 groups (avg 10000) produces eager aggregation under default settings; 500k rows / 500k groups (avg 1) does not, under identical GUCs, suppressed purely by the ratio. A first attempt at this was invalidated and re-run after catching that parallel aggregation emits the same Partial/Finalize node names and had produced a false positive.

Revisit trigger. Nothing enforces this. If a future beta/GA lowers min_eager_agg_group_size, changes estimate_rel_size, or lets coordinator shells acquire real blocks, grouped joinrels will appear at the coordinator and this issue's predicted duplication becomes live. The 30/30 result says it would still be harmless, but the containment fix to reach for is a single IS_GROUPED_REL(joinrel) early return in multi_join_restriction_hook, not either option in the original body.

Corrections to the original issue body

1. Option (a) is mis-cited. shared_library_init.c:1591-1606 is a GUC definition (citus.enable_outer_joins_with_pseudoconstant_quals_pre_pg17), not a save/restore, so this is not the small mirror the body implies. The correct precedent is in distributed_planner.c: saveNestLevel :171, DisableSelfJoinElimination() :238, AtEOXact_GUC :289-290, NewGUCNestLevel()/set_config_option() :3095-3096.
2. Option (a) would also accomplish nothing, since the coordinator does not build grouped joinrels for distributed tables in the first place.
3. enable_eager_aggregate is not propagated coordinator to worker. A coordinator SET silently leaves workers unchanged. Propagation requires citus.propagate_set_commands = 'local' (default none; only none and local are selectable), a SET LOCAL, and an explicit multi-statement transaction.
4. joinpath.c:379-381 firing with no grouped-rel guard, and relnode.c:499-537 build_grouped_rel being a flat memcpy that never clears relids, are both correct as written in the original body and confirmed empirically.

Recommendation

Keep open as a watch item with no code change. Both levers in the original body have zero measurable benefit and one has real downside: the residual cost is a few pallocs per join per worker shard query into a context nothing reads, while eager aggregation is simultaneously delivering a genuine Partial/Finalize optimization on those same shard queries. GUC-based containment risks removing a benefit to prevent a harm that does not exist.

Stated as unproven

• Setting min_eager_agg_group_size = 0 at the coordinator and directly observing grouped joinrels appear for a distributed join was not done; it requires re-instrumenting the hook. The mechanism above rests on source plus the 0-block measurement plus the local-table triangulation.
• The full Citus regression suite was not run under forced duplication. 30 hand-built queries including error paths is strong but not exhaustive.
• Behaviour beyond PG19 beta2 is unknown; see the revisit trigger above.

[ihalatci]

Status: no active correctness defect reproduced and no local fix/PR. Current immunity is incidental but 30/30 runtime probes preserved plans/results. Needs: product decision to close as a watch item or retain a narrowly scoped detection/containment proposal; implementation only if coordinator shells begin carrying statistics or a future PG change activates the predicted path.