Re: psql FETCH_COUNT feature does not work with combined queries
On Thu, Jul 16, 2020 at 04:44:47PM -0700, David G. Johnston wrote: > When FETCH_COUNT is non-zero, and you send a multi-statement command > to the server (for example via -c or the \; meta-command), the results > are undefined. This combination is presently allowed for backward > compatibility. This has been waiting on author for two months now, so I have marked the patch as RwF in the CF app. -- Michael signature.asc Description: PGP signature
Re: psql FETCH_COUNT feature does not work with combined queries
On Thu, Jul 16, 2020 at 4:24 PM Alvaro Herrera wrote: > On 2020-Jul-16, David Johnston wrote: > > > Instead of a note maybe add a paragraph stating: > > > > "This setting is ignored when multiple statements are sent to the server > as a single command (i.e., via the -c command line option or the \; > meta-command). Additionally, it is possible for certain combinations of > statements sent in that manner to instead return no results, or even be > ignored, instead of returning the result set of the last query. In short, > when FETCH_COUNT is non-zero, and you send a multi-statement command to the > server, the results are undefined. This combination in presently allowed > for backward compatibility." > > I personally dislike documenting things that don't work, if worded in a > way that don't leave open the possibility of fixing it in the future. > So I didn't like Fabien's original wording either, though I was thinking > that just adding "This may change in a future release of Postgres" might > have been enough. That seems more difficult with your proposed wording, > but maybe you see a good way to do it? > > After rereading it a few times, I think it's too specific about how it > fails. Maybe it's possible to reduce to just the last two phrases, > along the lines of > > > When FETCH_COUNT is non-zero, and you send a multi-statement command > > to the server (for example via -c or the \; meta-command), the results > > are undefined. This combination in presently allowed for backward > > compatibility and may be changed in a future release. > > Everything may change in a future release so I am generally opposed to including that. But I'm ok with being less descriptive on the observed failure modes and sweeping it all under "undefined". Need to fix my typo, "This combination is presently allowed". When FETCH_COUNT is non-zero, and you send a multi-statement command to the server (for example via -c or the \; meta-command), the results are undefined. This combination is presently allowed for backward compatibility. David J.
Re: psql FETCH_COUNT feature does not work with combined queries
On 2020-Jul-16, David Johnston wrote: > Instead of a note maybe add a paragraph stating: > > "This setting is ignored when multiple statements are sent to the server as a > single command (i.e., via the -c command line option or the \; meta-command). > Additionally, it is possible for certain combinations of statements sent in > that manner to instead return no results, or even be ignored, instead of > returning the result set of the last query. In short, when FETCH_COUNT is > non-zero, and you send a multi-statement command to the server, the results > are undefined. This combination in presently allowed for backward > compatibility." I personally dislike documenting things that don't work, if worded in a way that don't leave open the possibility of fixing it in the future. So I didn't like Fabien's original wording either, though I was thinking that just adding "This may change in a future release of Postgres" might have been enough. That seems more difficult with your proposed wording, but maybe you see a good way to do it? After rereading it a few times, I think it's too specific about how it fails. Maybe it's possible to reduce to just the last two phrases, along the lines of > When FETCH_COUNT is non-zero, and you send a multi-statement command > to the server (for example via -c or the \; meta-command), the results > are undefined. This combination in presently allowed for backward > compatibility and may be changed in a future release. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: psql FETCH_COUNT feature does not work with combined queries
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:tested, passed Fabien, There is a minor typo (works -> work) but overall I'm not a fan of the wording. Instead of a note maybe add a paragraph stating: "This setting is ignored when multiple statements are sent to the server as a single command (i.e., via the -c command line option or the \; meta-command). Additionally, it is possible for certain combinations of statements sent in that manner to instead return no results, or even be ignored, instead of returning the result set of the last query. In short, when FETCH_COUNT is non-zero, and you send a multi-statement command to the server, the results are undefined. This combination in presently allowed for backward compatibility." I would suggest, however, adding some tests in src/test/psql.sql demonstrating the broken behavior. A potentially useful test setup would be: create temp table testtbl (idx int, div int); insert into testtbl (1,1), (2,1),(3,1),(4,0),(5,1); and combine that with FETCH_COUNT 3 Some other things I tried, with and without FETCH_COUNT: begin \; select 2 \; commit \; select 1 / div from (select div from testtbl order by idx) as src; select 1/0 \; select 1 / div from (select div from testtbl where div <> 0 order by idx) as src; begin \; select 2 \; select 1 \; commit; begin \; select 2 \; select 1; commit; I'm not sure how to go about getting a equivalent result of "select pg_sleep(2) \; select 1;" not sleeping. If I put select 1/0 instead of pg_sleep I get an error and any DML seems to end up working just fine (minus actual batch fetching) update testtbl set val = 2 \; select 1; --result (1) David J. The new status of this patch is: Waiting on Author
Re: psql FETCH_COUNT feature does not work with combined queries
Assuming there's no one willing to fix the behavior (and that seems unlikely given we're in the middle of a 2020-01 commitfest) it makes sense to at least document the behavior. That being said, I think the proposed patch may be a tad too brief. I don't think we need to describe the exact behavior, of course, but maybe it'd be good to mention that the behavior depends on the type of the last command etc. Also, I don't think "combined query" is a term used anywhere in the code/docs - I think the proper term is "multi-query string". Any thoughts on Tomas' comments? Sorry, I did not notice them. I tried "multi-command", matching some wording use elsewhere in the file. I'm at lost about how to describe the insane behavior the feature has when they are used, which is really just an unfixed bug, so I expanded a minima in the attached v2. -- Fabien.diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 402c4b1b26..b4a9fc3b60 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -3781,6 +3781,14 @@ bar fail after having already displayed some rows. + + +This feature does not works properly with multi-command +(\;) queries as it depends on the command +types encountered in the string. + + + Although you can use any output format with this feature,
Re: psql FETCH_COUNT feature does not work with combined queries
Hi Fabien, On 1/6/20 5:20 PM, Tomas Vondra wrote: On Fri, Jul 26, 2019 at 04:13:23PM +, Fabien COELHO wrote: FETCH_COUNT does not work with combined queries, and probably has never worked since 2006. For the record, this bug has already been reported & discussed by Daniel Vérité a few months back, see: https://www.postgresql.org/message-id/flat/a0a854b6-563c-4a11-bf1c-d6c6f924004d%40manitou-mail.org Given the points of view expressed on this thread, especially Tom's view against improving the lexer used by psql to known where query ends, it looks that my documentation patch is the way to go in the short term, and that if the "always show query results" patch is committed, it might simplify a bit solving this issue as the PQexec call is turned into PQsendQuery. Assuming there's no one willing to fix the behavior (and that seems unlikely given we're in the middle of a 2020-01 commitfest) it makes sense to at least document the behavior. That being said, I think the proposed patch may be a tad too brief. I don't think we need to describe the exact behavior, of course, but maybe it'd be good to mention that the behavior depends on the type of the last command etc. Also, I don't think "combined query" is a term used anywhere in the code/docs - I think the proper term is "multi-query string". Any thoughts on Tomas' comments? I'll mark this Waiting on Author because I feel like some response and/or a new patch is needed. Regards, -- -David da...@pgmasters.net
Re: psql FETCH_COUNT feature does not work with combined queries
On Fri, Jul 26, 2019 at 04:13:23PM +, Fabien COELHO wrote: FETCH_COUNT does not work with combined queries, and probably has never worked since 2006. For the record, this bug has already been reported & discussed by Daniel Vérité a few months back, see: https://www.postgresql.org/message-id/flat/a0a854b6-563c-4a11-bf1c-d6c6f924004d%40manitou-mail.org Given the points of view expressed on this thread, especially Tom's view against improving the lexer used by psql to known where query ends, it looks that my documentation patch is the way to go in the short term, and that if the "always show query results" patch is committed, it might simplify a bit solving this issue as the PQexec call is turned into PQsendQuery. Assuming there's no one willing to fix the behavior (and that seems unlikely given we're in the middle of a 2020-01 commitfest) it makes sense to at least document the behavior. That being said, I think the proposed patch may be a tad too brief. I don't think we need to describe the exact behavior, of course, but maybe it'd be good to mention that the behavior depends on the type of the last command etc. Also, I don't think "combined query" is a term used anywhere in the code/docs - I think the proper term is "multi-query string". regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: psql FETCH_COUNT feature does not work with combined queries
FETCH_COUNT does not work with combined queries, and probably has never worked since 2006. For the record, this bug has already been reported & discussed by Daniel Vérité a few months back, see: https://www.postgresql.org/message-id/flat/a0a854b6-563c-4a11-bf1c-d6c6f924004d%40manitou-mail.org Given the points of view expressed on this thread, especially Tom's view against improving the lexer used by psql to known where query ends, it looks that my documentation patch is the way to go in the short term, and that if the "always show query results" patch is committed, it might simplify a bit solving this issue as the PQexec call is turned into PQsendQuery. -- Fabien.
psql FETCH_COUNT feature does not work with combined queries
Hello devs, As pointed out by Kyotaro Horiguchi in https://www.postgresql.org/message-id/20190726.131704.86173346.horikyota@gmail.com FETCH_COUNT does not work with combined queries, and probably has never worked since 2006. What seems to happen is that ExecQueryUsingCursor is hardcoded to handle one simple query. It simply inserts the cursor generation in front of the query believed to be a select: DECLARE ... For combined queries, say two selects, it results in: DECLARE ... ; Then PQexec returns the result of the second one, and nothing is printed. However, if the second query is not a select, eg: "select ... \; update ... ;", the result of the *first* query is shown. How fun! This is because PQexec returns the second result. The cursor declaration expects a PGRES_COMMAND_OK before proceeding. With a select it gets PGRES_TUPLES_OK so decides it is an error and silently skips to the end. With the update it indeed obtains the expected PGRES_COMMAND_OK, not really for the command it sent but who cares, and proceeds to show the cursor results. Basically, the whole logic is broken. The minimum is to document that it does not work properly with combined queries. Attached patch does that, so that the bug becomes a documented bug, aka a feature:-) Otherwise, probably psql lexer could detect, with some efforts, that it is a combined query (detect embedded ; and check that they are not empty queries), so that it could skip the feature if it is the case. Another approach would be to try to detect if the returned result does not correspond to the cursor one reliably. Maybe some result counting could be added somewhere so that the number of results under PQexec is accessible to the user, i.e. result struct would contain its own number. Hmmm. A more complex approach would be to keep the position of embedded queries, and to insert cursor declarations where needed, currently the last one if it is a SELECT. However, for the previous ones the allocation and such could be prohibitive as no cursor would be used. Not sure it is worth the effort as the bug has not been detected for 13 years. -- Fabien.diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 7789fc6177..d217d82f57 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -3684,6 +3684,12 @@ bar fail after having already displayed some rows. + + +This feature does not work properly with combined queries. + + + Although you can use any output format with this feature,
