Re: COALESCE patch

Thanks for the review.

Leading Const: good point, I moved that check before the loop, so it runs
once instead of on every iteration.

Early-stop threshold: you're right that the weight of a term is the product
of both probabilities, so I changed the inner check to left_prefix[li] *
right_prefix[ri] < 1e-12.

Relative threshold: I decided against it. In practice a COALESCE has two or
three arguments, so the double loop is tiny and the break almost never
fires. The absolute guard is only there to skip the useless work when a
column is almost entirely NULL. A relative cutoff would drop terms and
change the estimate for very little gain, so I kept the sum exact.

v4 is attached. It passes the regression tests and gives the same estimates
as before.

Feedback is welcome.

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

пт, 7 авг. 2026 г. в 22:23, Alexandre Felipe <o(dot)alexandre(dot)felipe(at)gmail(dot)com>:

>
> Hi Egor,
>
> + foreach(lc, c->args)
> + {
> + Node *arg = (Node *) lfirst(lc);
> ...
> + /* leading Const makes COALESCE itself constant */
> + if (arg == NULL || (lc == list_head(c->args) && IsA(arg, Const)))
>
> Wouldn't it make sense to check for leading const before starting the loop?
>
> You have
> + if (left_prefix[li] < 1.0e-12)
> and
> + if (right_prefix[ri] < 1.0e-12)
>
> but the contribution of each term is
> + acc_selec += left_prefix[li] * right_prefix[ri] * contrib;
>
> What about using if(left_prefix[li] * right_prefix[ri] < 1e-12)?
>
> Isn't the relative contribution more relevant than the absolute
> e.g. ignore terms that would add no more than 0.1% to the
> selectivity one could use
> if(left_prefix[li] * right_prefix[ri] < 1e-3 * acc_selec)
>
> Regards,
> Alexandre
>
> On Fri, Aug 7, 2026 at 1:57 PM prankware <esavelievcode(at)gmail(dot)com> wrote:
>
>> Thanks for the review.
>>
>> v3 (attached) adds comments at the two spots you asked about, and
>> answers the rest below.
>>
>> The special case for a rejected CoalesceExpr:
>> match_coalesce_join_side() returns false both when a side is not a
>> COALESCE and when it is a COALESCE we chose not to decompose. In the
>> second case we must not treat the whole COALESCE as a single branch,
>> so we stop and let the caller estimate the clause the usual way. I
>> added a comment that says this.
>>
>> The bool assigned to a float: I reworked it so the boolean result of
>> the operator maps explicitly to a selectivity of 1.0 or 0.0, with a
>> comment.
>>
>> The check for fewer than two arguments: you're right that it's
>> redundant — a single-argument COALESCE is handled correctly without
>> it, so I removed it.
>>
>> Feedback is welcome.
>>
>> Regards,
>> Egor Savelev,
>> Tantor Labs LLC,
>> https://tantorlabs.com
>>
>> чт, 16 июл. 2026 г. в 17:13, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>:
>> >
>> > On Fri, 2026-07-10 at 15:54 +0300, prankware wrote:
>> > > Thanks for the review — the test cases were very helpful.
>> > > You're right that v1 didn't improve the coalesce(col, const) case. The
>> > > reason is that a comparison of two constants got the default 0.005
>> > > instead of its real result, and joins with a constant on both sides
>> > > were skipped entirely.
>> > > v2 (attached) fixes both, and these four examples now estimate close
>> > > to the actual row counts.
>> >
>> > This version works fine.
>> >
>> > It passes the regression tests. It adds none of its own, but I
>> > can't think of a good way to have stable regression tests for
>> > anything that depends on optimizer statistics.
>> >
>> > My biggest criticism at this point is the readability of the
>> > code. The function comments are alright, but try_coalesce_eq()
>> > is tricky and could do with some comments that explain what is
>> > going on and what the invariants are.
>> >
>> > - Why is there a special treatment of a CoalesceExpr that
>> > match_coalesce_join_side() rejected?
>> >
>> > - Why is it fine to assign a "bool" to a floating point variable?
>> > (An explicit type cast might be a good idea too.)
>> >
>> > There are more places that could do with some illumination.
>> >
>> > Also, why do you explicitly check for CoalesceExpr with less than
>> > two arguments in match_coalesce_join_side()?
>> >
>> > Yours,
>> > Laurenz Albe
>>
>