Re: COALESCE patch

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