projects / postgresql.git / commitdiff
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 240076b)
Fix dblink_build_sql_insert() and related functions to handle dropped
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 15 Jun 2010 19:04:59 +0000 (19:04 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 15 Jun 2010 19:04:59 +0000 (19:04 +0000)
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 patch | blob | blame | history
contrib/dblink/expected/dblink.out patch | blob | blame | history
contrib/dblink/sql/dblink.sql patch | blob | blame | history

diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index b5e1c89370bdc6b288637e9fccef51680bb84031..783977e4010504d5b36f5b9ba3ae4f1f15735936 100644 (file)
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -1456,7 +1456,7 @@ get_sql_insert(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
    appendStringInfo(str, ") VALUES(");
 
    /*
-    * remember attvals are 1 based
+    * Note: i is physical column number (counting from 0).
     */
    needComma = false;
    for (i = 0; i < natts; i++)
@@ -1467,12 +1467,9 @@ get_sql_insert(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
        if (needComma)
            appendStringInfo(str, ",");
 
-       if (tgt_pkattvals != NULL)
-           key = get_attnum_pk_pos(pkattnums, pknumatts, i);
-       else
-           key = -1;
+       key = get_attnum_pk_pos(pkattnums, pknumatts, i);
 
-       if (key > -1)
+       if (key >= 0)
            val = pstrdup(tgt_pkattvals[key]);
        else
            val = SPI_getvalue(tuple, tupdesc, i + 1);
@@ -1503,7 +1500,6 @@ get_sql_delete(Relation rel, int *pkattnums, int pknumatts, char **tgt_pkattvals
    int         natts;
    StringInfo  str = makeStringInfo();
    char       *sql;
-   char       *val = NULL;
    int         i;
 
    /* get relation name including any needed schema prefix and quoting */
@@ -1523,17 +1519,9 @@ get_sql_delete(Relation rel, int *pkattnums, int pknumatts, char **tgt_pkattvals
        appendStringInfo(str, "%s",
        quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));
 
-       if (tgt_pkattvals != NULL)
-           val = pstrdup(tgt_pkattvals[i]);
-       else
-           /* internal error */
-           elog(ERROR, "target key array must not be NULL");
-
-       if (val != NULL)
-       {
-           appendStringInfo(str, " = %s", quote_literal_cstr(val));
-           pfree(val);
-       }
+       if (tgt_pkattvals[i] != NULL)
+           appendStringInfo(str, " = %s",
+                            quote_literal_cstr(tgt_pkattvals[i]));
        else
            appendStringInfo(str, " IS NULL");
    }
@@ -1585,12 +1573,9 @@ get_sql_update(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
        appendStringInfo(str, "%s = ",
                  quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));
 
-       if (tgt_pkattvals != NULL)
-           key = get_attnum_pk_pos(pkattnums, pknumatts, i);
-       else
-           key = -1;
+       key = get_attnum_pk_pos(pkattnums, pknumatts, i);
 
-       if (key > -1)
+       if (key >= 0)
            val = pstrdup(tgt_pkattvals[key]);
        else
            val = SPI_getvalue(tuple, tupdesc, i + 1);
@@ -1617,16 +1602,10 @@ get_sql_update(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
        appendStringInfo(str, "%s",
        quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));
 
-       if (tgt_pkattvals != NULL)
-           val = pstrdup(tgt_pkattvals[i]);
-       else
-           val = SPI_getvalue(tuple, tupdesc, pkattnum + 1);
+       val = tgt_pkattvals[i];
 
        if (val != NULL)
-       {
            appendStringInfo(str, " = %s", quote_literal_cstr(val));
-           pfree(val);
-       }
        else
            appendStringInfo(str, " IS NULL");
    }
@@ -1694,6 +1673,7 @@ get_tuple_of_interest(Relation rel, int *pkattnums, int pknumatts, char **src_pk
 {
    char       *relname;
    TupleDesc   tupdesc;
+   int         natts;
    StringInfo  str = makeStringInfo();
    char       *sql = NULL;
    int         ret;
@@ -1701,11 +1681,6 @@ get_tuple_of_interest(Relation rel, int *pkattnums, int pknumatts, char **src_pk
    int         i;
    char       *val = NULL;
 
-   /* get relation name including any needed schema prefix and quoting */
-   relname = generate_relation_name(rel);
-
-   tupdesc = rel->rd_att;
-
    /*
     * Connect to SPI manager
     */
@@ -1713,11 +1688,34 @@ get_tuple_of_interest(Relation rel, int *pkattnums, int pknumatts, char **src_pk
        /* internal error */
        elog(ERROR, "SPI connect failure - returned %d", ret);
 
+   /* get relation name including any needed schema prefix and quoting */
+   relname = generate_relation_name(rel);
+
+   tupdesc = rel->rd_att;
+   natts = tupdesc->natts;
+
    /*
-    * Build sql statement to look up tuple of interest Use src_pkattvals
-    * as the criteria.
+    * 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.
     */
-   appendStringInfo(str, "SELECT * FROM %s WHERE ", relname);
+   appendStringInfoString(str, "SELECT ");
+
+   for (i = 0; i < natts; i++)
+   {
+       if (i > 0)
+           appendStringInfoString(str, ", ");
+
+       if (tupdesc->attrs[i]->attisdropped)
+           appendStringInfoString(str, "NULL");
+       else
+           appendStringInfoString(str,
+                                  quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));
+   }
+
+   appendStringInfo(str, " FROM %s WHERE ", relname);
 
    for (i = 0; i < pknumatts; i++)
    {
diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out
index cbb272c74f64e6a6c6e8944e6456aee897dae8a7..9bc7bae2a20bee203c9c66c75059cfc3fb607211 100644 (file)
--- a/contrib/dblink/expected/dblink.out
+++ b/contrib/dblink/expected/dblink.out
@@ -477,3 +477,39 @@ SELECT dblink_disconnect('myconn');
 -- should get 'connection "myconn" not available' error
 SELECT dblink_disconnect('myconn');
 ERROR:  connection "myconn" not available
+-- 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"
+ALTER TABLE test_dropped DROP COLUMN col1;
+ALTER TABLE test_dropped DROP COLUMN col2;
+ALTER TABLE test_dropped ADD COLUMN col3 VARCHAR(10);
+ALTER TABLE test_dropped ADD COLUMN col4 INT;
+INSERT INTO test_dropped VALUES(default,default,'foo','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)
+
diff --git a/contrib/dblink/sql/dblink.sql b/contrib/dblink/sql/dblink.sql
index 15e33144e135e84e48acf9b315ba1e67000a869b..da33e033e461bf5f89ed2d3f69fb992c20525201 100644 (file)
--- a/contrib/dblink/sql/dblink.sql
+++ b/contrib/dblink/sql/dblink.sql
@@ -244,3 +244,28 @@ SELECT dblink_disconnect('myconn');
 -- close the named persistent connection again
 -- should get 'connection "myconn" not available' error
 SELECT dblink_disconnect('myconn');
+
+-- 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
+);
+
+ALTER TABLE test_dropped DROP COLUMN col1;
+ALTER TABLE test_dropped DROP COLUMN col2;
+ALTER TABLE test_dropped ADD COLUMN col3 VARCHAR(10);
+ALTER TABLE test_dropped ADD COLUMN col4 INT;
+
+INSERT INTO test_dropped VALUES(default,default,'foo','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]);
This is the main PostgreSQL git repository.