Keep the ungrouped vectorized aggregate over a unique-key inner join

[linuxhikerpm]

Summary

• An ungrouped aggregate over a unique-key inner Hash Join can keep Columnar Vectorized Aggregates when pgcolumnar.enable_ungrouped_vector_agg is on. The join is a filter of the fact table, so the fold no longer disappears the moment the input is a joinrel.
• Duplicate-key dimensions, LEFT joins, and grouped aggregation over a join stay on core Agg. The GUC stays off by default. This does not close A runtime filter is worth nothing unless the fact table is clustered on the join key, and the fold a join removes is worth 4.1x but is off by default #752.
• Independent twins: test/native_join_vector_agg.sh and test/pytest/test_join_vector_agg.py. Causation: dropping the JOINREL branch turns only the unique-join marker red.

Test plan

• Shell twin green on distro PG 18.6
• Pytest twin green on distro PG 18.6
• Causation mutation: unique-join marker red, duplicate and LEFT still green
• CI matrix on the PR

Made with Cursor

[OffgridwithJD]

This returns a wrong answer. Please hold it. The fold drops a join condition it never
looks at, and the aggregate comes back 34.7% too high on a four-line fixture.

The defect

pgcolumnar_join_fold_vars reads only hashPath->path_hashclauses:

if (list_length(hashPath->path_hashclauses) != 1)
    return false;
restrictInfo = linitial_node(RestrictInfo, hashPath->path_hashclauses);

joinrestrictinfo is never consulted -- it does not appear anywhere in the file. A join
can carry the one hash clause and further conditions that the planner applies as a
Join Filter. Those survive into the real plan and vanish from the fold, because the fold
replaces the join node with a key-membership test and a key-membership test is not what
the join meant.

innerrel_is_unique does not cover this: it is asked about path_hashclauses, so it
proves the dimension is unique on the key and says nothing about the rest of the
predicate.

Only a CROSS-relation non-equi clause is lost, which is why the existing arms pass. A
condition on the dimension alone lands in its baserestrictinfo and the drained dim path
applies it; one on the fact alone lands in factRel->baserestrictinfo and the scan applies
it. It is exactly the clause mentioning both sides that has nowhere to go.

Measured, on 48531f36, pg18a

CREATE TABLE d(k int PRIMARY KEY, t int);
INSERT INTO d SELECT g, 50 FROM generate_series(1,100) g;
CREATE TABLE f(k int, v int) USING pgcolumnar;
INSERT INTO f SELECT (g % 100) + 1, g % 100 FROM generate_series(1,10000) g;

SELECT sum(f.v) FROM f JOIN d ON f.k = d.k AND f.v > d.t;

premise: the extra condition removes rows   4900 of 10000

heap oracle (no columnar anywhere)          367500
GUC off   -- core Agg over a real Hash Join 367500
GUC on    -- the fold under test            495000     <-- wrong, +34.7%

495000 is exactly the sum with no condition at all, which the control confirms:

CONTROL, plain equi-join where the fold IS valid:
  heap 495000   GUC off 495000   GUC on 495000   fold engaged: yes

So the fold is right about the join it handles and wrong about the one it should have
refused. The control matters: without it these numbers would only show that the fold
disagrees with core, not that it is the fold that is wrong.

The plans say it outright. GUC off:

Aggregate
  ->  Hash Join
        Hash Cond: (f.k = d.k)
        Join Filter: (f.v > d.t)          <-- applied
        ->  Custom Scan (PgColumnarScan) on f
        ->  Hash  ->  Seq Scan on d

GUC on:

Custom Scan (PgColumnarScan)
  Columnar Vectorized Aggregates: 1
  Columnar Join Fold: yes
  Columnar Batch Fold: yes
  ->  Seq Scan on d                        <-- and no Join Filter anywhere

The condition is not applied, not deferred, not mentioned. It is gone.

Fix

Refuse the fold unless the hash clause is the only join clause:

if (list_length(hashPath->jpath.joinrestrictinfo) != 1)
    return false;

paired with the existing path_hashclauses check, so "one hash clause" and "one join
clause" have to be the same clause. That keeps the PR's stated scope -- a unique-key inner
join that is a pure filter of the fact table -- and it is the condition under which the
summary's sentence "the join is a filter of the fact table" is actually true.

I would also want the arm, because the existing causation mutation cannot reach this: it
drops the JOINREL branch and watches the unique-join marker go red, which tests that the
fold engages, not that the fold is right. The arm this needs compares a sum against an
oracle on a join carrying an extra condition -- the fixture above is yours to take, and
the heap twin is load-bearing in it.

Worth adding the same shape for the other ways a join can mean more than its hash clause,
since one missing guard of this kind usually has siblings: ON f.k = d.k AND f.v <> d.t,
and a three-way join (currently refused by bms_num_members(joinrel->relids) != 2, so that
one is already covered -- worth a comment saying so, as it is the same question).

Smaller things, none of them blockers

1. joinCollation comes from the fact column, not the clause. state->joinCollation = att->attcollation, but the equality the join means is the OpExpr's inputcollid.
   For deterministic collations these agree on equality, so I could not construct a wrong
   answer from it and I am not claiming one. Reading the collation from the clause that
   defines the comparison would still be the honest source, and it costs one line.

2. pgcolumnar_join_fold_grow re-copies every key. It calls
   pgcolumnar_join_fold_insert(state, oldKeys[i]), and insert does
   datumCopy(value, ...) -- so a rehash re-copies datums that are already in the fold
   context, leaving the old copies there until the context dies. Harmless for by-value
   types, O(total bytes) of waste for a varlena key on a large dimension. Inserting the
   existing Datum without re-copying during a rehash avoids it.

3. The dimension is drained in BeginAggScan, so it is read even on a path that
   returns no rows. After the EXEC_FLAG_EXPLAIN_ONLY return, so EXPLAIN is unaffected.
   Only worth a sentence in a comment.

Everything else I went looking for held up: NULL keys are skipped on both sides and INNER
= never matches NULL, so that is right; parallel is correctly excluded
(dimPath == NULL && on the parallel gate); the open-addressed probe terminates on a full
table; the custom_private index juggling is consistent between
PgColumnarPlanAggPath and PgColumnarCreateAggScanState (0-2 base, 3-5 join, 6 resno).

[OffgridwithJD]

Checked b7b7f8de. The push is a rebase onto 90f7fff5 -- the wrong answer is still there.

By inspection of the file at that head, not a diff:

$ git show b7b7f8de:src/columnar_vector.c | grep -n joinrestrictinfo
(no output)

$ git show b7b7f8de:src/columnar_vector.c | grep -n path_hashclauses
1042:	if (list_length(hashPath->path_hashclauses) != 1)
1044:	restrictInfo = linitial_node(RestrictInfo, hashPath->path_hashclauses);
1152:							innerrel, JOIN_INNER, hashPath->path_hashclauses,

So the fold still reads only the hash clause, and a join carrying an extra cross-relation
condition still has that condition dropped. Flagging it in case the push was meant to
address this and landed the rebase only.

To be precise about what I have and have not done at this head: the absence of the guard
is verified here, the wrong answer was measured at 48531f36. I will re-run the
fixture against b7b7f8de and post the numbers -- the box is busy with a five-major matrix
for #1010 and I would rather not interleave a second workload with its timing arms than give
you a number taken under contention. Nothing about the rebase should change the result, and
I will say so either way.

The fixture and the one-line guard are in my earlier comment, unchanged.

[jdatcmd]

Approving at be0ef93c. The safety argument rests on core's own uniqueness proof rather than a hand-rolled one, the refusals are stricter than they need to be, and the removal proof is recorded in the ledger rather than asserted in prose.

First, what I did to this PR and why

It was DIRTY with an empty statusCheckRollup — which reads as pending=0, bad=0 and is not the same thing as green. No CI run had ever existed for it. I said "checks already complete" earlier on that basis and was wrong.

The conflict was mine: #1007, #1005 and #1008 merged within the hour and all three touch docs/user-guide.md and test/pytest/TESTS.md. Two additive collisions, both resolved by keeping both sides. The TESTS.md one is #996 in a different file — both sides appended a section and both called it 28; #1007's landed first and keeps 28, this one became 29, with heading, contents entry and anchor moved together.

Then CI ran for the first time and refused the tree:

FAIL  every script that declares an interpreter is executable:
      got [[1: test/native_join_vector_agg.sh]] want [[]]

Mode 100644 with a shebang. I checked whether my merge caused it: the blob hash and the mode are identical on the pre-merge head 48531f36. It arrived with the branch and nothing had ever looked. Fixed as its own commit. Rebasing did not break this PR; rebasing is what first let anything see it.

The correctness argument holds where it matters

The claim is that a unique-key inner join is a filter of the fact table, so the ungrouped fold stays valid. That is true only if no fact row can match twice, and this does not try to prove that itself:

if (!innerrel_is_unique(root, joinrel->relids, outerPath->parent->relids,
                        innerrel, JOIN_INNER, hashPath->path_hashclauses, true))
    return false;

Core's own proof, the one the planner uses for join removal. That is the right call — a hand-rolled uniqueness test here would be a second implementation of something subtle, diverging silently from the planner's.

The refusals are stricter than the minimum:

guard	why it is the careful version
scans root->join_info_list and rejects any non-INNER SpecialJoinInfo covering the rel	an INNER hash path can sit under an outer join; checking jpath.jointype alone would miss it
bms_num_members(joinrel->relids) != 2	no multi-way
innerPath->param_info != NULL rejected	no parameterized inner
single hash clause, OpExpr, two Var args	no compound or expression keys

The join_info_list scan is the one I went looking for and expected not to find.

Evidence

native_join_vector_agg=PASS in the CI run, all 13 checks green
duplicate dim keys refuse the join fold   + answer equals heap
LEFT join refuses the join fold           + answer equals heap
unique join fold answer equals GUC off    + answer equals heap

The two refusal arms are the ones that matter: each is paired with a heap-equality arm, so a wrongly-accepted fold would show as a wrong answer rather than only as a missing marker.

The removal proof is in the ledger, not in the PR body:

native_join_vector_agg  unique join uses vectorized agg when GUC on  2026-09-12  drop JOINREL fold

That is the only row in the whole ledger carrying a mutation, and it is this PR's. The claim "dropping the JOINREL branch turns only the unique-join marker red" is therefore recorded where a later run can contradict it, which is the point of the column.

GUC off by default (pgcolumnar_enable_ungrouped_vector_agg = false), so this ships inert and does not close #752.

One note

The pytest twin's independence is asserted in its docstring — "Independently of native_join_vector_agg.sh, this session builds its own fixtures" — and it holds: no import of the shell harness, no invocation of the suite. Worth saying explicitly since the two files share the public EXPLAIN string, which is the seam they are supposed to share.

Verdict

Approved. Merging is not mine to decide, and note the head now carries two commits of mine — a merge resolution and the mode fix — so @linuxhikerpm may want to look before it lands.

[OffgridwithJD]

Re-measured at be0ef93c, the approved head. It still returns a wrong answer.

I said I would re-run the fixture and post the numbers either way. Same box, same fixture,
pg18a, the head that now carries an approval:

premise: the extra condition removes rows   4900 of 10000

heap oracle (no columnar anywhere)          367500
GUC off  -- core Agg over a real Hash Join  367500
GUC on   -- the fold under test             495000     <-- +34.7%

CONTROL, plain equi-join where the fold IS valid:
  heap 495000   GUC off 495000   GUC on 495000

495000 is exactly the sum with no condition at all. The control shows the fold is correct
on the join it should handle and wrong on the one it should have refused, so this is not the
fold disagreeing with core -- it is the fold dropping a predicate.

The plans say it outright. GUC off, at this head:

Aggregate
  ->  Custom Scan (Columnar Runtime Filter Coordinator)
        ->  Hash Join
              Hash Cond: (f.k = d.k)
              Join Filter: (f.v > d.t)          <-- applied

GUC on:

Custom Scan (PgColumnarScan)
  Columnar Vectorized Aggregates: 1
  Columnar Join Fold: yes
  Columnar Batch Fold: yes
  ->  Seq Scan on d                              <-- no Join Filter anywhere

And the cause is unchanged across all three pushes:

$ git show be0ef93c:src/columnar_vector.c | grep -c joinrestrictinfo
0

joinrestrictinfo appears nowhere in the file. pgcolumnar_join_fold_vars reads
hashPath->path_hashclauses and guards list_length(...) != 1, which constrains the clauses
the join can HASH on and says nothing about the clauses it carries.

This is the reason CI is green

13 checks, 0 failures, and the aggregate is wrong. No arm in either harness joins on a
key and also constrains a second column, so nothing in the tree can see this. The PR's own
causation mutation drops the JOINREL branch and watches the unique-join marker go red, which
proves the fold engages -- never that it is right.

That is worth separating from the defect itself: a wrong-answer bug that passes the full gate
means the gate has no arm for this shape, and it will not acquire one by this PR merging.

The fix, again, unchanged

if (list_length(hashPath->jpath.joinrestrictinfo) != 1)
    return false;

beside the existing path_hashclauses check, so "one hash clause" and "one join clause" must
be the same clause. That is exactly the condition under which the summary's sentence -- "the
join is a filter of the fact table" -- is true.

Only a CROSS-relation non-equi clause is at risk: a predicate on the dimension alone lands in
its baserestrictinfo and the drained dim path applies it; one on the fact alone lands in the
fact's. That is why every existing arm passes, and it is why the guard is one line.

The arm this needs compares a sum against an oracle on a join carrying an extra
condition. The fixture above is four lines of SQL and it is yours to take:

CREATE TABLE d(k int PRIMARY KEY, t int);
INSERT INTO d SELECT g, 50 FROM generate_series(1,100) g;
CREATE TABLE f(k int, v int) USING pgcolumnar;
INSERT INTO f SELECT (g % 100) + 1, g % 100 FROM generate_series(1,10000) g;
SELECT sum(f.v) FROM f JOIN d ON f.k = d.k AND f.v > d.t;   -- 367500, not 495000

I am not able to block this and I am not trying to -- the approval is yours to act on. But
the number is the number, it is reproducible in under a minute, and I would rather say it
plainly three times than have it land quietly.

[linuxhikerpm]

Hold fix is on fbc1e5d2d03cbf620193bd1419823d93e282a892.

The fold read only path_hashclauses, so a Join Filter besides the hash clause was dropped and the sum over-counted. Independent shell and pytest twins failed red first (marker present, answer disagreed with heap). The fold now refuses unless joinrestrictinfo holds exactly that one hash clause. Causation: dropping only that length check turns the extra-filter checks red and leaves the unique-join fold green.

[jdatcmd]

Re-approving at fbc1e5d2. The head moved after my approval and the delta is a tightening, so I checked it rather than letting the badge carry:

/* A Join Filter besides the hash clause is not a membership test. The
 * fold would ignore it and over-count. */
if (list_length(hashPath->jpath.joinrestrictinfo) != 1)
    return false;

That is a real hole and you found it yourselves: an extra join clause means the hash clause is no longer the whole join condition, so treating the join as a membership filter would over-count. Strictly more conservative than what I approved, with five new arms across both harnesses and heap-equality on both the extra-filter and inequality cases.

Merging as soon as CI settles.

[OffgridwithJD]

Fixed, and verified by running it rather than reading it

fbc1e5d2. Same fixture, same box, pg18a:

premise: the extra condition removes rows   4900 of 10000
heap oracle                                 367500
GUC off                                     367500
GUC on   -- the fold under test             367500     <-- was 495000
fold engaged?                                    0     <-- correctly refuses

CONTROL, plain equi-join where the fold IS valid:
  heap 495000   GUC off 495000   GUC on 495000
  fold engaged?  1                                     <-- still folds

The control is the half that matters for a guard like this. joinrestrictinfo != 1 could
have been too strict and quietly disabled the feature it exists to protect; it is not. The
valid equi-join still folds, and the plan under GUC on for the invalid one is now identical
to GUC off -- core Agg over a real Hash Join with the Join Filter applied.

I had the guard by inspection first and did not report it as fixed on that basis. Reading
code told me the right thing twice today and the wrong thing twice, so the run is what
settles it.

The arms are the right shape

expect.rows([on], [off],  "extra join filter answer equals GUC off")
expect.rows([on], [heap], "extra join filter answer equals heap")

A value against an oracle, with a heap twin, plus the plan assertion that the vectorized
agg node is absent. And test_inequality_join_filter_refuses_the_fold covers the <> shape.
Twins on both sides. That is the arm the earlier causation mutation could not be: dropping
the JOINREL branch proves the fold engages, this proves it is right.

The comment names the reason, which is the part that survives:

/*
 * A Join Filter besides the hash clause is not a membership test. The
 * fold would ignore it and over-count. Three-way joins are already
 * refused above: more than two relids.
 */

Not approving yet, for one reason

build (PG 15, x86_64) is still IN_PROGRESS -- 11 checks, 1 pending, 0 failing. I approve on
a green rollup, never a pending one, and I will come back when it completes.

Worth one note for the record: the existing approval was given at be0ef93c, which is the
head I measured the wrong answer on, and this repository does not dismiss reviews on a push.
So that approval predates the fix. Mine, when it comes, will be on fbc1e5d2.