Re: COALESCE patch

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
>