Fix dblink_build_sql_insert() and related functions to handle dropped

columns correctly.  In passing, get rid of some dead logic in the
underlying get_sql_insert() etc functions --- there is no caller that
will pass null value-arrays to them.

Per bug report from Robert Voinea.

contrib/dblink/dblink.c

@@ -8,7 +8,7 @@
  * Darko Prenosil <Darko.Prenosil@finteh.hr>
  * Shridhar Daithankar <shridhar_daithankar@persistent.co.in>
  *
- * $PostgreSQL: pgsql/contrib/dblink/dblink.c,v 1.82.2.7 2010/06/15 16:22:26 tgl Exp $
+ * $PostgreSQL: pgsql/contrib/dblink/dblink.c,v 1.82.2.8 2010/06/15 19:04:22 tgl Exp $
  * Copyright (c) 2001-2009, PostgreSQL Global Development Group
  * ALL RIGHTS RESERVED;
  *
@@ -1777,7 +1777,7 @@ get_sql_insert(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
 	appendStringInfo(&buf, ") VALUES(");
 
 	/*
-	 * remember attvals are 1 based
+	 * Note: i is physical column number (counting from 0).
 	 */
 	needComma = false;
 	for (i = 0; i < natts; i++)
@@ -1788,12 +1788,9 @@ get_sql_insert(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
 		if (needComma)
 			appendStringInfo(&buf, ",");
 
-		if (tgt_pkattvals != NULL)
+		key = get_attnum_pk_pos(pkattnums, pknumatts, i);
-			key = get_attnum_pk_pos(pkattnums, pknumatts, i);
-		else
-			key = -1;
 
-		if (key > -1)
+		if (key >= 0)
 			val = tgt_pkattvals[key] ? pstrdup(tgt_pkattvals[key]) : NULL;
 		else
 			val = SPI_getvalue(tuple, tupdesc, i + 1);
@@ -1838,10 +1835,6 @@ get_sql_delete(Relation rel, int *pkattnums, int pknumatts, char **tgt_pkattvals
 		appendStringInfoString(&buf,
 		   quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));
 
-		if (tgt_pkattvals == NULL)
-			/* internal error */
-			elog(ERROR, "target key array must not be NULL");
-
 		if (tgt_pkattvals[i] != NULL)
 			appendStringInfo(&buf, " = %s",
 							 quote_literal_cstr(tgt_pkattvals[i]));
@@ -1881,6 +1874,9 @@ get_sql_update(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
 
 	appendStringInfo(&buf, "UPDATE %s SET ", relname);
 
+	/*
+	 * Note: i is physical column number (counting from 0).
+	 */
 	needComma = false;
 	for (i = 0; i < natts; i++)
 	{
@@ -1893,12 +1889,9 @@ get_sql_update(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
 		appendStringInfo(&buf, "%s = ",
 					  quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));
 
-		if (tgt_pkattvals != NULL)
+		key = get_attnum_pk_pos(pkattnums, pknumatts, i);
-			key = get_attnum_pk_pos(pkattnums, pknumatts, i);
-		else
-			key = -1;
 
-		if (key > -1)
+		if (key >= 0)
 			val = tgt_pkattvals[key] ? pstrdup(tgt_pkattvals[key]) : NULL;
 		else
 			val = SPI_getvalue(tuple, tupdesc, i + 1);
@@ -1925,16 +1918,10 @@ get_sql_update(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
 		appendStringInfo(&buf, "%s",
 		   quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));
 
-		if (tgt_pkattvals != NULL)
+		val = tgt_pkattvals[i];
-			val = tgt_pkattvals[i] ? pstrdup(tgt_pkattvals[i]) : NULL;
-		else
-			val = SPI_getvalue(tuple, tupdesc, pkattnum + 1);
 
 		if (val != NULL)
-		{
 			appendStringInfo(&buf, " = %s", quote_literal_cstr(val));
-			pfree(val);
-		}
 		else
 			appendStringInfo(&buf, " IS NULL");
 	}
@@ -2000,30 +1987,49 @@ get_tuple_of_interest(Relation rel, int *pkattnums, int pknumatts, char **src_pk
 {
 	char	   *relname;
 	TupleDesc	tupdesc;
+	int			natts;
 	StringInfoData buf;
 	int			ret;
 	HeapTuple	tuple;
 	int			i;
 
+	/*
+	 * Connect to SPI manager
+	 */
+	if ((ret = SPI_connect()) < 0)
+		/* internal error */
+		elog(ERROR, "SPI connect failure - returned %d", ret);
+
 	initStringInfo(&buf);
 
 	/* get relation name including any needed schema prefix and quoting */
 	relname = generate_relation_name(rel);
 
 	tupdesc = rel->rd_att;
+	natts = tupdesc->natts;
 
 	/*
-	 * Connect to SPI manager
+	 * Build sql statement to look up tuple of interest, ie, the one matching
+	 * src_pkattvals.  We used to use "SELECT *" here, but it's simpler to
+	 * generate a result tuple that matches the table's physical structure,
+	 * with NULLs for any dropped columns.  Otherwise we have to deal with
+	 * two different tupdescs and everything's very confusing.
 	 */
-	if ((ret = SPI_connect()) < 0)
+	appendStringInfoString(&buf, "SELECT ");
-		/* internal error */
-		elog(ERROR, "SPI connect failure - returned %d", ret);
 
-	/*
+	for (i = 0; i < natts; i++)
-	 * Build sql statement to look up tuple of interest Use src_pkattvals as
+	{
-	 * the criteria.
+		if (i > 0)
-	 */
+			appendStringInfoString(&buf, ", ");
-	appendStringInfo(&buf, "SELECT * FROM %s WHERE ", relname);
+
+		if (tupdesc->attrs[i]->attisdropped)
+			appendStringInfoString(&buf, "NULL");
+		else
+			appendStringInfoString(&buf,
+								   quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));
+	}
+
+	appendStringInfo(&buf, " FROM %s WHERE ", relname);
 
 	for (i = 0; i < pknumatts; i++)
 	{

contrib/dblink/expected/dblink.out

@@ -836,3 +836,40 @@ DROP USER dblink_regression_test;
 DROP USER MAPPING FOR public SERVER fdtest;
 DROP SERVER fdtest;
 DROP FOREIGN DATA WRAPPER postgresql;
+-- test dropped columns in dblink_build_sql_insert, dblink_build_sql_update
+CREATE TEMP TABLE test_dropped
+(
+	col1 INT NOT NULL DEFAULT 111,
+	id SERIAL PRIMARY KEY,
+	col2 INT NOT NULL DEFAULT 112,
+	col2b INT NOT NULL DEFAULT 113
+);
+NOTICE:  CREATE TABLE will create implicit sequence "test_dropped_id_seq" for serial column "test_dropped.id"
+NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "test_dropped_pkey" for table "test_dropped"
+INSERT INTO test_dropped VALUES(default);
+ALTER TABLE test_dropped
+	DROP COLUMN col1,
+	DROP COLUMN col2,
+	ADD COLUMN col3 VARCHAR(10) NOT NULL DEFAULT 'foo',
+	ADD COLUMN col4 INT NOT NULL DEFAULT 42;
+SELECT dblink_build_sql_insert('test_dropped', '2', 1,
+                               ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
+                          dblink_build_sql_insert                          
+---------------------------------------------------------------------------
+ INSERT INTO test_dropped(id,col2b,col3,col4) VALUES('2','113','foo','42')
+(1 row)
+
+SELECT dblink_build_sql_update('test_dropped', '2', 1,
+                               ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
+                                  dblink_build_sql_update                                  
+-------------------------------------------------------------------------------------------
+ UPDATE test_dropped SET id = '2', col2b = '113', col3 = 'foo', col4 = '42' WHERE id = '2'
+(1 row)
+
+SELECT dblink_build_sql_delete('test_dropped', '2', 1,
+                               ARRAY['2'::TEXT]);
+         dblink_build_sql_delete         
+-----------------------------------------
+ DELETE FROM test_dropped WHERE id = '2'
+(1 row)
+

contrib/dblink/sql/dblink.sql

@@ -395,3 +395,29 @@ DROP USER dblink_regression_test;
 DROP USER MAPPING FOR public SERVER fdtest;
 DROP SERVER fdtest;
 DROP FOREIGN DATA WRAPPER postgresql;
+
+-- test dropped columns in dblink_build_sql_insert, dblink_build_sql_update
+CREATE TEMP TABLE test_dropped
+(
+	col1 INT NOT NULL DEFAULT 111,
+	id SERIAL PRIMARY KEY,
+	col2 INT NOT NULL DEFAULT 112,
+	col2b INT NOT NULL DEFAULT 113
+);
+
+INSERT INTO test_dropped VALUES(default);
+
+ALTER TABLE test_dropped
+	DROP COLUMN col1,
+	DROP COLUMN col2,
+	ADD COLUMN col3 VARCHAR(10) NOT NULL DEFAULT 'foo',
+	ADD COLUMN col4 INT NOT NULL DEFAULT 42;
+
+SELECT dblink_build_sql_insert('test_dropped', '2', 1,
+                               ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
+
+SELECT dblink_build_sql_update('test_dropped', '2', 1,
+                               ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
+
+SELECT dblink_build_sql_delete('test_dropped', '2', 1,
+                               ARRAY['2'::TEXT]);