Re: queryId constant squashing does not support prepared statements

On 2025-Jun-25, Michael Paquier wrote: > On Tue, Jun 24, 2025 at 07:45:15PM +0200, Alvaro Herrera wrote: > > + /* > > +* If we have an external param at this location, but no lists are > > +* being squashed across the query, then we skip here; this will > > make > > +

Re: queryId constant squashing does not support prepared statements

Hello I spent a much longer time staring at this patch than I wanted to, and at a point I almost wanted to boot the whole thing to pg19, but because upthread we already had an agreement that we should get it in for this cycle, I decided that the best course of action was to just move forward with

Re: queryId constant squashing does not support prepared statements

On Tue, Jun 24, 2025 at 07:45:15PM +0200, Alvaro Herrera wrote: > + /* > +* If we have an external param at this location, but no lists are > +* being squashed across the query, then we skip here; this will make > +* us print print the characters found in the original

Re: queryId constant squashing does not support prepared statements

On Thu, Jun 12, 2025 at 11:32 AM Álvaro Herrera wrote: > > Hello, > > I have pushed that now, thanks! > and here's a rebase of patch 0003 to add support > for PARAM_EXTERN. I'm not really sure about this one yet ... see v11. I added a missing test to show how external param normalization behav

Re: queryId constant squashing does not support prepared statements

/*, ... */)| 1 + SELECT * FROM test_squash WHERE id::text = ANY(ARRAY[$1 /*, ... */]) | 1 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 (3 rows) --- neither are prepared statements +-- prepared statements will also be squashed -- the IN

Re: queryId constant squashing does not support prepared statements

On Tue, Jun 10, 2025 at 07:25:27PM +0200, Alvaro Herrera wrote: > On 2025-Jun-10, Michael Paquier wrote: >> I think that this is can be reproduced by >> -DWRITE_READ_PARSE_PLAN_TREES -DCOPY_PARSE_PLAN_TREES >> -DRAW_EXPRESSION_COVERAGE_TEST that I always include in my builds. >> The freebsd task us

Re: queryId constant squashing does not support prepared statements

On 2025-Jun-10, Michael Paquier wrote: > I think that this is can be reproduced by > -DWRITE_READ_PARSE_PLAN_TREES -DCOPY_PARSE_PLAN_TREES > -DRAW_EXPRESSION_COVERAGE_TEST that I always include in my builds. > The freebsd task uses the same with debug_copy_parse_plan_trees=on, > debug_write_read_p

Re: queryId constant squashing does not support prepared statements

On Mon, Jun 09, 2025 at 12:44:59PM +0200, Alvaro Herrera wrote: > I also added a recursive call in IsSquashableExpression to itself. The > check for stack depth can be done without throwing an error. I tested > this by adding stack bloat in that function. I also renamed it to > IsSquashableConst

Re: queryId constant squashing does not support prepared statements

> I've spent a bunch of time looking at this series and here's my take on > the second one. Thanks! > I realized that the whole in_expr production in gram.y is pointless, and > the whole private struct that was added was unnecessary. A much simpler > solution is to remove in_expr, expand its use

Re: queryId constant squashing does not support prepared statements

OM test_squash WHERE id::text = ANY(ARRAY[$1, $2, $3, $4, $5]) | 1 + SELECT pg_stat_statements_reset() IS NOT NULL AS t| 1 +(3 rows) + +-- neither are prepared statements +-- the IN and ARRAY forms of this statement will have the same queryId +SELECT pg_stat_statements_re

Re: queryId constant squashing does not support prepared statements

I realized this thread did not have a CF entry, so here it is https://commitfest.postgresql.org/patch/5801/ -- Sami

Re: queryId constant squashing does not support prepared statements

> The test additions done in v7-0002 look sensible here. > > --- In the following two queries the operator expressions (+) and (@) have > --- different oppno, and will be given different query_id if squashed, even > though > --- the normalized query will be the same > > In v7-0002, this comment is

Re: queryId constant squashing does not support prepared statements

On Thu, May 29, 2025 at 11:30:12AM +0900, Michael Paquier wrote: > I still need to review the rest of the patch series.. The test additions done in v7-0002 look sensible here. --- In the following two queries the operator expressions (+) and (@) have --- different oppno, and will be given differe

Re: queryId constant squashing does not support prepared statements

On Wed, May 28, 2025 at 04:05:03PM -0500, Sami Imseih wrote: > That's my mistake. I added a new file called normalize.sql to test > specific normalization scenarios. Added in v7 Thanks. I was not sure that a new file was worth having for these tests, knowing that select.sql has similar coverage.

Re: queryId constant squashing does not support prepared statements

>> * 0001: > You have mentioned the addition of tests, but v6-0001 includes nothing > of the kind. Am I missing something? How much coverage did you > intend to add here? These seem to be included in squashing.sql in > patch v6-0002, but IMO this should be moved somewhere else to work > with th

Re: queryId constant squashing does not support prepared statements

On Tue, May 27, 2025 at 05:05:39PM -0500, Sami Imseih wrote: > * 0001: > > This is a normalization issue discovered when adding new > tests for squashing. This is also an issue that exists in > v17 and likely earlier versions and should probably be > backpatched. > > The crux of the problem is if

Re: queryId constant squashing does not support prepared statements

> I've spent a bit of time looking at this, and I want to > propose the following patchset. Sorry about this, but I missed to add a comment in one of the test cases for 0004 that describes the behavior of parameters and constants that live outside of the squashed list. The following 2 cases will

Re: queryId constant squashing does not support prepared statements

> > therefore, a user supplied query like this: > > ``` > > select where $5 in ($1, $2, $3) and $6 = $4 and 1 = 2 > > ``` > > > > will be normalized to: > > ``` > > select where $1 in ($2 /*...*/) and $3 = $4 and $5 = $6 > > ``` > > Hmm, interesting. > > I think this renumbering should not be a pro

Re: queryId constant squashing does not support prepared statements

On 2025-May-24, Sami Imseih wrote: > therefore, a user supplied query like this: > ``` > select where $5 in ($1, $2, $3) and $6 = $4 and 1 = 2 > ``` > > will be normalized to: > ``` > select where $1 in ($2 /*...*/) and $3 = $4 and $5 = $6 > ``` Hmm, interesting. I think this renumbering should

Re: queryId constant squashing does not support prepared statements

On Sat, May 24, 2025 at 09:35:24AM -0500, Sami Imseih wrote: > 2. If we have 1 or more squashed lists, then we can't guarantee > the $n parameter as was supplied by the user and we simply rename > the $n starting from 1. > > therefore, a user supplied query like this: > ``` > select where $5 in ($

Re: queryId constant squashing does not support prepared statements

> In v17, we are a bit smarter with the numbering, with a normalization > giving the following, starting at $1: > select where $5 in ($1, $2, $3) and $6 = $4 > > So your argument about the $n parameters is kind of true, but I think > the numbering logic in v17 to start at $1 is a less-confusing res

Re: queryId constant squashing does not support prepared statements

On Fri, May 23, 2025 at 08:05:47PM -0500, Sami Imseih wrote: > Since we assign new parameter symbols based on the highest external param > from the original query, as stated in the docs [0] "The parameter > symbols used to replace > constants in representative query texts start from the next number

Re: queryId constant squashing does not support prepared statements

> On Fri, May 23, 2025 at 04:29:45PM +0200, Dmitry Dolgov wrote: > > I think it's better to recursively call IsSquashableConst on the nested > > expression (arg or args for FuncExpr). Something like that was done in > > the original patch version and was concidered too much at that time, but > > si

Re: queryId constant squashing does not support prepared statements

On Fri, May 23, 2025 at 04:29:45PM +0200, Dmitry Dolgov wrote: > I think it's better to recursively call IsSquashableConst on the nested > expression (arg or args for FuncExpr). Something like that was done in > the original patch version and was concidered too much at that time, but > since it loo

Re: queryId constant squashing does not support prepared statements

> On Fri, May 23, 2025 at 09:05:54AM GMT, Sami Imseih wrote: > > > On Thu, May 22, 2025 at 10:23:31PM GMT, Sami Imseih wrote: > > > > This does not get squashed: > > > > Q: select where 2 in (1, 4) and > > > > 1 in (5, cast(7 as int), 6, (cast(8 as int)), 9, 10, (cast(8 as > > > > text))::int); >

Re: queryId constant squashing does not support prepared statements

> > On Thu, May 22, 2025 at 10:23:31PM GMT, Sami Imseih wrote: > > > This does not get squashed: > > > Q: select where 2 in (1, 4) and > > > 1 in (5, cast(7 as int), 6, (cast(8 as int)), 9, 10, (cast(8 as > > > text))::int); > > > R: select where $1 in ($2 /*, ... */) and > > > $3 in ($4, cast($5

Re: queryId constant squashing does not support prepared statements

> On Thu, May 22, 2025 at 10:23:31PM GMT, Sami Imseih wrote: > > This does not get squashed: > > Q: select where 2 in (1, 4) and > > 1 in (5, cast(7 as int), 6, (cast(8 as int)), 9, 10, (cast(8 as > > text))::int); > > R: select where $1 in ($2 /*, ... */) and > > $3 in ($4, cast($5 as int), $6, (

Re: queryId constant squashing does not support prepared statements

> For example with bind queries like that: > select where $1 in ($3, $2) and 1 in ($4, cast($5 as int)) > \bind 0 1 2 3 4 > > Should we have a bit more coverage, where we use multiple IN and/or > ARRAY lists with constants and/or external parameters? I will add more test coverage. All the tests we

Re: queryId constant squashing does not support prepared statements

On Thu, May 22, 2025 at 03:10:34PM -0500, Sami Imseih wrote: > > IMO adding a struct as suggested is okay, especially if it reduces the > > overall code complexity. But we don't want a node, just a bare struct. > > Adding a node would be more troublesome. > > In v4, a new private struct is added

Re: queryId constant squashing does not support prepared statements

> IMO adding a struct as suggested is okay, especially if it reduces the > overall code complexity. But we don't want a node, just a bare struct. > Adding a node would be more troublesome. In v4, a new private struct is added in gram.y, but we are also adding additional fields to track the expres

Re: queryId constant squashing does not support prepared statements

On 2025-May-22, Dmitry Dolgov wrote: > Just to call this out, I don't think there is an agreement on squashing > Params, which you have added into 0002. Actually I think we do have agreement on squashing PARAM_EXTERN Params. https://postgr.es/m/3086744.1746500...@sss.pgh.pa.us > Now, both flavou

Re: queryId constant squashing does not support prepared statements

> On Wed, May 21, 2025 at 08:22:19PM GMT, Sami Imseih wrote: > > > > At the same time AFAICT there isn't much more code paths > > > > to worry about in case of a LocationExpr as a node > > > > > > I can imagine there are others like value expressions, > > > row expressions, json array expressions,

Re: queryId constant squashing does not support prepared statements

> > > At the same time AFAICT there isn't much more code paths > > > to worry about in case of a LocationExpr as a node > > > > I can imagine there are others like value expressions, > > row expressions, json array expressions, etc. that we may > > want to also normalize. > Exactly. When using a n

Re: queryId constant squashing does not support prepared statements

> On Tue, May 20, 2025 at 04:50:12PM GMT, Sami Imseih wrote: > > At the same time AFAICT there isn't much more code paths > > to worry about in case of a LocationExpr as a node > > I can imagine there are others like value expressions, > row expressions, json array expressions, etc. that we may > w

Re: queryId constant squashing does not support prepared statements

> At the same time AFAICT there isn't much more code paths > to worry about in case of a LocationExpr as a node I can imagine there are others like value expressions, row expressions, json array expressions, etc. that we may want to also normalize. > I believe it's worth to not only to keep amoun

Re: queryId constant squashing does not support prepared statements

> On Tue, May 20, 2025 at 06:30:25AM GMT, Michael Paquier wrote: > On Mon, May 12, 2025 at 06:40:43PM -0400, Sami Imseih wrote: > > Also, LocationExpr is not really an expression node, but a wrapper to > > an expression node, so I think it's wrong to define it as a Node and be > > required to add t

Re: queryId constant squashing does not support prepared statements

> On Tue, May 20, 2025 at 11:03:52AM GMT, Dmitry Dolgov wrote: > > By the way, the new test cases for ARRAY lists are sent in the last > > patch of the series posted on this thread: > > https://www.postgresql.org/message-id/7zbzwk4btnxoo4o3xbtzefoqvht54cszjj4bol22fmej5nmgkf@dbcn4wtakw4y > > > > The

Re: queryId constant squashing does not support prepared statements

On Mon, May 12, 2025 at 06:40:43PM -0400, Sami Imseih wrote: > Also, LocationExpr is not really an expression node, but a wrapper to > an expression node, so I think it's wrong to define it as a Node and be > required to add the necessary handling for it in nodeFuncs.c. I think we > can just define

Re: queryId constant squashing does not support prepared statements

> On Mon, May 12, 2025 at 06:40:43PM GMT, Sami Imseih wrote: > > The static variables was only part of the concern, another part was > > using A_Expr to carry this information, which will have impact on lots > > of unrelated code. > > What would be the problem if A_Expr carries an extra pointer to

Re: queryId constant squashing does not support prepared statements

> > On Fri, May 09, 2025 at 12:47:19PM GMT, Sami Imseih wrote: > > So, I think we can create a new parse node ( parsenode.h ) that will only be > > used in parsing (and gram.c only ) to track the start/end locations > > and List and > > based on this node we can create A_ArrayExpr and A_Expr with t

Re: queryId constant squashing does not support prepared statements

> On Fri, May 09, 2025 at 12:47:19PM GMT, Sami Imseih wrote: > So, I think we can create a new parse node ( parsenode.h ) that will only be > used in parsing (and gram.c only ) to track the start/end locations > and List and > based on this node we can create A_ArrayExpr and A_Expr with the List >

Re: queryId constant squashing does not support prepared statements

> On Fri, May 09, 2025 at 10:12:24AM GMT, Dmitry Dolgov wrote: > Agree, I'll try to extend number of test cases here as a separate patch. Here is the extended version, where start/end is replaced by location/length, array_expr is handled as well, and more ARRAY cases are added. >From 81fe0b08473ea

Re: queryId constant squashing does not support prepared statements

> > 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 > > Array

Re: queryId constant squashing does not support prepared statements

> On Fri, May 09, 2025 at 08:47:58AM GMT, Michael Paquier wrote: > SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; > - query| calls > -+--- > - SELECT ARRAY[$1 /*, ... */]

Re: queryId constant squashing does not support prepared statements

> On Fri, May 09, 2025 at 02:35:33PM GMT, Michael Paquier wrote: > On Fri, May 09, 2025 at 11:05:43AM +0800, Junwang Zhao wrote: > > 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. > > These ar

Re: queryId constant squashing does not support prepared statements

On Fri, May 9, 2025 at 1:35 PM Michael Paquier wrote: > > On Fri, May 09, 2025 at 11:05:43AM +0800, Junwang Zhao wrote: > > 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. > > These are new co

Re: queryId constant squashing does not support prepared statements

On Fri, May 09, 2025 at 11:05:43AM +0800, Junwang Zhao wrote: > 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. These are new concepts as of Postgres 18 (aka only on HEAD), chosen mainly to match

Re: queryId constant squashing does not support prepared statements

Hi Dmitry, On Fri, May 9, 2025 at 3:36 AM Dmitry Dolgov <9erthali...@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 >

Re: queryId constant squashing does not support prepared statements

On Thu, May 08, 2025 at 03:50:32PM -0500, Sami Imseih wrote: > On Thu, May 8, 2025 at 2:36 PM Dmitry Dolgov <9erthali...@gmail.com> wrote: >> 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 consta

Re: queryId constant squashing does not support prepared statements

On Thu, May 8, 2025 at 2:36 PM Dmitry Dolgov <9erthali...@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

Re: queryId constant squashing does not support prepared statements

> 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

Re: queryId constant squashing does not support prepared statements

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 purpose

Re: queryId constant squashing does not support prepared statements

> On Tue, May 06, 2025 at 03:01:32PM GMT, Sami Imseih wrote: > > > Without properly accounting for the boundaries of the list of > > > expressions, i.e., > > > the start and end positions of '(' and ')' or '[' and ']' and normalizing > > > the > > > expressions in between, it will be very difficu

Re: queryId constant squashing does not support prepared statements

On Tue, May 06, 2025 at 01:32:48PM -0500, Sami Imseih wrote: > Without properly accounting for the boundaries of the list of expressions, > i.e., > the start and end positions of '(' and ')' or '[' and ']' and normalizing the > expressions in between, it will be very difficult for the normalizatio

Re: queryId constant squashing does not support prepared statements

> > Without properly accounting for the boundaries of the list of expressions, > > i.e., > > the start and end positions of '(' and ')' or '[' and ']' and normalizing > > the > > expressions in between, it will be very difficult for the normalization to > > behave sanely. > > I don't think having

Re: queryId constant squashing does not support prepared statements

> On Tue, May 06, 2025 at 01:32:48PM GMT, Sami Imseih wrote: > > I also agree with Alvaro that this discussion doesn't justify a > > revert. If the pre-v18 behavior wasn't chiseled on stone tablets, > > the new behavior isn't either. We can improve it some more later. > > As I was looking further

Re: queryId constant squashing does not support prepared statements

> I also agree with Alvaro that this discussion doesn't justify a > revert. If the pre-v18 behavior wasn't chiseled on stone tablets, > the new behavior isn't either. We can improve it some more later. As I was looking further into what we currently have in v18 and HEAD the normalization could b

Re: queryId constant squashing does not support prepared statements

> On Tue, May 06, 2025 at 11:50:07PM GMT, Junwang Zhao wrote: > Would it make sense to rename `RecordConstLocation` to something like > `RecordExpressionLocation` instead? Yeah, naming is hard. RecordExpressionLocation is somehow more vague, but I see what you mean, maybe something along these lin

Re: queryId constant squashing does not support prepared statements

Hi Dmitry, On Sun, May 4, 2025 at 6:19 PM Dmitry Dolgov <9erthali...@gmail.com> wrote: > > > On Thu, May 01, 2025 at 09:55:47PM GMT, Dmitry Dolgov wrote: > > > On Thu, May 01, 2025 at 09:29:13AM GMT, Michael Paquier wrote: > > > > > > I agree that the current solution we have in the tree feels inc

Re: queryId constant squashing does not support prepared statements

Michael Paquier writes: > On Thu, May 01, 2025 at 03:57:16PM -0500, Sami Imseih wrote: >> I think what we should really allow the broader scope of expressions that >> are allowed via prepared statements, and this will make this implementation >> consistent between pre

Re: queryId constant squashing does not support prepared statements

er > interface doesn't change. > FWIW, i'm +1 on leaving it in pg18. Prepared statements often look a little different in other ways, and there are a bunch of other quirks in how queryid's are calculated too. Didn't there used to be something with CALL being hand

Re: queryId constant squashing does not support prepared statements

> On Thu, May 01, 2025 at 09:55:47PM GMT, Dmitry Dolgov wrote: > > On Thu, May 01, 2025 at 09:29:13AM GMT, Michael Paquier wrote: > > > > I agree that the current solution we have in the tree feels incomplete > > because we are not taking into account the most common cases that > > users would care

Re: queryId constant squashing does not support prepared statements

On 2025-May-02, Michael Paquier wrote: > That depends. If we conclude that tracking this information through > the parser based on the start and end positions in a query string > for a set of values is more relevant, then we would be redesigning the > facility from the ground, so the old approach

Re: queryId constant squashing does not support prepared statements

> On Fri, May 02, 2025 at 04:18:37PM GMT, Michael Paquier wrote: > On Fri, May 02, 2025 at 09:13:39AM +0200, Dmitry Dolgov wrote: > > Squashing constants was ment to be a first step towards doing the same > > for other types of queries (params, rte_values), reverting it to > > implement everything

Re: queryId constant squashing does not support prepared statements

On Fri, May 02, 2025 at 09:13:39AM +0200, Dmitry Dolgov wrote: > Squashing constants was ment to be a first step towards doing the same > for other types of queries (params, rte_values), reverting it to > implement everything at once makes very little sense to me. That depends. If we conclude tha

Re: queryId constant squashing does not support prepared statements

> On Fri, May 02, 2025 at 07:10:19AM GMT, Michael Paquier wrote: > > I am really leaning towards that we should revert this feature as the > > limitation we have now with parameters is a rather large one and I think > > we need to go back and address this issue. > > I am wondering if this would not

Re: queryId constant squashing does not support prepared statements

On Thu, May 01, 2025 at 03:57:16PM -0500, Sami Imseih wrote: > I think what we should really allow the broader scope of expressions that > are allowed via prepared statements, and this will make this implementation > consistent between prepared vs non-prepared statements. I don't see

Re: queryId constant squashing does not support prepared statements

eters are different in some way. I don't think limiting this feature to Const only will suffice. I think what we should really allow the broader scope of expressions that are allowed via prepared statements, and this will make this implementation consistent between prepared vs non-prepared statemen

Re: queryId constant squashing does not support prepared statements

> On Thu, May 01, 2025 at 09:29:13AM GMT, Michael Paquier wrote: > > I agree that the current solution we have in the tree feels incomplete > because we are not taking into account the most common cases that > users would care about. Now, allowing PARAM_EXTERN means that we > allow any expression

Re: queryId constant squashing does not support prepared statements

the IN-LIST. > > The patch lacks the ability to apply this optimization to values > passed in as parameters ( i.e. parameter kind = PARAM_EXTERN ) > which will be the case for SQL prepared statements and protocol level > prepared statements, i.e. > > I think this is a pre

queryId constant squashing does not support prepared statements

passed in as parameters ( i.e. parameter kind = PARAM_EXTERN ) which will be the case for SQL prepared statements and protocol level prepared statements, i.e. ``` select from t where id in (1, 2, 3) \bind ``` or ``` prepare prp(int, int, int) as select from t where id in ($1, $2, $3); ``` Here is

Re: Bug in pgbench prepared statements

On Fri, Dec 01, 2023 at 07:06:40PM -0800, Lev Kokotov wrote: > Attached. PR against master also here > , just to make sure it's > mergeable . Thanks for the updated patch. It looks sensible seen fr

Re: Bug in pgbench prepared statements

> The patch you have sent does not apply cleanly on the master branch, > could you rebase please? Attached. PR against master also here , just to make sure it's mergeable . > Wouldn't it > better t

Re: Bug in pgbench prepared statements

On Thu, Nov 30, 2023 at 07:15:54PM -0800, Lev Kokotov wrote: >> I see prepareCommand() is called one more time in >> prepareCommandsInPipeline(). Should you also check the return value >> there? > > Yes, good catch. New patch attached. Agreed that this is not really helpful as it stands >> It ma

Re: Bug in pgbench prepared statements

> I see prepareCommand() is called one more time in > prepareCommandsInPipeline(). Should you also check the return value > there? Yes, good catch. New patch attached. > It may also be useful to throw this patch on the January commitfest if > no one else comes along to review/commit it. First ti

Re: Bug in pgbench prepared statements

On Wed Nov 29, 2023 at 7:38 PM CST, Lev Kokotov wrote: Patch attached, if there is any interest in fixing this small bug. I see prepareCommand() is called one more time in prepareCommandsInPipeline(). Should you also check the return value there? It may also be useful to throw this patch on

Bug in pgbench prepared statements

Hi, I noticed something that looks like a bug in pgbench when using the prepared protocol. pgbench assumes that all prepared statements are prepared correctly, even if they contain errors (e.g. syntax, column/table doesn't exist, etc.). My test script is just: SELECT one; The output

Re: Deleting prepared statements from libpq.

On Tue, Jul 04, 2023 at 04:09:43PM +0900, Michael Paquier wrote: > On Tue, Jul 04, 2023 at 08:28:40AM +0900, Michael Paquier wrote: >> Sure, feel free. I was planning to look at and play more with it. > > Well, done. For the sake of completeness, as I forgot to send my notes. + if (PQsendClose

Re: Deleting prepared statements from libpq.

On Tue, Jul 04, 2023 at 08:28:40AM +0900, Michael Paquier wrote: > Sure, feel free. I was planning to look at and play more with it. Well, done. -- Michael signature.asc Description: PGP signature

Re: Deleting prepared statements from libpq.

On Mon, Jul 03, 2023 at 02:33:55PM +0200, Jelte Fennema wrote: > @Michael is anything else needed from my side? If not, I'll mark the > commitfest entry as "Ready For Committer". Sure, feel free. I was planning to look at and play more with it. -- Michael signature.asc Description: PGP signatur

Re: Deleting prepared statements from libpq.

@Michael is anything else needed from my side? If not, I'll mark the commitfest entry as "Ready For Committer".

Re: Deleting prepared statements from libpq.

On Fri, Jun 23, 2023 at 09:39:00AM +0200, Jelte Fennema wrote: > To be clear, it didn't actually change the behaviour. I only changed > the error message, since it said the exact opposite of what it was > expecting. I split this minor fix into its own commit now to clarify > that. I think it would

Re: Deleting prepared statements from libpq.

On Fri, 23 Jun 2023 at 05:59, Michael Paquier wrote: > [...] > res = PQgetResult(conn); > if (res == NULL) > - pg_fatal("expected NULL result"); > + pg_fatal("expected non-NULL result"); > > This should check for the NULL-ness of the result returned for > PQclosePrepared() rath

Re: Deleting prepared statements from libpq.

On Tue, Jun 20, 2023 at 01:42:13PM +0200, Jelte Fennema wrote: Thanks for updating the patch. > On Tue, 20 Jun 2023 at 06:18, Michael Paquier wrote: >> The amount of duplication between the describe and close paths >> concerns me a bit. Should PQsendClose() and PQsendDescribe() be >> merged int

Re: Deleting prepared statements from libpq.

On Tue, 20 Jun 2023 at 06:18, Michael Paquier wrote: > The amount of duplication between the describe and close paths > concerns me a bit. Should PQsendClose() and PQsendDescribe() be > merged into a single routine (say PQsendCommand), that uses a message > type for pqPutMsgStart and a queryclass

Re: Deleting prepared statements from libpq.

On Mon, Jun 19, 2023 at 02:49:44PM +0200, Jelte Fennema wrote: > On Mon, 19 Jun 2023 at 14:17, jian he wrote: >> I am not sure the following two following function comments are right > > They were incorrect indeed. Attached is a patch with those two updated. The amount of duplication between

Re: Deleting prepared statements from libpq.

ma wrote: > > > > On Mon, 19 Jun 2023 at 11:44, Jelte Fennema wrote: > > > Done > > > > Now with the actual attachment. > > > > PS. Another connection pooler (PgCat) now also supports prepared > > statements, but only using Close not DEALLOCATE

Re: Deleting prepared statements from libpq.

On Mon, Jun 19, 2023 at 5:50 PM Jelte Fennema wrote: > > On Mon, 19 Jun 2023 at 11:44, Jelte Fennema wrote: > > Done > > Now with the actual attachment. > > PS. Another connection pooler (PgCat) now also supports prepared > statements, but only using Cl

Re: Deleting prepared statements from libpq.

On Mon, 19 Jun 2023 at 11:44, Jelte Fennema wrote: > Done Now with the actual attachment. PS. Another connection pooler (PgCat) now also supports prepared statements, but only using Close not DEALLOCATE: https://postgresml.org/blog/making-postgres-30-percent-faster-in-production F

Re: Deleting prepared statements from libpq.

On Mon, 19 Jun 2023 at 04:52, jian he wrote: > > /* Now that it's closed we should get an error when describing */ > > res = PQdescribePortal(conn, "cursor_one"); > > if (PQresultStatus(res) != PGRES_FATAL_ERROR) > > pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res))); > shou

Re: Deleting prepared statements from libpq.

On Mon, 19 Jun 2023 at 01:57, Michael Paquier wrote: > +static int > +PQsendClose(PGconn *conn, char close_type, const char *close_target) > > Could it be better for this code path to issue an error if using a > non-supported close_type rather than sending it? Okay, you are > consistent with desc

Re: Deleting prepared statements from libpq.

now it works. /src/test/modules/libpq_pipeline/libpq_pipeline.c > > /* Now that it's closed we should get an error when describing */ > res = PQdescribePortal(conn, "cursor_one"); > if (PQresultStatus(res) != PGRES_FATAL_ERROR) > pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(r

Re: Deleting prepared statements from libpq.

On Sun, Jun 18, 2023 at 01:03:57PM +0200, Jelte Fennema wrote: > Sorry about that. I attached a new patch that allows linking to the > new functions (I forgot to add the functions to exports.txt). This new > patch also adds some basic tests for these new functions. I am okay with the arguments abo

Re: Deleting prepared statements from libpq.

On Sun, Jun 18, 2023 at 09:23:22PM +0800, jian he wrote: > previously I cannot link it. with > v2-0001-Support-sending-Close-messages-from-libpq.patch. now I can > compile it, link it, but then run time error. > same c program in the first email. > when I run it ./a.out, then error: > ./a.out: symb

Re: Deleting prepared statements from libpq.

On Sun, Jun 18, 2023 at 7:04 PM Jelte Fennema wrote: > > On Sat, 17 Jun 2023 at 15:34, jian he wrote: > > I failed to link it. I don't know why. > > Sorry about that. I attached a new patch that allows linking to the > new functions (I forgot to add the functions to exports.txt). This new > patch

Re: Deleting prepared statements from libpq.

On Sat, 17 Jun 2023 at 15:34, jian he wrote: > I failed to link it. I don't know why. Sorry about that. I attached a new patch that allows linking to the new functions (I forgot to add the functions to exports.txt). This new patch also adds some basic tests for these new functions. v2-0001-Supp

Re: Deleting prepared statements from libpq.

On Fri, Jun 16, 2023 at 11:28 PM Jelte Fennema wrote: > > On Fri, 16 Jun 2023 at 16:26, Craig Ringer wrote: > > Nobody's implemented it. > > > > A patch to add PQclosePrepared and PQsendClosePrepared would be welcome. At > > least, I think so... > > This might have been a pretty old thread. But

Re: Deleting prepared statements from libpq.

On Fri, 16 Jun 2023 at 16:26, Craig Ringer wrote: > Nobody's implemented it. > > A patch to add PQclosePrepared and PQsendClosePrepared would be welcome. At > least, I think so... This might have been a pretty old thread. But I just took it upon me to implement these functions (or well I mostly

Re: MERGE and parsing with prepared statements

On 2022-Aug-12, Simon Riggs wrote: > Sorry, but I disagree with this chunk in the latest commit, > specifically, changing the MATCHED from after to before the NOT > MATCHED clause. > > The whole point of the second example was to demonstrate that the > order of the MATCHED/NOT MATCHED clauses mad

Re: MERGE and parsing with prepared statements

On Fri, Aug 12, 2022 at 01:53:25PM +0200, Alvaro Herrera wrote: > On 2022-Aug-12, Simon Riggs wrote: > > > Sorry, but I disagree with this chunk in the latest commit, > > specifically, changing the MATCHED from after to before the NOT > > MATCHED clause. 3d895bc84 also moved a semicolon into the

  1   2   >