Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Applied. Thanks. --- Bruce Momjian wrote: Boszormenyi Zoltan wrote: Ah, I didn't even see that that section needed to be updated. Good catch. I'd suggest the following wording: For a commandSELECT/command or commandCREATE TABLE AS/command command, the tag is SELECT rows... [and the rest as you have it] We should probably also retitle that section from Retrieving Result Information for Other Commands to Retrieving Other Result Information and adjust the text of the opening sentence similarly. Also I think you need to update the docs for PQcmdtuples to mention that it not works for SELECT and CTAS. Ok, I will update libpq.sgml where this section resides. What's a CTA, btw? Do you mean CTE, a.k.a. Common Table Expression? Sorry, CTAS = CREATE TABLE AS SELECT. Okay, new patch is attached. Please read the docs changes, and comment. I have reviewed this patch and made some adjustments, attached. The major change is that our return of 0 0 in certain cases must remain, though I have improved the C comment explaining it with a separate CVS commit: /* * If a command completion tag was supplied, use it. Otherwise use the * portal's commandTag as the default completion tag. * * Exception: Clients expect INSERT/UPDATE/DELETE tags to have * counts, so fake them with zeros. This can happen with DO INSTEAD * rules if there is no replacement query of the same type as the * original. We print 0 0 here because technically there is no * query of the matching tag type, and printing a non-zero count for * a different query type seems wrong, e.g. an INSERT that does * an UPDATE instead should not print 0 1 if one row * was updated. See QueryRewrite(), step 3, for details. */ I have removed the part of the patch that chagned 0 0; it seems to run fine without it. The rest of my adjustments were minor. One major part of the patch seems to be the collection of the PORTAL_ONE_SELECT switch label into the label below it, which is a nice cleanup. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/libpq.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v retrieving revision 1.297 diff -c -c -r1.297 libpq.sgml *** doc/src/sgml/libpq.sgml 5 Feb 2010 03:09:04 - 1.297 --- doc/src/sgml/libpq.sgml 14 Feb 2010 03:11:00 - *** *** 2869,2880 /sect2 sect2 id=libpq-exec-nonselect !titleRetrieving Result Information for Other Commands/title para ! These functions are used to extract information from ! structnamePGresult/structname objects that are not ! commandSELECT/ results. /para variablelist --- 2869,2879 /sect2 sect2 id=libpq-exec-nonselect !titleRetrieving Other Result Information/title para ! These functions are used to extract other information from ! structnamePGresult/structname objects. /para variablelist *** *** 2925,2936 This function returns a string containing the number of rows affected by the acronymSQL/ statement that generated the structnamePGresult/. This function can only be used following !the execution of an commandINSERT/, commandUPDATE/, !commandDELETE/, commandMOVE/, commandFETCH/, or !commandCOPY/ statement, or an commandEXECUTE/ of a !prepared query that contains an commandINSERT/, !commandUPDATE/, or commandDELETE/ statement. If the !command that generated the structnamePGresult/ was anything else, functionPQcmdTuples/ returns an empty string. The caller should not free the return value directly. It will be freed when the associated structnamePGresult/ handle is passed to --- 2924,2935 This function returns a string containing the number of rows affected by the acronymSQL/ statement that generated the structnamePGresult/. This function can only be used following !the execution of a commandSELECT/, commandCREATE TABLE AS/, !commandINSERT/, commandUPDATE/, commandDELETE/, !commandMOVE/, commandFETCH/, or commandCOPY/ statement, !or an commandEXECUTE/ of a prepared query that contains an !commandINSERT/, commandUPDATE/, or commandDELETE/ statement. !If the command that generated the structnamePGresult/ was anything else, functionPQcmdTuples/ returns an empty string. The caller should not free the return value directly. It
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Boszormenyi Zoltan wrote: Ah, I didn't even see that that section needed to be updated. Good catch. I'd suggest the following wording: For a commandSELECT/command or commandCREATE TABLE AS/command command, the tag is SELECT rows... [and the rest as you have it] We should probably also retitle that section from Retrieving Result Information for Other Commands to Retrieving Other Result Information and adjust the text of the opening sentence similarly. Also I think you need to update the docs for PQcmdtuples to mention that it not works for SELECT and CTAS. Ok, I will update libpq.sgml where this section resides. What's a CTA, btw? Do you mean CTE, a.k.a. Common Table Expression? Sorry, CTAS = CREATE TABLE AS SELECT. Okay, new patch is attached. Please read the docs changes, and comment. I have reviewed this patch and made some adjustments, attached. The major change is that our return of 0 0 in certain cases must remain, though I have improved the C comment explaining it with a separate CVS commit: /* * If a command completion tag was supplied, use it. Otherwise use the * portal's commandTag as the default completion tag. * * Exception: Clients expect INSERT/UPDATE/DELETE tags to have * counts, so fake them with zeros. This can happen with DO INSTEAD * rules if there is no replacement query of the same type as the * original. We print 0 0 here because technically there is no * query of the matching tag type, and printing a non-zero count for * a different query type seems wrong, e.g. an INSERT that does * an UPDATE instead should not print 0 1 if one row * was updated. See QueryRewrite(), step 3, for details. */ I have removed the part of the patch that chagned 0 0; it seems to run fine without it. The rest of my adjustments were minor. One major part of the patch seems to be the collection of the PORTAL_ONE_SELECT switch label into the label below it, which is a nice cleanup. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/libpq.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v retrieving revision 1.297 diff -c -c -r1.297 libpq.sgml *** doc/src/sgml/libpq.sgml 5 Feb 2010 03:09:04 - 1.297 --- doc/src/sgml/libpq.sgml 14 Feb 2010 03:11:00 - *** *** 2869,2880 /sect2 sect2 id=libpq-exec-nonselect !titleRetrieving Result Information for Other Commands/title para ! These functions are used to extract information from ! structnamePGresult/structname objects that are not ! commandSELECT/ results. /para variablelist --- 2869,2879 /sect2 sect2 id=libpq-exec-nonselect !titleRetrieving Other Result Information/title para ! These functions are used to extract other information from ! structnamePGresult/structname objects. /para variablelist *** *** 2925,2936 This function returns a string containing the number of rows affected by the acronymSQL/ statement that generated the structnamePGresult/. This function can only be used following !the execution of an commandINSERT/, commandUPDATE/, !commandDELETE/, commandMOVE/, commandFETCH/, or !commandCOPY/ statement, or an commandEXECUTE/ of a !prepared query that contains an commandINSERT/, !commandUPDATE/, or commandDELETE/ statement. If the !command that generated the structnamePGresult/ was anything else, functionPQcmdTuples/ returns an empty string. The caller should not free the return value directly. It will be freed when the associated structnamePGresult/ handle is passed to --- 2924,2935 This function returns a string containing the number of rows affected by the acronymSQL/ statement that generated the structnamePGresult/. This function can only be used following !the execution of a commandSELECT/, commandCREATE TABLE AS/, !commandINSERT/, commandUPDATE/, commandDELETE/, !commandMOVE/, commandFETCH/, or commandCOPY/ statement, !or an commandEXECUTE/ of a prepared query that contains an !commandINSERT/, commandUPDATE/, or commandDELETE/ statement. !If the command that generated the structnamePGresult/ was anything else, functionPQcmdTuples/ returns an empty string. The caller should not free the return value directly. It will be freed when the associated structnamePGresult/ handle is passed to Index: doc/src/sgml/protocol.sgml === RCS file:
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
On Sat, Feb 13, 2010 at 10:15 PM, Bruce Momjian br...@momjian.us wrote: Boszormenyi Zoltan wrote: Ah, I didn't even see that that section needed to be updated. Good catch. I'd suggest the following wording: For a commandSELECT/command or commandCREATE TABLE AS/command command, the tag is SELECT rows... [and the rest as you have it] We should probably also retitle that section from Retrieving Result Information for Other Commands to Retrieving Other Result Information and adjust the text of the opening sentence similarly. Also I think you need to update the docs for PQcmdtuples to mention that it not works for SELECT and CTAS. Ok, I will update libpq.sgml where this section resides. What's a CTA, btw? Do you mean CTE, a.k.a. Common Table Expression? Sorry, CTAS = CREATE TABLE AS SELECT. Okay, new patch is attached. Please read the docs changes, and comment. I have reviewed this patch and made some adjustments, attached. The major change is that our return of 0 0 in certain cases must remain, though I have improved the C comment explaining it with a separate CVS commit: /* * If a command completion tag was supplied, use it. Otherwise use the * portal's commandTag as the default completion tag. * * Exception: Clients expect INSERT/UPDATE/DELETE tags to have * counts, so fake them with zeros. This can happen with DO INSTEAD * rules if there is no replacement query of the same type as the * original. We print 0 0 here because technically there is no * query of the matching tag type, and printing a non-zero count for * a different query type seems wrong, e.g. an INSERT that does * an UPDATE instead should not print 0 1 if one row * was updated. See QueryRewrite(), step 3, for details. */ I have removed the part of the patch that chagned 0 0; it seems to run fine without it. The rest of my adjustments were minor. One major part of the patch seems to be the collection of the PORTAL_ONE_SELECT switch label into the label below it, which is a nice cleanup. Thanks for taking the time to review this. I think your adjustments are good. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Robert Haas írta:
On Mon, Feb 8, 2010 at 5:53 AM, Boszormenyi Zoltan z...@cybertec.at wrote:
New patch is attached with the discussed changes.
This looks OK to me now, but it lacks docs.
I'll set it to Waiting on Author.
...Robert
I added a small change to protocol.sgml for the
CommandComplete message describing the change.
Is it enough?
Best regards,
Zoltán Böszörményi
--
Bible has answers for everything. Proof:
But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil. (Matthew 5:37) - basics of digital technology.
May your kingdom come - superficial description of plate tectonics
--
Zoltán Böszörményi
Cybertec Schönig Schönig GmbH
http://www.postgresql.at/
diff -dcrpN pgsql.orig/doc/src/sgml/protocol.sgml pgsql/doc/src/sgml/protocol.sgml
*** pgsql.orig/doc/src/sgml/protocol.sgml 2010-02-03 11:12:23.0 +0100
--- pgsql/doc/src/sgml/protocol.sgml 2010-02-12 16:44:18.0 +0100
*** CommandComplete (B)
*** ,2227
--- ,2234
/para
para
+ For a commandSELECT [INTO]/command and
+ commandCREATE TABLE ... AS query/command commands,
+ the tag is literalSELECT replaceablerows/replaceable/literal
+ where replaceablerows/replaceable is the number of rows retrieved.
+/para
+
+para
For a commandMOVE/command command, the tag is
literalMOVE replaceablerows/replaceable/literal where
replaceablerows/replaceable is the number of rows the
diff -dcrpN pgsql.orig/src/backend/tcop/pquery.c pgsql/src/backend/tcop/pquery.c
*** pgsql.orig/src/backend/tcop/pquery.c 2010-01-03 12:54:25.0 +0100
--- pgsql/src/backend/tcop/pquery.c 2010-02-08 11:46:33.0 +0100
*** ProcessQuery(PlannedStmt *plan,
*** 205,211
switch (queryDesc-operation)
{
case CMD_SELECT:
! strcpy(completionTag, SELECT);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
--- 205,212
switch (queryDesc-operation)
{
case CMD_SELECT:
! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT %u, queryDesc-estate-es_processed);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
*** PortalRun(Portal portal, long count, boo
*** 714,719
--- 715,721
char *completionTag)
{
bool result;
+ uint32 nprocessed;
ResourceOwner saveTopTransactionResourceOwner;
MemoryContext saveTopTransactionContext;
Portal saveActivePortal;
*** PortalRun(Portal portal, long count, boo
*** 776,814
switch (portal-strategy)
{
case PORTAL_ONE_SELECT:
- (void) PortalRunSelect(portal, true, count, dest);
-
- /* we know the query is supposed to set the tag */
- if (completionTag portal-commandTag)
- strcpy(completionTag, portal-commandTag);
-
- /* Mark portal not active */
- portal-status = PORTAL_READY;
-
- /*
- * Since it's a forward fetch, say DONE iff atEnd is now true.
- */
- result = portal-atEnd;
- break;
-
case PORTAL_ONE_RETURNING:
case PORTAL_UTIL_SELECT:
/*
* If we have not yet run the command, do so, storing its
! * results in the portal's tuplestore.
*/
! if (!portal-holdStore)
FillPortalStore(portal, isTopLevel);
/*
* Now fetch desired portion of results.
*/
! (void) PortalRunSelect(portal, true, count, dest);
! /* we know the query is supposed to set the tag */
if (completionTag portal-commandTag)
! strcpy(completionTag, portal-commandTag);
/* Mark portal not active */
portal-status = PORTAL_READY;
--- 778,812
switch (portal-strategy)
{
case PORTAL_ONE_SELECT:
case PORTAL_ONE_RETURNING:
case PORTAL_UTIL_SELECT:
/*
* If we have not yet run the command, do so, storing its
! * results in the portal's tuplestore. Do this only for the
! * PORTAL_ONE_RETURNING and PORTAL_UTIL_SELECT cases.
*/
! if ((portal-strategy != PORTAL_ONE_SELECT) (!portal-holdStore))
FillPortalStore(portal, isTopLevel);
/*
* Now fetch desired portion of results.
*/
! nprocessed = PortalRunSelect(portal, true, count, dest);
! /*
! * If the portal result contains a command tag and the caller
! * gave us a pointer to store it, copy it. Patch the SELECT
! * tag to also provide the rowcount.
! */
if (completionTag portal-commandTag)
! {
! if (strcmp(portal-commandTag, SELECT) == 0)
! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT %u, nprocessed);
! else
! strcpy(completionTag, portal-commandTag);
! }
/* Mark portal not active */
portal-status = PORTAL_READY;
*** PortalRunMulti(Portal portal, bool isTop
*** 1318,1337
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
On Fri, Feb 12, 2010 at 10:48 AM, Boszormenyi Zoltan z...@cybertec.at wrote: I added a small change to protocol.sgml for the CommandComplete message describing the change. Is it enough? Ah, I didn't even see that that section needed to be updated. Good catch. I'd suggest the following wording: For a commandSELECT/command or commandCREATE TABLE AS/command command, the tag is SELECT rows... [and the rest as you have it] We should probably also retitle that section from Retrieving Result Information for Other Commands to Retrieving Other Result Information and adjust the text of the opening sentence similarly. Also I think you need to update the docs for PQcmdtuples to mention that it not works for SELECT and CTAS. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Robert Haas írta: On Fri, Feb 12, 2010 at 10:48 AM, Boszormenyi Zoltan z...@cybertec.at wrote: I added a small change to protocol.sgml for the CommandComplete message describing the change. Is it enough? Ah, I didn't even see that that section needed to be updated. Good catch. I'd suggest the following wording: For a commandSELECT/command or commandCREATE TABLE AS/command command, the tag is SELECT rows... [and the rest as you have it] We should probably also retitle that section from Retrieving Result Information for Other Commands to Retrieving Other Result Information and adjust the text of the opening sentence similarly. Also I think you need to update the docs for PQcmdtuples to mention that it not works for SELECT and CTAS. Ok, I will update libpq.sgml where this section resides. What's a CTA, btw? Do you mean CTE, a.k.a. Common Table Expression? Thanks, Zoltán Böszörményi -- Bible has answers for everything. Proof: But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil. (Matthew 5:37) - basics of digital technology. May your kingdom come - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Robert Haas írta:
On Fri, Feb 12, 2010 at 10:48 AM, Boszormenyi Zoltan z...@cybertec.at wrote:
I added a small change to protocol.sgml for the
CommandComplete message describing the change.
Is it enough?
Ah, I didn't even see that that section needed to be updated. Good
catch. I'd suggest the following wording:
For a commandSELECT/command or commandCREATE TABLE AS/command
command, the tag is SELECT rows... [and the rest as you have it]
We should probably also retitle that section from Retrieving Result
Information for Other Commands to Retrieving Other Result
Information and adjust the text of the opening sentence similarly.
Also I think you need to update the docs for PQcmdtuples to mention
that it not works for SELECT and CTAS.
I mentioned the WITH command, instead of CTEs.
Best regards,
Zoltán Böszörményi
--
Bible has answers for everything. Proof:
But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil. (Matthew 5:37) - basics of digital technology.
May your kingdom come - superficial description of plate tectonics
--
Zoltán Böszörményi
Cybertec Schönig Schönig GmbH
http://www.postgresql.at/
diff -dcrpN pgsql.orig/doc/src/sgml/libpq.sgml pgsql/doc/src/sgml/libpq.sgml
*** pgsql.orig/doc/src/sgml/libpq.sgml 2010-02-05 12:20:10.0 +0100
--- pgsql/doc/src/sgml/libpq.sgml 2010-02-12 19:27:08.0 +0100
*** typedef struct {
*** 2869,2880
/sect2
sect2 id=libpq-exec-nonselect
!titleRetrieving Result Information for Other Commands/title
para
! These functions are used to extract information from
! structnamePGresult/structname objects that are not
! commandSELECT/ results.
/para
variablelist
--- 2869,2879
/sect2
sect2 id=libpq-exec-nonselect
!titleRetrieving Other Result Information/title
para
! These functions are used to extract other information from
! structnamePGresult/structname objects.
/para
variablelist
*** typedef struct {
*** 2925,2936
This function returns a string containing the number of rows
affected by the acronymSQL/ statement that generated the
structnamePGresult/. This function can only be used following
!the execution of an commandINSERT/, commandUPDATE/,
!commandDELETE/, commandMOVE/, commandFETCH/, or
!commandCOPY/ statement, or an commandEXECUTE/ of a
!prepared query that contains an commandINSERT/,
!commandUPDATE/, or commandDELETE/ statement. If the
!command that generated the structnamePGresult/ was anything
else, functionPQcmdTuples/ returns an empty string. The caller
should not free the return value directly. It will be freed when
the associated structnamePGresult/ handle is passed to
--- 2924,2935
This function returns a string containing the number of rows
affected by the acronymSQL/ statement that generated the
structnamePGresult/. This function can only be used following
!the execution of a commandSELECT/, commandWITH/,
!commandINSERT/, commandUPDATE/, commandDELETE/,
!commandMOVE/, commandFETCH/, or commandCOPY/ statement,
!or an commandEXECUTE/ of a prepared query that contains an
!commandINSERT/, commandUPDATE/, or commandDELETE/ statement.
!If the command that generated the structnamePGresult/ was anything
else, functionPQcmdTuples/ returns an empty string. The caller
should not free the return value directly. It will be freed when
the associated structnamePGresult/ handle is passed to
diff -dcrpN pgsql.orig/doc/src/sgml/protocol.sgml pgsql/doc/src/sgml/protocol.sgml
*** pgsql.orig/doc/src/sgml/protocol.sgml 2010-02-03 11:12:23.0 +0100
--- pgsql/doc/src/sgml/protocol.sgml 2010-02-12 19:17:24.0 +0100
*** CommandComplete (B)
*** ,2227
--- ,2233
/para
para
+ For a commandSELECT/command or commandCREATE TABLE AS/command
+ command, the tag is literalSELECT replaceablerows/replaceable/literal
+ where replaceablerows/replaceable is the number of rows retrieved.
+/para
+
+para
For a commandMOVE/command command, the tag is
literalMOVE replaceablerows/replaceable/literal where
replaceablerows/replaceable is the number of rows the
diff -dcrpN pgsql.orig/src/backend/tcop/pquery.c pgsql/src/backend/tcop/pquery.c
*** pgsql.orig/src/backend/tcop/pquery.c 2010-01-03 12:54:25.0 +0100
--- pgsql/src/backend/tcop/pquery.c 2010-02-08 11:46:33.0 +0100
*** ProcessQuery(PlannedStmt *plan,
*** 205,211
switch (queryDesc-operation)
{
case CMD_SELECT:
! strcpy(completionTag, SELECT);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
On Fri, Feb 12, 2010 at 1:22 PM, Boszormenyi Zoltan z...@cybertec.at wrote: Robert Haas írta: On Fri, Feb 12, 2010 at 10:48 AM, Boszormenyi Zoltan z...@cybertec.at wrote: I added a small change to protocol.sgml for the CommandComplete message describing the change. Is it enough? Ah, I didn't even see that that section needed to be updated. Good catch. I'd suggest the following wording: For a commandSELECT/command or commandCREATE TABLE AS/command command, the tag is SELECT rows... [and the rest as you have it] We should probably also retitle that section from Retrieving Result Information for Other Commands to Retrieving Other Result Information and adjust the text of the opening sentence similarly. Also I think you need to update the docs for PQcmdtuples to mention that it not works for SELECT and CTAS. Ok, I will update libpq.sgml where this section resides. What's a CTA, btw? Do you mean CTE, a.k.a. Common Table Expression? Sorry, CTAS = CREATE TABLE AS SELECT. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Robert Haas írta:
On Fri, Feb 12, 2010 at 1:22 PM, Boszormenyi Zoltan z...@cybertec.at wrote:
Robert Haas írta:
On Fri, Feb 12, 2010 at 10:48 AM, Boszormenyi Zoltan z...@cybertec.at
wrote:
I added a small change to protocol.sgml for the
CommandComplete message describing the change.
Is it enough?
Ah, I didn't even see that that section needed to be updated. Good
catch. I'd suggest the following wording:
For a commandSELECT/command or commandCREATE TABLE AS/command
command, the tag is SELECT rows... [and the rest as you have it]
We should probably also retitle that section from Retrieving Result
Information for Other Commands to Retrieving Other Result
Information and adjust the text of the opening sentence similarly.
Also I think you need to update the docs for PQcmdtuples to mention
that it not works for SELECT and CTAS.
Ok, I will update libpq.sgml where this section resides.
What's a CTA, btw? Do you mean CTE, a.k.a. Common Table Expression?
Sorry, CTAS = CREATE TABLE AS SELECT.
Okay, new patch is attached. Please read the docs changes, and comment.
Thanks,
Zoltán Böszörményi
--
Bible has answers for everything. Proof:
But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil. (Matthew 5:37) - basics of digital technology.
May your kingdom come - superficial description of plate tectonics
--
Zoltán Böszörményi
Cybertec Schönig Schönig GmbH
http://www.postgresql.at/
diff -dcrpN pgsql.orig/doc/src/sgml/libpq.sgml pgsql/doc/src/sgml/libpq.sgml
*** pgsql.orig/doc/src/sgml/libpq.sgml 2010-02-05 12:20:10.0 +0100
--- pgsql/doc/src/sgml/libpq.sgml 2010-02-12 19:53:36.0 +0100
*** typedef struct {
*** 2869,2880
/sect2
sect2 id=libpq-exec-nonselect
!titleRetrieving Result Information for Other Commands/title
para
! These functions are used to extract information from
! structnamePGresult/structname objects that are not
! commandSELECT/ results.
/para
variablelist
--- 2869,2879
/sect2
sect2 id=libpq-exec-nonselect
!titleRetrieving Other Result Information/title
para
! These functions are used to extract other information from
! structnamePGresult/structname objects.
/para
variablelist
*** typedef struct {
*** 2925,2936
This function returns a string containing the number of rows
affected by the acronymSQL/ statement that generated the
structnamePGresult/. This function can only be used following
!the execution of an commandINSERT/, commandUPDATE/,
!commandDELETE/, commandMOVE/, commandFETCH/, or
!commandCOPY/ statement, or an commandEXECUTE/ of a
!prepared query that contains an commandINSERT/,
!commandUPDATE/, or commandDELETE/ statement. If the
!command that generated the structnamePGresult/ was anything
else, functionPQcmdTuples/ returns an empty string. The caller
should not free the return value directly. It will be freed when
the associated structnamePGresult/ handle is passed to
--- 2924,2935
This function returns a string containing the number of rows
affected by the acronymSQL/ statement that generated the
structnamePGresult/. This function can only be used following
!the execution of a commandSELECT/, commandCREATE TABLE AS/,
!commandINSERT/, commandUPDATE/, commandDELETE/,
!commandMOVE/, commandFETCH/, or commandCOPY/ statement,
!or an commandEXECUTE/ of a prepared query that contains an
!commandINSERT/, commandUPDATE/, or commandDELETE/ statement.
!If the command that generated the structnamePGresult/ was anything
else, functionPQcmdTuples/ returns an empty string. The caller
should not free the return value directly. It will be freed when
the associated structnamePGresult/ handle is passed to
diff -dcrpN pgsql.orig/doc/src/sgml/protocol.sgml pgsql/doc/src/sgml/protocol.sgml
*** pgsql.orig/doc/src/sgml/protocol.sgml 2010-02-03 11:12:23.0 +0100
--- pgsql/doc/src/sgml/protocol.sgml 2010-02-12 19:17:24.0 +0100
*** CommandComplete (B)
*** ,2227
--- ,2233
/para
para
+ For a commandSELECT/command or commandCREATE TABLE AS/command
+ command, the tag is literalSELECT replaceablerows/replaceable/literal
+ where replaceablerows/replaceable is the number of rows retrieved.
+/para
+
+para
For a commandMOVE/command command, the tag is
literalMOVE replaceablerows/replaceable/literal where
replaceablerows/replaceable is the number of rows the
diff -dcrpN pgsql.orig/src/backend/tcop/pquery.c pgsql/src/backend/tcop/pquery.c
*** pgsql.orig/src/backend/tcop/pquery.c 2010-01-03
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
On Fri, Feb 12, 2010 at 1:55 PM, Boszormenyi Zoltan z...@cybertec.at wrote: Okay, new patch is attached. Please read the docs changes, and comment. I like it. I'll mark it as Ready for Committer. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
On Mon, Feb 8, 2010 at 5:53 AM, Boszormenyi Zoltan z...@cybertec.at wrote: New patch is attached with the discussed changes. This looks OK to me now, but it lacks docs. I'll set it to Waiting on Author. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Boszormenyi Zoltan escribió: Robert Haas írta: ... OK, please change it. New patch is attached with the discussed changes. This looks all wrong. PORTAL_ONE_SELECT is now being passed through FillPortalStore, which runs it to completion, whereas it was previously passed via PortalRunSelect first, which has different semantics regarding the count arg. Also, even if that weren't wrong, FillPortalStore states at its header comment that it is only used for the other two cases (ONE_RETURNING and UTIL_SELECT), but now is being used for ONE_SELECT as well. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
On Thu, Feb 11, 2010 at 2:04 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Boszormenyi Zoltan escribió: Robert Haas írta: ... OK, please change it. New patch is attached with the discussed changes. This looks all wrong. PORTAL_ONE_SELECT is now being passed through FillPortalStore, which runs it to completion, whereas it was previously passed via PortalRunSelect first, which has different semantics regarding the count arg. Also, even if that weren't wrong, FillPortalStore states at its header comment that it is only used for the other two cases (ONE_RETURNING and UTIL_SELECT), but now is being used for ONE_SELECT as well. I was all prepared to admit that I hadn't actually looked at the patch carefully enough, but I just looked at (and CVS HEAD) again and what you've written here doesn't appear to describe what I'm seeing in the code: if ((portal-strategy != PORTAL_ONE_SELECT) (!portal-holdStore)) FillPortalStore(portal, isTopLevel); So one of us is confused... it may well be me. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Robert Haas escribió: I was all prepared to admit that I hadn't actually looked at the patch carefully enough, but I just looked at (and CVS HEAD) again and what you've written here doesn't appear to describe what I'm seeing in the code: if ((portal-strategy != PORTAL_ONE_SELECT) (!portal-holdStore)) FillPortalStore(portal, isTopLevel); So one of us is confused... it may well be me. Ah, it seems I misread it ... but then I don't quite see the point in that change. Well, not doing a full review anyway, so never mind me. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
On Thu, Feb 11, 2010 at 2:38 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Robert Haas escribió: I was all prepared to admit that I hadn't actually looked at the patch carefully enough, but I just looked at (and CVS HEAD) again and what you've written here doesn't appear to describe what I'm seeing in the code: if ((portal-strategy != PORTAL_ONE_SELECT) (!portal-holdStore)) FillPortalStore(portal, isTopLevel); So one of us is confused... it may well be me. Ah, it seems I misread it ... but then I don't quite see the point in that change. Well the point is just that Zoltan is adding some more code that applies to both branches of the switch, so merging them saves some duplication. Well, not doing a full review anyway, so never mind me. Actually I was sort of hoping you (or someone other than me) would pick this up for commit... ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Robert Haas escribió: On Thu, Feb 11, 2010 at 2:38 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Robert Haas escribió: I was all prepared to admit that I hadn't actually looked at the patch carefully enough, but I just looked at (and CVS HEAD) again and what you've written here doesn't appear to describe what I'm seeing in the code: if ((portal-strategy != PORTAL_ONE_SELECT) (!portal-holdStore)) FillPortalStore(portal, isTopLevel); So one of us is confused... it may well be me. Ah, it seems I misread it ... but then I don't quite see the point in that change. Well the point is just that Zoltan is adding some more code that applies to both branches of the switch, so merging them saves some duplication. But then there's no other branches, so why not just put it below the switch? Well, not doing a full review anyway, so never mind me. Actually I was sort of hoping you (or someone other than me) would pick this up for commit... Hmm ... -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
On Thu, Feb 11, 2010 at 2:47 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Robert Haas escribió: On Thu, Feb 11, 2010 at 2:38 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Robert Haas escribió: I was all prepared to admit that I hadn't actually looked at the patch carefully enough, but I just looked at (and CVS HEAD) again and what you've written here doesn't appear to describe what I'm seeing in the code: if ((portal-strategy != PORTAL_ONE_SELECT) (!portal-holdStore)) FillPortalStore(portal, isTopLevel); So one of us is confused... it may well be me. Ah, it seems I misread it ... but then I don't quite see the point in that change. Well the point is just that Zoltan is adding some more code that applies to both branches of the switch, so merging them saves some duplication. But then there's no other branches, so why not just put it below the switch? No, PORTAL_MULTI_QUERY is still separate. Well, not doing a full review anyway, so never mind me. Actually I was sort of hoping you (or someone other than me) would pick this up for commit... Hmm ... Maybe I came to the wrong place. :-) ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Alvaro Herrera írta: Boszormenyi Zoltan escribió: Robert Haas írta: ... OK, please change it. New patch is attached with the discussed changes. This looks all wrong. PORTAL_ONE_SELECT is now being passed through FillPortalStore, Where do you read that? The code in my patch reads: /* * If we have not yet run the command, do so, storing its !* results in the portal's tuplestore. Do this only for the !* PORTAL_ONE_RETURNING and PORTAL_UTIL_SELECT cases. */ ! if ((portal-strategy != PORTAL_ONE_SELECT) (!portal-holdStore)) FillPortalStore(portal, isTopLevel); So, PORTAL_ONE_SELECT doesn't run through FillPortalStore(). which runs it to completion, whereas it was previously passed via PortalRunSelect first, which has different semantics regarding the count arg. Also, even if that weren't wrong, FillPortalStore states at its header comment that it is only used for the other two cases (ONE_RETURNING and UTIL_SELECT), but now is being used for ONE_SELECT as well. -- Bible has answers for everything. Proof: But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil. (Matthew 5:37) - basics of digital technology. May your kingdom come - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Alvaro Herrera írta: Robert Haas escribió: On Thu, Feb 11, 2010 at 2:38 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Robert Haas escribió: I was all prepared to admit that I hadn't actually looked at the patch carefully enough, but I just looked at (and CVS HEAD) again and what you've written here doesn't appear to describe what I'm seeing in the code: if ((portal-strategy != PORTAL_ONE_SELECT) (!portal-holdStore)) FillPortalStore(portal, isTopLevel); So one of us is confused... it may well be me. Ah, it seems I misread it ... but then I don't quite see the point in that change. Well the point is just that Zoltan is adding some more code that applies to both branches of the switch, so merging them saves some duplication. But then there's no other branches, so why not just put it below the switch? Well, for one because the PORTAL_MULTI_QUERY case doesn't set the command tag even there was one from the last one and the caller provided the non-NULL pointer. That would be behaviour change. Well, not doing a full review anyway, so never mind me. Actually I was sort of hoping you (or someone other than me) would pick this up for commit... Hmm ... -- Bible has answers for everything. Proof: But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil. (Matthew 5:37) - basics of digital technology. May your kingdom come - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Robert Haas írta:
...
OK, please change it.
New patch is attached with the discussed changes.
Someone who knows the overall structure of the code better than I do
will have to comment on whether there are any code paths to worry
about that do not go through PortalRun().
A general concern I have is that this what we're basically doing here
is handling the most common case in ProcessQuery() and then installing
fallback mechanisms to pick up any stragglers: but the fallback
mechanisms only guarantee that we'll add a number to the command tag,
not that it will be meaningful. Unfortunately, my limited imagination
can't quite figure out in what cases we'll get a SELECT command tag
back other than (1) plain old SELECT, (2) SELECT INTO, and (3) CTAS,
so I'm not sure what to go test.
Any thoughts on these issues, anyone?
...Robert
Best regards,
Zoltán Böszörményi
--
Bible has answers for everything. Proof:
But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil. (Matthew 5:37) - basics of digital technology.
May your kingdom come - superficial description of plate tectonics
--
Zoltán Böszörményi
Cybertec Schönig Schönig GmbH
http://www.postgresql.at/
diff -dcrpN pgsql.orig/src/backend/tcop/pquery.c pgsql/src/backend/tcop/pquery.c
*** pgsql.orig/src/backend/tcop/pquery.c 2010-01-03 12:54:25.0 +0100
--- pgsql/src/backend/tcop/pquery.c 2010-02-08 11:46:33.0 +0100
*** ProcessQuery(PlannedStmt *plan,
*** 205,211
switch (queryDesc-operation)
{
case CMD_SELECT:
! strcpy(completionTag, SELECT);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
--- 205,212
switch (queryDesc-operation)
{
case CMD_SELECT:
! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT %u, queryDesc-estate-es_processed);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
*** PortalRun(Portal portal, long count, boo
*** 714,719
--- 715,721
char *completionTag)
{
bool result;
+ uint32 nprocessed;
ResourceOwner saveTopTransactionResourceOwner;
MemoryContext saveTopTransactionContext;
Portal saveActivePortal;
*** PortalRun(Portal portal, long count, boo
*** 776,814
switch (portal-strategy)
{
case PORTAL_ONE_SELECT:
- (void) PortalRunSelect(portal, true, count, dest);
-
- /* we know the query is supposed to set the tag */
- if (completionTag portal-commandTag)
- strcpy(completionTag, portal-commandTag);
-
- /* Mark portal not active */
- portal-status = PORTAL_READY;
-
- /*
- * Since it's a forward fetch, say DONE iff atEnd is now true.
- */
- result = portal-atEnd;
- break;
-
case PORTAL_ONE_RETURNING:
case PORTAL_UTIL_SELECT:
/*
* If we have not yet run the command, do so, storing its
! * results in the portal's tuplestore.
*/
! if (!portal-holdStore)
FillPortalStore(portal, isTopLevel);
/*
* Now fetch desired portion of results.
*/
! (void) PortalRunSelect(portal, true, count, dest);
! /* we know the query is supposed to set the tag */
if (completionTag portal-commandTag)
! strcpy(completionTag, portal-commandTag);
/* Mark portal not active */
portal-status = PORTAL_READY;
--- 778,812
switch (portal-strategy)
{
case PORTAL_ONE_SELECT:
case PORTAL_ONE_RETURNING:
case PORTAL_UTIL_SELECT:
/*
* If we have not yet run the command, do so, storing its
! * results in the portal's tuplestore. Do this only for the
! * PORTAL_ONE_RETURNING and PORTAL_UTIL_SELECT cases.
*/
! if ((portal-strategy != PORTAL_ONE_SELECT) (!portal-holdStore))
FillPortalStore(portal, isTopLevel);
/*
* Now fetch desired portion of results.
*/
! nprocessed = PortalRunSelect(portal, true, count, dest);
! /*
! * If the portal result contains a command tag and the caller
! * gave us a pointer to store it, copy it. Patch the SELECT
! * tag to also provide the rowcount.
! */
if (completionTag portal-commandTag)
! {
! if (strcmp(portal-commandTag, SELECT) == 0)
! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT %u, nprocessed);
! else
! strcpy(completionTag, portal-commandTag);
! }
/* Mark portal not active */
portal-status = PORTAL_READY;
*** PortalRunMulti(Portal portal, bool isTop
*** 1318,1337
* If a command completion tag was supplied, use it. Otherwise use the
* portal's commandTag as the default completion tag.
*
! * Exception: clients will expect INSERT/UPDATE/DELETE tags to have
! * counts, so fake something up if necessary. (This could happen if the
* original
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Robert Haas írta: On Tue, Feb 2, 2010 at 4:03 AM, Boszormenyi Zoltan z...@cybertec.at wrote: Thanks for testing it, with the attached patch your test case also returns SELECT N. Thoughts: 1. Looks like you've falsified the last comment block in PortalRunMulti(). You mean the fake something up part? Will fix the comment. 2. I don't like the duplication of code in PortalRun() between the first and second branches of the switch statement. The PORTAL_ONE_SELECT and PORTAL_ONE_RETURNING/PORTAL_UTIL_SELECT cases differ only in that the latter case runs this below everything else: if (!portal-holdStore) FillPortalStore(portal, isTopLevel); Would it be desired to unify these cases? This way there would be no code duplication. Something like: if (portal-strategy != PORTAL_ONE_SELECT !portal-holdStore) FillPortalStore(portal, isTopLevel); ... (everything else) 3. You've falsified the comment preceding that code, too. This one? /* * Set up global portal context pointers. * * We have to play a special game here to support utility commands like * VACUUM and CLUSTER, which internally start and commit transactions. * When we are called to execute such a command, CurrentResourceOwner will * be pointing to the TopTransactionResourceOwner --- which will be * destroyed and replaced in the course of the internal commit and * restart. So we need to be prepared to restore it as pointing to the * exit-time TopTransactionResourceOwner. (Ain't that ugly? This idea of * internally starting whole new transactions is not good.) * CurrentMemoryContext has a similar problem, but the other pointers we * save here will be NULL or pointing to longer-lived objects. */ I can't see why. Can you clarify? Or this one? /* we know the query is supposed to set the tag */ if (completionTag portal-commandTag) ... The condition and the comment still seems to be true. Which comment are you talking about? 4. Is there any reason to use pg_strcasecmp() instead of plain old strcmp()? I don't have any particular reason, strcmp() would do. Someone who knows the overall structure of the code better than I do will have to comment on whether there are any code paths to worry about that do not go through PortalRun(). A general concern I have is that this what we're basically doing here is handling the most common case in ProcessQuery() and then installing fallback mechanisms to pick up any stragglers: but the fallback mechanisms only guarantee that we'll add a number to the command tag, not that it will be meaningful. Unfortunately, my limited imagination can't quite figure out in what cases we'll get a SELECT command tag back other than (1) plain old SELECT, (2) SELECT INTO, and (3) CTAS, so I'm not sure what to go test. ...Robert Best regards, Zoltán Böszörményi -- Bible has answers for everything. Proof: But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil. (Matthew 5:37) - basics of digital technology. May your kingdom come - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
On Sun, Feb 7, 2010 at 12:46 PM, Boszormenyi Zoltan z...@cybertec.at wrote: 1. Looks like you've falsified the last comment block in PortalRunMulti(). You mean the fake something up part? Will fix the comment. Yes. 2. I don't like the duplication of code in PortalRun() between the first and second branches of the switch statement. The PORTAL_ONE_SELECT and PORTAL_ONE_RETURNING/PORTAL_UTIL_SELECT cases differ only in that the latter case runs this below everything else: if (!portal-holdStore) FillPortalStore(portal, isTopLevel); Would it be desired to unify these cases? This way there would be no code duplication. Something like: if (portal-strategy != PORTAL_ONE_SELECT !portal-holdStore) FillPortalStore(portal, isTopLevel); ... (everything else) I thought about that but wasn't sure. I recommend that you give it a try that way and we'll see how it looks. 3. You've falsified the comment preceding that code, too. Or this one? /* we know the query is supposed to set the tag */ if (completionTag portal-commandTag) ... The condition and the comment still seems to be true. This is the one I was referring to. I guess falsified was too strong, but I don't think that comment describes the function of that code too well any more. Maybe it didn't before either, but I think it's worse now. 4. Is there any reason to use pg_strcasecmp() instead of plain old strcmp()? I don't have any particular reason, strcmp() would do. OK, please change it. Someone who knows the overall structure of the code better than I do will have to comment on whether there are any code paths to worry about that do not go through PortalRun(). A general concern I have is that this what we're basically doing here is handling the most common case in ProcessQuery() and then installing fallback mechanisms to pick up any stragglers: but the fallback mechanisms only guarantee that we'll add a number to the command tag, not that it will be meaningful. Unfortunately, my limited imagination can't quite figure out in what cases we'll get a SELECT command tag back other than (1) plain old SELECT, (2) SELECT INTO, and (3) CTAS, so I'm not sure what to go test. Any thoughts on these issues, anyone? ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
On Tue, Feb 2, 2010 at 4:03 AM, Boszormenyi Zoltan z...@cybertec.at wrote: Thanks for testing it, with the attached patch your test case also returns SELECT N. Thoughts: 1. Looks like you've falsified the last comment block in PortalRunMulti(). 2. I don't like the duplication of code in PortalRun() between the first and second branches of the switch statement. 3. You've falsified the comment preceding that code, too. 4. Is there any reason to use pg_strcasecmp() instead of plain old strcmp()? Someone who knows the overall structure of the code better than I do will have to comment on whether there are any code paths to worry about that do not go through PortalRun(). A general concern I have is that this what we're basically doing here is handling the most common case in ProcessQuery() and then installing fallback mechanisms to pick up any stragglers: but the fallback mechanisms only guarantee that we'll add a number to the command tag, not that it will be meaningful. Unfortunately, my limited imagination can't quite figure out in what cases we'll get a SELECT command tag back other than (1) plain old SELECT, (2) SELECT INTO, and (3) CTAS, so I'm not sure what to go test. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Robert Haas írta:
2010/1/12 Boszormenyi Zoltan z...@cybertec.at:
Tom Lane írta:
Alvaro Herrera alvhe...@commandprompt.com writes:
But it would be broken in very obvious ways, no? It's not like it would
be silently broken and thus escape testing ...
Well, if we wanted to adopt that approach, we should add the count to
*all* SELECT tags not just a small subset. As the patch stands it
seems entirely possible that a breakage would escape immediate notice.
Can you give me an example that would return
plain SELECT after my new patch? I added
one more change to the patch, is it enough to return
SELECT N in every case now?
I just tested this, so I can say definitely: no. I hacked psql with
the attached patch, and if you just do a plain old SELECT * FROM
table, you get back only SELECT, not SELECT N.
...Robert
Thanks for testing it, with the attached patch your test case also
returns SELECT N.
Best regards,
Zoltán Böszörményi
--
Bible has answers for everything. Proof:
But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil. (Matthew 5:37) - basics of digital technology.
May your kingdom come - superficial description of plate tectonics
--
Zoltán Böszörményi
Cybertec Schönig Schönig GmbH
http://www.postgresql.at/
diff -dcrpN pgsql.orig/src/backend/tcop/pquery.c pgsql/src/backend/tcop/pquery.c
*** pgsql.orig/src/backend/tcop/pquery.c 2010-01-03 12:54:25.0 +0100
--- pgsql/src/backend/tcop/pquery.c 2010-02-02 09:59:34.0 +0100
*** ProcessQuery(PlannedStmt *plan,
*** 205,211
switch (queryDesc-operation)
{
case CMD_SELECT:
! strcpy(completionTag, SELECT);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
--- 205,212
switch (queryDesc-operation)
{
case CMD_SELECT:
! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT %u, queryDesc-estate-es_processed);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
*** PortalRun(Portal portal, long count, boo
*** 714,719
--- 715,721
char *completionTag)
{
bool result;
+ uint32 nprocessed;
ResourceOwner saveTopTransactionResourceOwner;
MemoryContext saveTopTransactionContext;
Portal saveActivePortal;
*** PortalRun(Portal portal, long count, boo
*** 776,786
switch (portal-strategy)
{
case PORTAL_ONE_SELECT:
! (void) PortalRunSelect(portal, true, count, dest);
/* we know the query is supposed to set the tag */
if (completionTag portal-commandTag)
! strcpy(completionTag, portal-commandTag);
/* Mark portal not active */
portal-status = PORTAL_READY;
--- 778,794
switch (portal-strategy)
{
case PORTAL_ONE_SELECT:
! nprocessed = PortalRunSelect(portal, true, count, dest);
/* we know the query is supposed to set the tag */
if (completionTag portal-commandTag)
! {
! if (pg_strcasecmp(portal-commandTag, SELECT) == 0)
! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT %u, nprocessed);
! else
! strcpy(completionTag, portal-commandTag);
! }
/* Mark portal not active */
portal-status = PORTAL_READY;
*** PortalRun(Portal portal, long count, boo
*** 804,814
/*
* Now fetch desired portion of results.
*/
! (void) PortalRunSelect(portal, true, count, dest);
/* we know the query is supposed to set the tag */
if (completionTag portal-commandTag)
! strcpy(completionTag, portal-commandTag);
/* Mark portal not active */
portal-status = PORTAL_READY;
--- 812,828
/*
* Now fetch desired portion of results.
*/
! nprocessed = PortalRunSelect(portal, true, count, dest);
/* we know the query is supposed to set the tag */
if (completionTag portal-commandTag)
! {
! if (pg_strcasecmp(portal-commandTag, SELECT) == 0)
! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT %u, nprocessed);
! else
! strcpy(completionTag, portal-commandTag);
! }
/* Mark portal not active */
portal-status = PORTAL_READY;
*** PortalRunMulti(Portal portal, bool isTop
*** 1324,1337
*/
if (completionTag completionTag[0] == '\0')
{
if (portal-commandTag)
strcpy(completionTag, portal-commandTag);
if (strcmp(completionTag, INSERT) == 0)
! strcpy(completionTag, INSERT 0 0);
else if (strcmp(completionTag, UPDATE) == 0)
! strcpy(completionTag, UPDATE 0);
else if (strcmp(completionTag, DELETE) == 0)
! strcpy(completionTag, DELETE 0);
}
}
--- 1338,1362
*/
if (completionTag completionTag[0] == '\0')
{
+ Oid lastOid = InvalidOid;
+ uint32 processed = 0;
+
+ if (portal-queryDesc
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
2010/1/12 Boszormenyi Zoltan z...@cybertec.at:
Tom Lane írta:
Alvaro Herrera alvhe...@commandprompt.com writes:
But it would be broken in very obvious ways, no? It's not like it would
be silently broken and thus escape testing ...
Well, if we wanted to adopt that approach, we should add the count to
*all* SELECT tags not just a small subset. As the patch stands it
seems entirely possible that a breakage would escape immediate notice.
Can you give me an example that would return
plain SELECT after my new patch? I added
one more change to the patch, is it enough to return
SELECT N in every case now?
I just tested this, so I can say definitely: no. I hacked psql with
the attached patch, and if you just do a plain old SELECT * FROM
table, you get back only SELECT, not SELECT N.
...Robert
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 70b9310..5b37d3d 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -698,7 +698,6 @@ static bool
PrintQueryResults(PGresult *results)
{
bool success = false;
- const char *cmdstatus;
if (!results)
return false;
@@ -709,11 +708,7 @@ PrintQueryResults(PGresult *results)
/* print the data ... */
success = PrintQueryTuples(results);
/* if it's INSERT/UPDATE/DELETE RETURNING, also print status */
- cmdstatus = PQcmdStatus(results);
- if (strncmp(cmdstatus, INSERT, 6) == 0 ||
-strncmp(cmdstatus, UPDATE, 6) == 0 ||
-strncmp(cmdstatus, DELETE, 6) == 0)
-PrintQueryStatus(results);
+ PrintQueryStatus(results);
break;
case PGRES_COMMAND_OK:
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Tom Lane írta:
Alvaro Herrera alvhe...@commandprompt.com writes:
But it would be broken in very obvious ways, no? It's not like it would
be silently broken and thus escape testing ...
Well, if we wanted to adopt that approach, we should add the count to
*all* SELECT tags not just a small subset. As the patch stands it
seems entirely possible that a breakage would escape immediate notice.
regards, tom lane
Can you give me an example that would return
plain SELECT after my new patch? I added
one more change to the patch, is it enough to return
SELECT N in every case now?
Best regards,
Zoltán Böszörményi
--
Bible has answers for everything. Proof:
But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil. (Matthew 5:37) - basics of digital technology.
May your kingdom come - superficial description of plate tectonics
--
Zoltán Böszörményi
Cybertec Schönig Schönig GmbH
http://www.postgresql.at/
diff -dcrpN pgsql.orig/src/backend/tcop/pquery.c pgsql.1/src/backend/tcop/pquery.c
*** pgsql.orig/src/backend/tcop/pquery.c 2010-01-03 12:54:25.0 +0100
--- pgsql.1/src/backend/tcop/pquery.c 2010-01-12 15:16:32.0 +0100
*** ProcessQuery(PlannedStmt *plan,
*** 205,211
switch (queryDesc-operation)
{
case CMD_SELECT:
! strcpy(completionTag, SELECT);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
--- 205,212
switch (queryDesc-operation)
{
case CMD_SELECT:
! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT %u, queryDesc-estate-es_processed);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
*** PortalRunMulti(Portal portal, bool isTop
*** 1324,1337
*/
if (completionTag completionTag[0] == '\0')
{
if (portal-commandTag)
strcpy(completionTag, portal-commandTag);
if (strcmp(completionTag, INSERT) == 0)
! strcpy(completionTag, INSERT 0 0);
else if (strcmp(completionTag, UPDATE) == 0)
! strcpy(completionTag, UPDATE 0);
else if (strcmp(completionTag, DELETE) == 0)
! strcpy(completionTag, DELETE 0);
}
}
--- 1325,1349
*/
if (completionTag completionTag[0] == '\0')
{
+ Oid lastOid = InvalidOid;
+ uint32 processed = 0;
+
+ if (portal-queryDesc portal-queryDesc-estate)
+ {
+ lastOid = portal-queryDesc-estate-es_lastoid;
+ processed = portal-queryDesc-estate-es_processed;
+ }
+
if (portal-commandTag)
strcpy(completionTag, portal-commandTag);
if (strcmp(completionTag, INSERT) == 0)
! sprintf(completionTag, INSERT %u %u, lastOid, processed);
else if (strcmp(completionTag, UPDATE) == 0)
! sprintf(completionTag, UPDATE %u, processed);
else if (strcmp(completionTag, DELETE) == 0)
! sprintf(completionTag, DELETE %u, processed);
! else if (strcmp(completionTag, SELECT) == 0)
! sprintf(completionTag, SELECT %u, processed);
}
}
diff -dcrpN pgsql.orig/src/interfaces/libpq/fe-exec.c pgsql.1/src/interfaces/libpq/fe-exec.c
*** pgsql.orig/src/interfaces/libpq/fe-exec.c 2010-01-03 12:54:41.0 +0100
--- pgsql.1/src/interfaces/libpq/fe-exec.c 2010-01-12 15:05:06.0 +0100
*** PQcmdTuples(PGresult *res)
*** 2753,2758
--- 2753,2759
p++;
}
else if (strncmp(res-cmdStatus, DELETE , 7) == 0 ||
+ strncmp(res-cmdStatus, SELECT , 7) == 0 ||
strncmp(res-cmdStatus, UPDATE , 7) == 0)
p = res-cmdStatus + 7;
else if (strncmp(res-cmdStatus, FETCH , 6) == 0)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
On Mon, Dec 28, 2009 at 12:10 PM, Tom Lane t...@sss.pgh.pa.us wrote: Boszormenyi Zoltan z...@cybertec.at writes: attached is a small patch that makes it possible for clients to receive row count for SELECT ... INTO ... and CREATE TABLE ... AS ... Comments? This doesn't look tremendously well thought out to me. 1. As given, the patch changes the result not only for SELECT INTO but for any SELECT executed in PORTAL_MULTI_QUERY context (consider SELECTs added by rules for example). It seems like a pretty bad idea for the result of a statement to depend on context. 2. In the past we have regretted it when we made the same command tag sometimes have numbers attached and sometimes not (note the hack at the bottom of PortalRunMulti). It doesn't seem like terribly good design to do that here. On the other hand, always attaching a count to SELECT tags would greatly increase the risk of breaking clients. I'm not at all convinced that this is so useful as to justify taking any compatibility risks for. People who really need that count can get it easily enough by breaking the command into a CREATE followed by INSERT/SELECT. I don't know whether or not it's a good idea to do this for anything in a PORTAL_MULTI_QUERY context, because I haven't really thought through the issues. But, it's hard for me to imagine that anyone is depending on the command tag returned by CTAS or SELECT INTO. Generally, I think making command tags return more useful information is a good thing. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Tom Lane escribió: Boszormenyi Zoltan z...@cybertec.at writes: Tom Lane �rta: It's more the possibility of doing strcmp(tag, SELECT) on the command Actually it's strncmp(tag, SELECT , 7), so when you mix old server with new clients or new server with old client, it will just work as before, i.e. return . Are you deliberately ignoring the point? We have no way to know whether there is any client-side code that's doing a simple check for SELECT command tag, but it's certainly possible. The fact that it wouldn't be hard to fix does not mean that it wouldn't be broken. But it would be broken in very obvious ways, no? It's not like it would be silently broken and thus escape testing ... -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Alvaro Herrera alvhe...@commandprompt.com writes: But it would be broken in very obvious ways, no? It's not like it would be silently broken and thus escape testing ... Well, if we wanted to adopt that approach, we should add the count to *all* SELECT tags not just a small subset. As the patch stands it seems entirely possible that a breakage would escape immediate notice. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] Provide rowcount for utility SELECTs
Hi,
attached is a small patch that makes it possible for clients
to receive row count for SELECT ... INTO ... and CREATE TABLE ... AS ...
Comments?
Best regards,
Zoltán Böszörményi
--
Bible has answers for everything. Proof:
But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil. (Matthew 5:37) - basics of digital technology.
May your kingdom come - superficial description of plate tectonics
--
Zoltán Böszörményi
Cybertec Schönig Schönig GmbH
http://www.postgresql.at/
diff -dcrpN pgsql.4.1/src/backend/tcop/pquery.c pgsql.6/src/backend/tcop/pquery.c
*** pgsql.4.1/src/backend/tcop/pquery.c 2009-12-15 10:15:05.0 +0100
--- pgsql.6/src/backend/tcop/pquery.c 2009-12-22 12:02:55.0 +0100
*** ProcessQuery(PlannedStmt *plan,
*** 205,211
switch (queryDesc-operation)
{
case CMD_SELECT:
! strcpy(completionTag, SELECT);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
--- 205,212
switch (queryDesc-operation)
{
case CMD_SELECT:
! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT %u, queryDesc-estate-es_processed);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
diff -dcrpN pgsql.4.1/src/interfaces/libpq/fe-exec.c pgsql.6/src/interfaces/libpq/fe-exec.c
*** pgsql.4.1/src/interfaces/libpq/fe-exec.c 2009-08-07 13:06:30.0 +0200
--- pgsql.6/src/interfaces/libpq/fe-exec.c 2009-12-22 11:56:06.0 +0100
*** PQcmdTuples(PGresult *res)
*** 2753,2758
--- 2753,2759
p++;
}
else if (strncmp(res-cmdStatus, DELETE , 7) == 0 ||
+ strncmp(res-cmdStatus, SELECT , 7) == 0 ||
strncmp(res-cmdStatus, UPDATE , 7) == 0)
p = res-cmdStatus + 7;
else if (strncmp(res-cmdStatus, FETCH , 6) == 0)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
2009/12/28 Boszormenyi Zoltan z...@cybertec.at: Hi, attached is a small patch that makes it possible for clients to receive row count for SELECT ... INTO ... and CREATE TABLE ... AS ... Comments? good idea +1 Pavel Best regards, Zoltán Böszörményi -- Bible has answers for everything. Proof: But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil. (Matthew 5:37) - basics of digital technology. May your kingdom come - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
hello ... just as a background info: this will have some positive side effects on embedded C programs which should be portable. informix, for instance, will also return a row count on those commands. regards, hans Pavel Stehule wrote: 2009/12/28 Boszormenyi Zoltan z...@cybertec.at: Hi, attached is a small patch that makes it possible for clients to receive row count for SELECT ... INTO ... and CREATE TABLE ... AS ... Comments? good idea +1 Pavel Best regards, Zoltán Böszörményi -- Bible has answers for everything. Proof: But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil. (Matthew 5:37) - basics of digital technology. May your kingdom come - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Cybertec Schoenig Schoenig GmbH Reyergasse 9 / 2 A-2700 Wiener Neustadt Web: www.postgresql-support.de -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Hans-Juergen Schoenig írta: hello ... just as a background info: this will have some positive side effects on embedded C programs which should be portable. Not just embedded C programs, every driver that's based on libpq and used PQcmdTuples() will automatically see the benefit. informix, for instance, will also return a row count on those commands. regards, hans Pavel Stehule wrote: 2009/12/28 Boszormenyi Zoltan z...@cybertec.at: Hi, attached is a small patch that makes it possible for clients to receive row count for SELECT ... INTO ... and CREATE TABLE ... AS ... Comments? good idea +1 Pavel Best regards, Zoltán Böszörményi -- Bible has answers for everything. Proof: But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil. (Matthew 5:37) - basics of digital technology. May your kingdom come - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Bible has answers for everything. Proof: But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil. (Matthew 5:37) - basics of digital technology. May your kingdom come - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Boszormenyi Zoltan z...@cybertec.at writes: Hans-Juergen Schoenig Ãrta: just as a background info: this will have some positive side effects on embedded C programs which should be portable. Not just embedded C programs, every driver that's based on libpq and used PQcmdTuples() will automatically see the benefit. And, by the same token, the scope for possibly breaking clients is nearly unlimited ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
On mån, 2009-12-28 at 11:08 -0500, Tom Lane wrote: Boszormenyi Zoltan z...@cybertec.at writes: Hans-Juergen Schoenig írta: just as a background info: this will have some positive side effects on embedded C programs which should be portable. Not just embedded C programs, every driver that's based on libpq and used PQcmdTuples() will automatically see the benefit. And, by the same token, the scope for possibly breaking clients is nearly unlimited ... Why is that? Are there programs out there that expect PQcmdTuples() to return something that is *not* the tuple count for these commands and will violently misbehave otherwise? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Peter Eisentraut pete...@gmx.net writes: On mån, 2009-12-28 at 11:08 -0500, Tom Lane wrote: And, by the same token, the scope for possibly breaking clients is nearly unlimited ... Why is that? Are there programs out there that expect PQcmdTuples() to return something that is *not* the tuple count for these commands and will violently misbehave otherwise? It's more the possibility of doing strcmp(tag, SELECT) on the command tag that worries me. Describing the API change here as being limited to PQcmdTuples misses the point rather completely: this is a protocol change, and could break both clients and non-libpq driver libraries. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Boszormenyi Zoltan z...@cybertec.at writes: attached is a small patch that makes it possible for clients to receive row count for SELECT ... INTO ... and CREATE TABLE ... AS ... Comments? This doesn't look tremendously well thought out to me. 1. As given, the patch changes the result not only for SELECT INTO but for any SELECT executed in PORTAL_MULTI_QUERY context (consider SELECTs added by rules for example). It seems like a pretty bad idea for the result of a statement to depend on context. 2. In the past we have regretted it when we made the same command tag sometimes have numbers attached and sometimes not (note the hack at the bottom of PortalRunMulti). It doesn't seem like terribly good design to do that here. On the other hand, always attaching a count to SELECT tags would greatly increase the risk of breaking clients. I'm not at all convinced that this is so useful as to justify taking any compatibility risks for. People who really need that count can get it easily enough by breaking the command into a CREATE followed by INSERT/SELECT. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Tom Lane írta: Peter Eisentraut pete...@gmx.net writes: On mĂĽn, 2009-12-28 at 11:08 -0500, Tom Lane wrote: And, by the same token, the scope for possibly breaking clients is nearly unlimited ... Why is that? Are there programs out there that expect PQcmdTuples() to return something that is *not* the tuple count for these commands and will violently misbehave otherwise? It's more the possibility of doing strcmp(tag, SELECT) on the command Actually it's strncmp(tag, SELECT , 7), so when you mix old server with new clients or new server with old client, it will just work as before, i.e. return . tag that worries me. Describing the API change here as being limited to PQcmdTuples misses the point rather completely: this is a protocol change, and could break both clients and non-libpq driver libraries. regards, tom lane -- Bible has answers for everything. Proof: But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil. (Matthew 5:37) - basics of digital technology. May your kingdom come - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Boszormenyi Zoltan z...@cybertec.at writes: Tom Lane írta: It's more the possibility of doing strcmp(tag, SELECT) on the command Actually it's strncmp(tag, SELECT , 7), so when you mix old server with new clients or new server with old client, it will just work as before, i.e. return . Are you deliberately ignoring the point? We have no way to know whether there is any client-side code that's doing a simple check for SELECT command tag, but it's certainly possible. The fact that it wouldn't be hard to fix does not mean that it wouldn't be broken. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Tom Lane írta: Boszormenyi Zoltan z...@cybertec.at writes: Tom Lane írta: It's more the possibility of doing strcmp(tag, SELECT) on the command Actually it's strncmp(tag, SELECT , 7), so when you mix old server with new clients or new server with old client, it will just work as before, i.e. return . Are you deliberately ignoring the point? No, I just thought you were commenting on my patch's details... We have no way to know whether there is any client-side code that's doing a simple check for SELECT command tag, but it's certainly possible. The fact that it wouldn't be hard to fix does not mean that it wouldn't be broken. regards, tom lane -- Bible has answers for everything. Proof: But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil. (Matthew 5:37) - basics of digital technology. May your kingdom come - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs
Tom Lane írta: Boszormenyi Zoltan z...@cybertec.at writes: attached is a small patch that makes it possible for clients to receive row count for SELECT ... INTO ... and CREATE TABLE ... AS ... Comments? This doesn't look tremendously well thought out to me. 1. As given, the patch changes the result not only for SELECT INTO but for any SELECT executed in PORTAL_MULTI_QUERY context (consider SELECTs added by rules for example). It seems like a pretty bad idea for the result of a statement to depend on context. 2. In the past we have regretted it when we made the same command tag sometimes have numbers attached and sometimes not (note the hack at the bottom of PortalRunMulti). It doesn't seem like terribly good design to do that here. On the other hand, always attaching a count to SELECT tags would greatly increase the risk of breaking clients. Okay, how about introducing a new SELECTINTO N command tag, then? It's also a protocol change, but at least it can fall into the very last else anywhere, hence have a high chance of being ignored and handled the same way as other not rowcount-returning tags. I'm not at all convinced that this is so useful as to justify taking any compatibility risks for. People who really need that count can get it easily enough by breaking the command into a CREATE followed by INSERT/SELECT. Yes, and every WITH RECURSIVE statement can also be broken up manually as well. It's simply shorter and has a chance of being a little more resource-effective: - one parsing/planning phase instead of two on the server side - one error checking in the app instead of two - PostgreSQL already has the infrastructure to return the rowcount Best regards, Zoltán Böszörményi -- Bible has answers for everything. Proof: But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil. (Matthew 5:37) - basics of digital technology. May your kingdom come - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
