| From: | Sami Imseih <samimseih(at)gmail(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Ewan Young <kdbase(dot)hack(at)gmail(dot)com> |
| Subject: | Re: Disallow outer-level and WHERE-clause aggregates in GRAPH_TABLE |
| Date: | 2026-08-12 22:53:38 |
| Message-ID: | CAA5RZ0sTr_Yt_9_qtt5PyzN-EaRnebXgWDTSgMn1H1Ak5vU-yQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Thanks for the comments!
> I do not like this fix approach, and for that matter I don't like
> f58567105. This is expensive thanks to the extra tree traversal,
I was not feeling totally convinced about this either, but was not
sure if the tree traversal is that big of a problem. We should
definitely avoid doing this, if we can.
> f58567105 is user-unfriendly because it fails to say exactly what
> or where is the construct it's rejecting, and neither patch is
> following the perfectly good structure that the parser already
> has for this kind of check. IMO the correct way to handle this
> restriction is to check it in check_agglevels_and_constraints() based
> on the ParseExprKind of the surrounding expression.
You are right. This makes sense to me now. We can introduce
EXPR_KIND_GRAPH_TABLE_COLUMNS and
EXPR_KIND_GRAPH_TABLE_WHERE ParseExprKind's.
Inside check_agglevels_and_constraints(), we can check the restriction
before we walk up to the query level the aggregate belongs to.
Also, transformWindowFuncCall() and check_srf_call_placement() should
do the same thing, meaning the work done in f58567105is effectively
reverted.
The attached patch does this. Is this what you have in mind?
--
Sami
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Rework-GRAPH_TABLE-aggregate-window-SRF-rejection.patch | application/octet-stream | 14.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-08-12 22:56:37 | Re: LibreSSL and OpenSSL separation in libpq to support 1.1.1 deprecation |
| Previous Message | Michael Paquier | 2026-08-12 22:34:37 | Re: Split index and table statistics into different types of stats |