| From: | Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> |
|---|---|
| To: | prankware <esavelievcode(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Cc: | ilya(dot)evdokimov(at)tantorlabs(dot)com |
| Subject: | Re: COALESCE patch |
| Date: | 2026-07-16 14:13:31 |
| Message-ID: | 2a99ac998e6e90df32efbe6c872c3665f447da23.camel@cybertec.at |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-07-16 14:22:45 | Re: Type assertions without GCC builtins |
| Previous Message | Tomas Vondra | 2026-07-16 14:06:42 | Re: PGLZ Compression Optimization |