PG19: contain eager aggregation in the Citus join restriction hook
- Dominant language
- C
- Stars
- 12.8k
- Forks
- 794
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 31
Description
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.
Contributor guide
Research direction
Start with a live PG19 cluster and reproduce a distributed join with grouping while toggling enable_eager_aggregate, comparing collected restriction counts and final plans. Read optimizer/path/joinrels.c, optimizer/path/joinpath.c, and distributed_planner.c:2101-2141; done means the chosen integration avoids duplicate grouped-join restrictions without changing valid planning behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, postgresql
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100