Skip site navigation (1) Skip section navigation (2)

Re: [PATCH] Provide rowcount for utility SELECTs

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Boszormenyi Zoltan <zb(at)cybertec(dot)at>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Hans-Juergen Schoenig <hs(at)cybertec(dot)at>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Provide rowcount for utility SELECTs
Date: 2010-02-16 20:58:23
Message-ID: 201002162058.o1GKwNH25481@momjian.us (view raw or flat)
Thread:
Lists: pgsql-hackers
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  <bruce(at)momjian(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

-- 
  Bruce Momjian  <bruce(at)momjian(dot)us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

In response to

pgsql-hackers by date

Next:From: Robert DoerflerDate: 2010-02-16 21:03:07
Subject: Re: OpenVMS?
Previous:From: Joshua D. DrakeDate: 2010-02-16 20:52:08
Subject: Re: OpenVMS?

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group