Re: queryId constant squashing does not support prepared statements
Junwang Zhao <zhjwpku@gmail.com>
From: Junwang Zhao <zhjwpku@gmail.com>
To: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Sami Imseih <samimseih@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-05-09T03:05:43Z
Lists: pgsql-hackers
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Fix typo in comment
- a3994ec6acb2 18.0 landed
-
Make query jumbling also squash PARAM_EXTERN params
- c2da1a5d6325 18.0 landed
-
Fix squashing algorithm for query texts
- 0f65f3eec478 18.0 landed
-
pg_stat_statements: Fix parameter number gaps in normalized queries
- 3c03b8cd7979 13.22 landed
- 8a1459f62ad1 14.19 landed
- 130300a15407 15.14 landed
- 7e8b44f4e0e6 16.10 landed
- 290e8ab32ac5 17.6 landed
- 35a428f30b15 18.0 landed
Hi Dmitry,
On Fri, May 9, 2025 at 3:36 AM Dmitry Dolgov <9erthalion6@gmail.com> wrote:
>
> > On Thu, May 08, 2025 at 02:22:00PM GMT, Michael Paquier wrote:
> > On Wed, May 07, 2025 at 10:41:22AM +0200, Dmitry Dolgov wrote:
> > > Ah, I see what you mean. I think the idea is fine, it will simplify
> > > certain things as well as address the issue. But I'm afraid adding
> > > start/end location to A_Expr is a bit too invasive, as it's being used
> > > for many other purposes. How about introducing a new expression for this
> > > purposes, and use it only in in_expr/array_expr, and wrap the
> > > corresponding expressions into it? This way the change could be applied
> > > in a more targeted fashion.
> >
> > Yes, that feels invasive. The use of two static variables to track
> > the start and the end positions in an expression list can also be a
> > bit unstable to rely on, I think. It seems to me that this part
> > could be handled in a new Node that takes care of tracking the two
> > positions, instead, be it a start/end couple or a location/length
> > couple? That seems necessary to have when going through
> > jumbleElements().
>
> To clarify, I had in mind something like in the attached patch. The
> idea is to make start/end location capturing relatively independent from
> the constants squashing. The new parsing node conveys the location
> information, which is then getting transformed to be a part of an
> ArrayExpr. It's done for in_expr only here, something similar would be
> needed for array_expr as well. Feedback is appreciated.
+/*
+ * A wrapper expression to record start and end location
+ */
+typedef struct LocationExpr
+{
+ NodeTag type;
+
+ /* the node to be wrapped */
+ Node *expr;
+ /* token location, or -1 if unknown */
+ ParseLoc start_location;
+ /* token location, or -1 if unknown */
+ ParseLoc end_location;
+} LocationExpr;
Why not a location and a length, it should be more natural, it
seems we use this convention in some existing nodes, like
RawStmt, InsertStmt etc.
--
Regards
Junwang Zhao