Change the interpretation of the primary_key_attnums parameter of

tglsfdc · tglsfdc · commit c0989c67fa8f · 2010-06-15T20:29:01.000Z
dblink_build_sql_insert() and related functions.  Now the column numbers
are treated as logical not physical column numbers.  This will provide saner
behavior in the presence of dropped columns; furthermore, if we ever get
around to allowing rearrangement of logical column ordering, the original
definition would become nearly untenable from a usability standpoint.
Per recent discussion of dblink's handling of dropped columns.
Not back-patched for fear of breaking existing applications.
diff --git a/contrib/dblink/dblink.c b/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.97 2010/06/15 19:04:15 tgl Exp $
+ * $PostgreSQL: pgsql/contrib/dblink/dblink.c,v 1.98 2010/06/15 20:29:01 tgl Exp $
  * Copyright (c) 2001-2010, PostgreSQL Global Development Group
  * ALL RIGHTS RESERVED;
  *
@@ -2381,11 +2381,13 @@ escape_param_str(const char *str)
  * Validate the PK-attnums argument for dblink_build_sql_insert() and related
  * functions, and translate to the internal representation.
  *
- * The user supplies an int2vector of 1-based physical attnums, plus a count
+ * The user supplies an int2vector of 1-based logical attnums, plus a count
  * argument (the need for the separate count argument is historical, but we
  * still check it).  We check that each attnum corresponds to a valid,
  * non-dropped attribute of the rel.  We do *not* prevent attnums from being
  * listed twice, though the actual use-case for such things is dubious.
+ * Note that before Postgres 9.0, the user's attnums were interpreted as
+ * physical not logical column numbers; this was changed for future-proofing.
  *
  * The internal representation is a palloc'd int array of 0-based physical
  * attnums.
@@ -2416,12 +2418,32 @@ validate_pkattnums(Relation rel,
 	for (i = 0; i < pknumatts_arg; i++)
 	{
 		int		pkattnum = pkattnums_arg->values[i];
+		int		lnum;
+		int		j;
 
-		if (pkattnum <= 0 || pkattnum > natts ||
-			tupdesc->attrs[pkattnum - 1]->attisdropped)
+		/* Can throw error immediately if out of range */
+		if (pkattnum <= 0 || pkattnum > natts)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("invalid attribute number %d", pkattnum)));
+
+		/* Identify which physical column has this logical number */
+		lnum = 0;
+		for (j = 0; j < natts; j++)
+		{
+			/* dropped columns don't count */
+			if (tupdesc->attrs[j]->attisdropped)
+				continue;
+
+			if (++lnum == pkattnum)
+				break;
+		}
+
+		if (j < natts)
+			(*pkattnums)[i] = j;
+		else
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 					 errmsg("invalid attribute number %d", pkattnum)));
-		(*pkattnums)[i] = pkattnum - 1;
 	}
 }
diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out
@@ -903,21 +903,21 @@ ALTER TABLE test_dropped
 	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,
+SELECT dblink_build_sql_insert('test_dropped', '1', 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,
+SELECT dblink_build_sql_update('test_dropped', '1', 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,
+SELECT dblink_build_sql_delete('test_dropped', '1', 1,
                                ARRAY['2'::TEXT]);
          dblink_build_sql_delete         
 -----------------------------------------
diff --git a/contrib/dblink/sql/dblink.sql b/contrib/dblink/sql/dblink.sql
@@ -430,11 +430,11 @@ ALTER TABLE test_dropped
 	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,
+SELECT dblink_build_sql_insert('test_dropped', '1', 1,
                                ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
 
-SELECT dblink_build_sql_update('test_dropped', '2', 1,
+SELECT dblink_build_sql_update('test_dropped', '1', 1,
                                ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
 
-SELECT dblink_build_sql_delete('test_dropped', '2', 1,
+SELECT dblink_build_sql_delete('test_dropped', '1', 1,
                                ARRAY['2'::TEXT]);
diff --git a/doc/src/sgml/dblink.sgml b/doc/src/sgml/dblink.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/dblink.sgml,v 1.12 2010/06/07 02:01:08 itagaki Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/dblink.sgml,v 1.13 2010/06/15 20:29:01 tgl Exp $ -->
 
 <sect1 id="dblink">
  <title>dblink</title>
@@ -1294,9 +1294,9 @@ SELECT *
    <title>Description</title>
 
    <para>
-    <function>dblink_get_notify</> retrieves notifications on either 
+    <function>dblink_get_notify</> retrieves notifications on either
     the unnamed connection, or on a named connection if specified.
-    To receive notifications via dblink, <function>LISTEN</> must 
+    To receive notifications via dblink, <function>LISTEN</> must
     first be issued, using <function>dblink_exec</>.
     For details see <xref linkend="sql-listen"> and <xref linkend="sql-notify">.
    </para>
@@ -1620,6 +1620,10 @@ SELECT * FROM dblink_get_notify();
     <programlisting>
 CREATE TYPE dblink_pkey_results AS (position int, colname text);
     </programlisting>
+
+    The <literal>position</> column simply runs from 1 to <replaceable>N</>;
+    it is the number of the field within the primary key, not the number
+    within the table's columns.
    </para>
   </refsect1>
 
@@ -1659,7 +1663,7 @@ test=# select * from dblink_get_pkey('foobar');
    <synopsis>
     dblink_build_sql_insert(text relname,
                             int2vector primary_key_attnums,
-                            int2 num_primary_key_atts,
+                            integer num_primary_key_atts,
                             text[] src_pk_att_vals_array,
                             text[] tgt_pk_att_vals_array) returns text
    </synopsis>
@@ -1745,6 +1749,20 @@ test=# select * from dblink_get_pkey('foobar');
    <para>Returns the requested SQL statement as text.</para>
   </refsect1>
 
+  <refsect1>
+   <title>Notes</title>
+
+   <para>
+    As of <productname>PostgreSQL</> 9.0, the attribute numbers in
+    <parameter>primary_key_attnums</parameter> are interpreted as logical
+    column numbers, corresponding to the column's position in
+    <literal>SELECT * FROM relname</>.  Previous versions interpreted the
+    numbers as physical column positions.  There is a difference if any
+    column(s) to the left of the indicated column have been dropped during
+    the lifetime of the table.
+   </para>
+  </refsect1>
+
   <refsect1>
    <title>Example</title>
 
@@ -1775,7 +1793,7 @@ test=# select * from dblink_get_pkey('foobar');
    <synopsis>
     dblink_build_sql_delete(text relname,
                             int2vector primary_key_attnums,
-                            int2 num_primary_key_atts,
+                            integer num_primary_key_atts,
                             text[] tgt_pk_att_vals_array) returns text
    </synopsis>
   </refsynopsisdiv>
@@ -1845,6 +1863,20 @@ test=# select * from dblink_get_pkey('foobar');
    <para>Returns the requested SQL statement as text.</para>
   </refsect1>
 
+  <refsect1>
+   <title>Notes</title>
+
+   <para>
+    As of <productname>PostgreSQL</> 9.0, the attribute numbers in
+    <parameter>primary_key_attnums</parameter> are interpreted as logical
+    column numbers, corresponding to the column's position in
+    <literal>SELECT * FROM relname</>.  Previous versions interpreted the
+    numbers as physical column positions.  There is a difference if any
+    column(s) to the left of the indicated column have been dropped during
+    the lifetime of the table.
+   </para>
+  </refsect1>
+
   <refsect1>
    <title>Example</title>
 
@@ -1875,7 +1907,7 @@ test=# select * from dblink_get_pkey('foobar');
    <synopsis>
     dblink_build_sql_update(text relname,
                             int2vector primary_key_attnums,
-                            int2 num_primary_key_atts,
+                            integer num_primary_key_atts,
                             text[] src_pk_att_vals_array,
                             text[] tgt_pk_att_vals_array) returns text
    </synopsis>
@@ -1964,6 +1996,20 @@ test=# select * from dblink_get_pkey('foobar');
    <para>Returns the requested SQL statement as text.</para>
   </refsect1>
 
+  <refsect1>
+   <title>Notes</title>
+
+   <para>
+    As of <productname>PostgreSQL</> 9.0, the attribute numbers in
+    <parameter>primary_key_attnums</parameter> are interpreted as logical
+    column numbers, corresponding to the column's position in
+    <literal>SELECT * FROM relname</>.  Previous versions interpreted the
+    numbers as physical column positions.  There is a difference if any
+    column(s) to the left of the indicated column have been dropped during
+    the lifetime of the table.
+   </para>
+  </refsect1>
+
   <refsect1>
    <title>Example</title>
 
