Re: COALESCE patch

From: prankware <esavelievcode(at)gmail(dot)com>
To: Osama Abdul Qader <osamaabdulqader(dot)cs(at)gmail(dot)com>, Ilia Evdokimov <ilya(dot)evdokimov(at)tantorlabs(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: COALESCE patch
Date: 2026-09-07 12:23:39
Message-ID: CAF=hKRCfRUaApyc_wGgNXDhwqb+VAJSBGcHZKsRyhH5Oj6GPLw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for the review, and for the clear repro — you're right on both the
diagnosis and the fix.
The context was inferred from fcinfo (fn_oid == F_EQJOINSEL), and that
breaks on recursion: try_coalesce_eq() calls eqjoinsel() through
DirectFunctionCall, which leaves fcinfo->flinfo NULL. So when a branch is
itself a COALESCE, the nested eqjoinsel() saw is_eqjoin = false and
estimated the join as a restriction, which is where the ~6x came from.
v6 (attached) passes is_eqjoin explicitly, as you suggested:
eqsel_internal() passes false and eqjoinsel() passes true, and the fcinfo
check is gone. On your example the estimate drops from ~66M to ~7.8M
(actual 10M), and the earlier cases are unchanged.
Feedback is welcome.

Osama Abdul Qader, thanks for the offer, but it's already handled — v6
(just posted to the thread) passes is_eqjoin explicitly and removes the
F_EQJOINSEL check, so no extra work is needed here.

Regards, Egor Savelev, Tantor Labs LLC, https://tantorlabs.com

пн, 7 сент. 2026 г. в 14:50, Osama Abdul Qader <osamaabdulqader(dot)cs(at)gmail(dot)com
>:

> Hi Ilia,
>
> Thanks for pointing this out. I understand the issue with relying on
> F_EQJOINSEL to determine the equality-join context, especially since
> PostgreSQL has multiple join selectivity estimators.
>
> Is this issue still open? If so, I’d be happy to take a look at
> implementing the explicit is_eqjoin propagation you suggested and add the
> necessary regression tests.
>
> With Regards,
>
> Osama Abdul Qader
>
> On Mon, 7 Sept, 2026, 4:34 pm Ilia Evdokimov, <
> ilya(dot)evdokimov(at)tantorlabs(dot)com> wrote:
>
>> While reviewing try_coalesce_eq() I noticed this
>>
>> + bool is_eqjoin = (!fcinfo->flinfo != NULL && fcinfo->flinfo->fn_oid ==
>> F_EQJOINSEL)
>>
>> This is checking against one specific selectivity function, but
>> eqjoinsel() is not the only join-selectivity estimation - it's just the
>> most common one (used by = operators). pg_proc.dat alone registers over
>> a dozen others as JOIN estimators. We need a different mechanism. The
>> only way I see is to stop interfering the context and pass it in
>> explicitly give try_coalesce_eq() a bool is_eqjoin parameter.
>>
>> For example, consider this scenario:
>>
>> ```
>> CREATE TABLE a (x1 int, x2 int, y int);
>> CREATE TABLE b (w int);
>>
>> INSERT INTO a (x1, x2, y)
>> SELECT
>> CASE WHEN i % 3 = 0 THEN NULL ELSE i % 1000 END,
>> CASE WHEN i % 3 = 0 THEN i % 500 ELSE NULL END,
>> i % 200
>> FROM generate_series(1, 100000) i;
>>
>> INSERT INTO b (w)
>> SELECT i % 1000
>> FROM generate_series(1, 100000) i;
>>
>> CREATE INDEX a_coalesce_x1x2_idx ON a (COALESCE(x1, x2));
>>
>> ANALYZE a, b;
>>
>> EXPLAIN ANALYZE
>> SELECT * FROM a JOIN b ON COALESCE(COALESCE(a.x1, a.x2), a.y) = b.w;
>> QUERY PLAN
>>
>> -----------------------------------------------------------------------------------------------------------------------
>> Hash Join (cost=2693.00..688019.33 rows=66488333 width=16) (actual
>> time=11.824..343.641 rows=10000000.00 loops=1)
>> Hash Cond: (COALESCE(COALESCE(a.x1, a.x2), a.y) = b.w)
>> Buffers: shared read=886
>> -> Seq Scan on a (cost=0.00..1443.00 rows=100000 width=12) (actual
>> time=0.355..2.294 rows=100000.00 loops=1)
>> Buffers: shared read=443
>> -> Hash (cost=1443.00..1443.00 rows=100000 width=4) (actual
>> time=10.691..10.693 rows=100000.00 loops=1)
>> Buckets: 131072 Batches: 1 Memory Usage: 4540kB
>> Buffers: shared read=443
>> -> Seq Scan on b (cost=0.00..1443.00 rows=100000 width=4)
>> (actual time=0.243..3.641 rows=100000.00 loops=1)
>> Buffers: shared read=443
>> Planning:
>> Buffers: shared hit=139 read=33
>> Planning Time: 1.712 ms
>> Execution Time: 431.061 ms
>> (14 rows)
>> ```
>>
>> Estimated rows are 6 times bigger than actual ones.
>>
>> --
>> Best regards,
>> Ilia Evdokimov,
>> Tantor Labs LLC,
>> https://tantorlabs.com/
>>
>>
>>
>>

Attachment Content-Type Size
v6-0001-Coalesce-eqsel-eqjoinsel.patch text/x-patch 14.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2026-09-07 12:26:12 Re: Offline data checksum changes can cause incorrect checksum state on standbys
Previous Message solai v 2026-09-07 12:20:38 Re: pg_rewind: Skip vanished source files during traversal