| From: | Richard Guo <guofenglinux(at)gmail(dot)com> |
|---|---|
| To: | wenhui qiu <qiuwenhuifx(at)gmail(dot)com> |
| Cc: | Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Fix HAVING-to-WHERE pushdown with nondeterministic collations |
| Date: | 2026-05-01 02:47:20 |
| Message-ID: | CAMbWs48x_Oth+DMUZZpyWNARpK_1Ba67_RVmbuO9sPyk5V8KRA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Apr 30, 2026 at 12:08 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> I was about to push the v2 patch, but I just can't shake off the
> concern Wenhui Qiu raised about the repeated subtree scan. I still
> don't have a concrete real-world case where a query has a large enough
> HAVING clause for it to matter, but let's just be paranoid.
>
> I think we can fix it easily. The current walker calls
> pull_var_clause() at every collation-aware node, which re-walks the
> subtree. The fix is to flip it inside out: walk top-down, push
> inputcollids onto a LIFO stack, and at each GROUP Var check against
> the stack. This way, we only need to walk the expression tree once.
> Attached v3 does this.
>
> v3 also fixes the RowCompareExpr case. Unlike the node types covered
> by exprInputCollation(), RowCompareExpr carries per-column
> inputcollids[] rather than a single inputcollid, so we need to descend
> into each (largs[i], rargs[i]) pair with the matching collation pushed
> onto the stack. Without this, a HAVING clause like:
>
> HAVING ROW(x, 1) < ROW('ABC' COLLATE case_sensitive, 1)
>
> over a case_insensitive group would give wrong results.
I've committed this and back-patched it to v18. I was not
back-patching further because pre-v18 branches would need a very
different and more complex fix due to the lack of the RTE_GROUP
mechanism. I think it's too risky, and doesn't seem justified given
the absence of field reports.
- Richard
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Xuneng Zhou | 2026-05-01 02:57:08 | Re: BUG: Cascading standby fails to reconnect after falling back to archive recovery |
| Previous Message | Xuneng Zhou | 2026-05-01 02:44:00 | Re: Implement waiting for wal lsn replay: reloaded |