Re: hashjoins vs. Bloom filters (yet again)

On Sun Aug 9, 2026 at 1:01 PM -03, Rui Zhao wrote:
> Hi Tomas,
>
> Five planner comments on v9, and a correctness check -- I went looking for wrong
> answers rather than for speed, and did not find any.
>
> 1 a conditional rowcount living in pathlist, which every reader then has to
> be taught to ignore -- and one still isn't; 0001 moves those paths out
> 2 the probe cost is charged on the wrong row count -- 0002
> 3 bloom_filter_pushdown_max_build_relids = 1 disables pushdown entirely
> 4 six arms of find_bloom_filter_recipient() are never taken
> 5 whether the pre-pass needs to estimate at all -- a question, not a patch
> 6 the correctness check: 480 runs with the filter off and on, 0 differences
>

Thanks a lot for the review, especially the correctness run in 6. I've
addressed 2, 3 and 4 in v10; 1 and 5 I'd like to take up as their own
follow-ups rather than fold into this round, and I also want to think
more about it, so I'll not reply to these points yet on this message.

2. Charge the probe cost on the rows actually probed.

I incorporated your 0002 into v10 (folded into 0020). The diagnosis
seems correct to me: the probe runs before the scan's own quals, so it's
paid on the fetched count, not the output count, and charging on
path->rows understated it by the qual selectivity. The fkest plan in
join drops the filter now, which is the correct direction. Thanks for
the patch.

3. bloom_filter_pushdown_max_build_relids = 1 disabling pushdown.

Fixed. As you spotted, bloom_build_side_join_ratio() looks up {owner} +
build_relids, which is one relation larger than the build side, so a
K-relation build side needs the enumeration to reach level K+1. The
enumeration now runs one level beyond max_build_relids, so a build side
of exactly that many relations gets a real estimate. =1 now produces
filters, and the default 3 allows three-relation build sides, matching
guc_parameters.dat.

4. Dead arms of find_bloom_filter_recipient().

Fixed. The five row-preserving arms (Sort/IncrementalSort/Material/
Memoize/Unique) now capture the recursion result and Assert(res ==
NULL), which records that they never actually route a filter today and
will fire if that ever changes, at which point, as you say, the rescan
case needs an answer first. I split T_Limit out to return NULL
unconditionally: pushing a filter below a LIMIT would change which rows
the LIMIT returns, so that one is wrong rather than merely unhandled,
and I'd rather stop there in all builds than rely on a cassert-only
guard.

I've folded 2/3/4 into v10 and will post the patchset in a follow-up
message with some other fixes. Thanks again for working on this.

--
Matheus Alcantara
EDB: https://www.enterprisedb.com