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 <command>SELECT</command> or <command>CREATE 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.us> http://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 -0000 1.297 > --- doc/src/sgml/libpq.sgml 14 Feb 2010 03:11:00 -0000 > *************** > *** 2869,2880 **** > </sect2> > > <sect2 id="libpq-exec-nonselect"> > ! <title>Retrieving Result Information for Other Commands</title> > > <para> > ! These functions are used to extract information from > ! <structname>PGresult</structname> objects that are not > ! <command>SELECT</> results. > </para> > > <variablelist> > --- 2869,2879 ---- > </sect2> > > <sect2 id="libpq-exec-nonselect"> > ! <title>Retrieving Other Result Information</title> > > <para> > ! These functions are used to extract other information from > ! <structname>PGresult</structname> objects. > </para> > > <variablelist> > *************** > *** 2925,2936 **** > This function returns a string containing the number of rows > affected by the <acronym>SQL</> statement that generated the > <structname>PGresult</>. This function can only be used following > ! the execution of an <command>INSERT</>, <command>UPDATE</>, > ! <command>DELETE</>, <command>MOVE</>, <command>FETCH</>, or > ! <command>COPY</> statement, or an <command>EXECUTE</> of a > ! prepared query that contains an <command>INSERT</>, > ! <command>UPDATE</>, or <command>DELETE</> statement. If the > ! command that generated the <structname>PGresult</> was anything > else, <function>PQcmdTuples</> returns an empty string. The caller > should not free the return value directly. It will be freed when > the associated <structname>PGresult</> handle is passed to > --- 2924,2935 ---- > This function returns a string containing the number of rows > affected by the <acronym>SQL</> statement that generated the > <structname>PGresult</>. This function can only be used following > ! the execution of a <command>SELECT</>, <command>CREATE TABLE AS</>, > ! <command>INSERT</>, <command>UPDATE</>, <command>DELETE</>, > ! <command>MOVE</>, <command>FETCH</>, or <command>COPY</> statement, > ! or an <command>EXECUTE</> of a prepared query that contains an > ! <command>INSERT</>, <command>UPDATE</>, or <command>DELETE</> > statement. > ! If the command that generated the <structname>PGresult</> was > anything > else, <function>PQcmdTuples</> returns an empty string. The caller > should not free the return value directly. It will be freed when > the associated <structname>PGresult</> handle is passed to > Index: doc/src/sgml/protocol.sgml > =================================================================== > RCS file: /cvsroot/pgsql/doc/src/sgml/protocol.sgml,v > retrieving revision 1.78 > diff -c -c -r1.78 protocol.sgml > *** doc/src/sgml/protocol.sgml 3 Feb 2010 09:47:19 -0000 1.78 > --- doc/src/sgml/protocol.sgml 14 Feb 2010 03:11:00 -0000 > *************** > *** 2222,2227 **** > --- 2222,2233 ---- > </para> > > <para> > + For a <command>SELECT</command> or <command>CREATE TABLE > AS</command> > + command, the tag is <literal>SELECT > <replaceable>rows</replaceable></literal> > + where <replaceable>rows</replaceable> is the number of rows > retrieved. > + </para> > + > + <para> > For a <command>MOVE</command> command, the tag is > <literal>MOVE <replaceable>rows</replaceable></literal> where > <replaceable>rows</replaceable> is the number of rows the > Index: src/backend/tcop/pquery.c > =================================================================== > RCS file: /cvsroot/pgsql/src/backend/tcop/pquery.c,v > retrieving revision 1.135 > diff -c -c -r1.135 pquery.c > *** src/backend/tcop/pquery.c 13 Feb 2010 22:45:41 -0000 1.135 > --- src/backend/tcop/pquery.c 14 Feb 2010 03:11:04 -0000 > *************** > *** 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) > *************** > *** 714,719 **** > --- 715,721 ---- > char *completionTag) > { > bool result; > + uint32 nprocessed; > ResourceOwner saveTopTransactionResourceOwner; > MemoryContext saveTopTransactionContext; > Portal saveActivePortal; > *************** > *** 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; > *************** > *** 1331,1337 **** > { > 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"); > --- 1329,1337 ---- > { > if (portal->commandTag) > strcpy(completionTag, portal->commandTag); > ! if (strcmp(completionTag, "SELECT") == 0) > ! sprintf(completionTag, "SELECT 0 0"); > ! else if (strcmp(completionTag, "INSERT") == 0) > strcpy(completionTag, "INSERT 0 0"); > else if (strcmp(completionTag, "UPDATE") == 0) > strcpy(completionTag, "UPDATE 0"); > Index: src/interfaces/libpq/fe-exec.c > =================================================================== > RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v > retrieving revision 1.208 > diff -c -c -r1.208 fe-exec.c > *** src/interfaces/libpq/fe-exec.c 21 Jan 2010 18:43:25 -0000 1.208 > --- src/interfaces/libpq/fe-exec.c 14 Feb 2010 03:11:04 -0000 > *************** > *** 2752,2758 **** > goto interpret_error; /* no space? */ > p++; > } > ! else if (strncmp(res->cmdStatus, "DELETE ", 7) == 0 || > strncmp(res->cmdStatus, "UPDATE ", 7) == 0) > p = res->cmdStatus + 7; > else if (strncmp(res->cmdStatus, "FETCH ", 6) == 0) > --- 2752,2759 ---- > goto interpret_error; /* no space? */ > p++; > } > ! else if (strncmp(res->cmdStatus, "SELECT ", 7) == 0 || > ! strncmp(res->cmdStatus, "DELETE ", 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 -- Bruce Momjian <br...@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
