Message-ID: <4A249B92.5080006@joeconway.com>
Date: Mon, 01 Jun 2009 20:25:06 -0700
From: Joe Conway <mail@joeconway.com>
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US;
	rv:1.8.0.12) Gecko/20071019 Fedora/1.5.0.12-3.fc6 pango-text
	Thunderbird/1.5.0.12 Mnenhy/0.7.5.666
MIME-Version: 1.0
To: Tom Lane <tgl@sss.pgh.pa.us>
CC: "Hackers (PostgreSQL)" <pgsql-hackers@postgresql.org>
Subject: Re: dblink patches for comment
References: <4A1C9C8C.6030405@joeconway.com> <20613.1243466644@sss.pgh.pa.us>
	<4A22DCCC.8030807@joeconway.com> <29955.1243904994@sss.pgh.pa.us>
In-Reply-To: <29955.1243904994@sss.pgh.pa.us>
Content-Type: multipart/mixed; boundary="------------020306090905060802050703"

This is a multi-part message in MIME format.
--------------020306090905060802050703
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>> Probably better if I break this up in logical chunks too. This patch 
>> only addresses the refactoring you requested here:
>> http://archives.postgresql.org/message-id/28719.1230996378@sss.pgh.pa.us
> 
> This looks sane to me in a quick once-over, though I've not tested it.

Thanks -- committed.

> A small suggestion for future patches: don't bother to reindent code
> chunks that aren't changing --- it just complicates the diff with a
> lot of uninteresting whitespace changes.  You can either do that after
> review, or leave it to be done by pgindent.  (Speaking of which, we
> need to schedule that soon...)

Sorry. "cvs diff -cb" seems to help (see attached). It is half the size 
and much more readable.

Joe

--------------020306090905060802050703
Content-Type: text/x-patch;
 name="dblink.2009.06.01.01-async_refactor.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
	filename="dblink.2009.06.01.01-async_refactor.diff"

Index: dblink.c
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.77
diff -c -b -r1.77 dblink.c
*** dblink.c	1 Jan 2009 17:23:31 -0000	1.77
--- dblink.c	2 Jun 2009 03:04:04 -0000
***************
*** 77,83 ****
  /*
   * Internal declarations
   */
! static Datum dblink_record_internal(FunctionCallInfo fcinfo, bool is_async, bool do_get);
  static remoteConn *getConnectionByName(const char *name);
  static HTAB *createConnHash(void);
  static void createNewConnection(const char *name, remoteConn * rconn);
--- 77,83 ----
  /*
   * Internal declarations
   */
! static Datum dblink_record_internal(FunctionCallInfo fcinfo, bool is_async);
  static remoteConn *getConnectionByName(const char *name);
  static HTAB *createConnHash(void);
  static void createNewConnection(const char *name, remoteConn * rconn);
***************
*** 689,713 ****
  Datum
  dblink_record(PG_FUNCTION_ARGS)
  {
! 	return dblink_record_internal(fcinfo, false, false);
  }
  
  PG_FUNCTION_INFO_V1(dblink_send_query);
  Datum
  dblink_send_query(PG_FUNCTION_ARGS)
  {
! 	return dblink_record_internal(fcinfo, true, false);
  }
  
  PG_FUNCTION_INFO_V1(dblink_get_result);
  Datum
  dblink_get_result(PG_FUNCTION_ARGS)
  {
! 	return dblink_record_internal(fcinfo, true, true);
  }
  
  static Datum
! dblink_record_internal(FunctionCallInfo fcinfo, bool is_async, bool do_get)
  {
  	FuncCallContext *funcctx;
  	TupleDesc	tupdesc = NULL;
--- 689,735 ----
  Datum
  dblink_record(PG_FUNCTION_ARGS)
  {
! 	return dblink_record_internal(fcinfo, false);
  }
  
  PG_FUNCTION_INFO_V1(dblink_send_query);
  Datum
  dblink_send_query(PG_FUNCTION_ARGS)
  {
! 	PGconn	   *conn = NULL;
! 	char	   *connstr = NULL;
! 	char	   *sql = NULL;
! 	remoteConn *rconn = NULL;
! 	char	   *msg;
! 	bool		freeconn = false;
! 	int			retval;
! 
! 	if (PG_NARGS() == 2)
! 	{
! 		DBLINK_GET_CONN;
! 		sql = text_to_cstring(PG_GETARG_TEXT_PP(1));
! 	}
! 	else
! 		/* shouldn't happen */
! 		elog(ERROR, "wrong number of arguments");
! 
! 	/* async query send */
! 	retval = PQsendQuery(conn, sql);
! 	if (retval != 1)
! 		elog(NOTICE, "%s", PQerrorMessage(conn));
! 
! 	PG_RETURN_INT32(retval);
  }
  
  PG_FUNCTION_INFO_V1(dblink_get_result);
  Datum
  dblink_get_result(PG_FUNCTION_ARGS)
  {
! 	return dblink_record_internal(fcinfo, true);
  }
  
  static Datum
! dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
  {
  	FuncCallContext *funcctx;
  	TupleDesc	tupdesc = NULL;
***************
*** 775,788 ****
  				/* shouldn't happen */
  				elog(ERROR, "wrong number of arguments");
  		}
! 		else if (is_async && do_get)
  		{
  			/* get async result */
  			if (PG_NARGS() == 2)
  			{
  				/* text,bool */
  				DBLINK_GET_CONN;
! 				fail = PG_GETARG_BOOL(2);
  			}
  			else if (PG_NARGS() == 1)
  			{
--- 797,810 ----
  				/* shouldn't happen */
  				elog(ERROR, "wrong number of arguments");
  		}
! 		else /* is_async */
  		{
  			/* get async result */
  			if (PG_NARGS() == 2)
  			{
  				/* text,bool */
  				DBLINK_GET_CONN;
! 				fail = PG_GETARG_BOOL(1);
  			}
  			else if (PG_NARGS() == 1)
  			{
***************
*** 793,816 ****
  				/* shouldn't happen */
  				elog(ERROR, "wrong number of arguments");
  		}
- 		else
- 		{
- 			/* send async query */
- 			if (PG_NARGS() == 2)
- 			{
- 				DBLINK_GET_CONN;
- 				sql = text_to_cstring(PG_GETARG_TEXT_PP(1));
- 			}
- 			else
- 				/* shouldn't happen */
- 				elog(ERROR, "wrong number of arguments");
- 		}
  
  		if (!conn)
  			DBLINK_CONN_NOT_AVAIL;
  
- 		if (!is_async || (is_async && do_get))
- 		{
  			/* synchronous query, or async result retrieval */
  			if (!is_async)
  				res = PQexec(conn, sql);
--- 815,824 ----
***************
*** 911,929 ****
  			funcctx->attinmeta = attinmeta;
  
  			MemoryContextSwitchTo(oldcontext);
- 		}
- 		else
- 		{
- 			/* async query send */
- 			MemoryContextSwitchTo(oldcontext);
- 			PG_RETURN_INT32(PQsendQuery(conn, sql));
- 		}
- 	}
- 
- 	if (is_async && !do_get)
- 	{
- 		/* async query send -- should not happen */
- 		elog(ERROR, "async query send called more than once");
  
  	}
  
--- 919,924 ----

--------------020306090905060802050703--