On 06/14/2010 11:21 AM, Tom Lane wrote:
> Actually, I was working on it myself.  On further reflection I think
> that logical numbers are clearly the right thing --- if we define it
> as being physical numbers then we will have headaches in the future
> when/if we support rearranging columns.  However, there is some small
> chance of breaking things in existing DBs if we back-patch that change.
> Thoughts?

I didn't even think people were using those functions for many years
since I never heard any complaints. I'd say better to not backpatch
changes to logical ordering, but FWIW the attached at least fixes the
immediate bug in head and ought to work at least a few branches.

> It strikes me also that the code is not nearly careful enough about
> defending itself against garbage input in the primary_key_attnums
> argument ...

Probably not :-(

Joe

Index: dblink.c
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.94
diff -c -r1.94 dblink.c
*** dblink.c	9 Jun 2010 03:39:26 -0000	1.94
--- dblink.c	14 Jun 2010 18:37:07 -0000
***************
*** 94,100 ****
  static char *quote_literal_cstr(char *rawstr);
  static char *quote_ident_cstr(char *rawstr);
  static int16 get_attnum_pk_pos(int2vector *pkattnums, int16 pknumatts, int16 key);
! static HeapTuple get_tuple_of_interest(Oid relid, int2vector *pkattnums, int16 pknumatts, char **src_pkattvals);
  static Oid	get_relid_from_relname(text *relname_text);
  static char *generate_relation_name(Oid relid);
  static void dblink_connstr_check(const char *connstr);
--- 94,100 ----
  static char *quote_literal_cstr(char *rawstr);
  static char *quote_ident_cstr(char *rawstr);
  static int16 get_attnum_pk_pos(int2vector *pkattnums, int16 pknumatts, int16 key);
! static HeapTuple get_tuple_of_interest(Oid relid, int2vector *pkattnums, int16 pknumatts, char **src_pkattvals, TupleDesc *reltupdesc);
  static Oid	get_relid_from_relname(text *relname_text);
  static char *generate_relation_name(Oid relid);
  static void dblink_connstr_check(const char *connstr);
***************
*** 1768,1774 ****
  static char *
  get_sql_insert(Oid relid, int2vector *pkattnums, int16 pknumatts, char **src_pkattvals, char **tgt_pkattvals)
  {
- 	Relation	rel;
  	char	   *relname;
  	HeapTuple	tuple;
  	TupleDesc	tupdesc;
--- 1768,1773 ----
***************
*** 1784,1801 ****
  	/* get relation name including any needed schema prefix and quoting */
  	relname = generate_relation_name(relid);
  
! 	/*
! 	 * Open relation using relid
! 	 */
! 	rel = relation_open(relid, AccessShareLock);
! 	tupdesc = rel->rd_att;
! 	natts = tupdesc->natts;
! 
! 	tuple = get_tuple_of_interest(relid, pkattnums, pknumatts, src_pkattvals);
  	if (!tuple)
  		ereport(ERROR,
  				(errcode(ERRCODE_CARDINALITY_VIOLATION),
  				 errmsg("source row not found")));
  
  	appendStringInfo(&buf, "INSERT INTO %s(", relname);
  
--- 1783,1794 ----
  	/* get relation name including any needed schema prefix and quoting */
  	relname = generate_relation_name(relid);
  
! 	tuple = get_tuple_of_interest(relid, pkattnums, pknumatts, src_pkattvals, &tupdesc);
  	if (!tuple)
  		ereport(ERROR,
  				(errcode(ERRCODE_CARDINALITY_VIOLATION),
  				 errmsg("source row not found")));
+ 	natts = tupdesc->natts;
  
  	appendStringInfo(&buf, "INSERT INTO %s(", relname);
  
***************
*** 1848,1854 ****
  	}
  	appendStringInfo(&buf, ")");
  
- 	relation_close(rel, AccessShareLock);
  	return (buf.data);
  }
  
--- 1841,1846 ----
***************
*** 1903,1909 ****
  static char *
  get_sql_update(Oid relid, int2vector *pkattnums, int16 pknumatts, char **src_pkattvals, char **tgt_pkattvals)
  {
- 	Relation	rel;
  	char	   *relname;
  	HeapTuple	tuple;
  	TupleDesc	tupdesc;
--- 1895,1900 ----
***************
*** 1919,1936 ****
  	/* get relation name including any needed schema prefix and quoting */
  	relname = generate_relation_name(relid);
  
! 	/*
! 	 * Open relation using relid
! 	 */
! 	rel = relation_open(relid, AccessShareLock);
! 	tupdesc = rel->rd_att;
! 	natts = tupdesc->natts;
! 
! 	tuple = get_tuple_of_interest(relid, pkattnums, pknumatts, src_pkattvals);
  	if (!tuple)
  		ereport(ERROR,
  				(errcode(ERRCODE_CARDINALITY_VIOLATION),
  				 errmsg("source row not found")));
  
  	appendStringInfo(&buf, "UPDATE %s SET ", relname);
  
--- 1910,1921 ----
  	/* get relation name including any needed schema prefix and quoting */
  	relname = generate_relation_name(relid);
  
! 	tuple = get_tuple_of_interest(relid, pkattnums, pknumatts, src_pkattvals, &tupdesc);
  	if (!tuple)
  		ereport(ERROR,
  				(errcode(ERRCODE_CARDINALITY_VIOLATION),
  				 errmsg("source row not found")));
+ 	natts = tupdesc->natts;
  
  	appendStringInfo(&buf, "UPDATE %s SET ", relname);
  
***************
*** 1992,1998 ****
  			appendStringInfo(&buf, " IS NULL");
  	}
  
- 	relation_close(rel, AccessShareLock);
  	return (buf.data);
  }
  
--- 1977,1982 ----
***************
*** 2050,2064 ****
  }
  
  static HeapTuple
! get_tuple_of_interest(Oid relid, int2vector *pkattnums, int16 pknumatts, char **src_pkattvals)
  {
  	Relation	rel;
  	char	   *relname;
- 	TupleDesc	tupdesc;
  	StringInfoData buf;
  	int			ret;
  	HeapTuple	tuple;
  	int			i;
  
  	initStringInfo(&buf);
  
--- 2034,2049 ----
  }
  
  static HeapTuple
! get_tuple_of_interest(Oid relid, int2vector *pkattnums, int16 pknumatts, char **src_pkattvals, TupleDesc *tupdesc)
  {
  	Relation	rel;
  	char	   *relname;
  	StringInfoData buf;
  	int			ret;
+ 	TupleDesc	local_tupdesc;
  	HeapTuple	tuple;
  	int			i;
+ 	MemoryContext caller_cxt = CurrentMemoryContext;
  
  	initStringInfo(&buf);
  
***************
*** 2069,2075 ****
  	 * Open relation using relid
  	 */
  	rel = relation_open(relid, AccessShareLock);
! 	tupdesc = CreateTupleDescCopy(rel->rd_att);
  	relation_close(rel, AccessShareLock);
  
  	/*
--- 2054,2060 ----
  	 * Open relation using relid
  	 */
  	rel = relation_open(relid, AccessShareLock);
! 	local_tupdesc = CreateTupleDescCopy(rel->rd_att);
  	relation_close(rel, AccessShareLock);
  
  	/*
***************
*** 2093,2099 ****
  			appendStringInfo(&buf, " AND ");
  
  		appendStringInfoString(&buf,
! 		   quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum - 1]->attname)));
  
  		if (src_pkattvals[i] != NULL)
  			appendStringInfo(&buf, " = %s",
--- 2078,2084 ----
  			appendStringInfo(&buf, " AND ");
  
  		appendStringInfoString(&buf,
! 		   quote_ident_cstr(NameStr(local_tupdesc->attrs[pkattnum - 1]->attname)));
  
  		if (src_pkattvals[i] != NULL)
  			appendStringInfo(&buf, " = %s",
***************
*** 2118,2124 ****
  
  	else if (ret == SPI_OK_SELECT && SPI_processed == 1)
  	{
! 		SPITupleTable *tuptable = SPI_tuptable;
  
  		tuple = SPI_copytuple(tuptable->vals[0]);
  		SPI_finish();
--- 2103,2117 ----
  
  	else if (ret == SPI_OK_SELECT && SPI_processed == 1)
  	{
! 		SPITupleTable  *tuptable = SPI_tuptable;
! 		TupleDesc		spi_tupdesc;
! 		MemoryContext	oldcxt;
! 
! 		oldcxt = MemoryContextSwitchTo(caller_cxt);
! 		spi_tupdesc = CreateTupleDescCopy(tuptable->tupdesc);
! 		MemoryContextSwitchTo(oldcxt);
! 
! 		*tupdesc = spi_tupdesc;
  
  		tuple = SPI_copytuple(tuptable->vals[0]);
  		SPI_finish();

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to