Skip to content

Commit 873b9b8

Browse files
tglsfdctglsfdc
committed
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.
1 parent 240076b commit 873b9b8

File tree

3 files changed

+97
-38
lines changed

3 files changed

+97
-38
lines changed

‎contrib/dblink/dblink.c‎

Lines changed: 36 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1456,7 +1456,7 @@ get_sql_insert(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
14561456
appendStringInfo(str, ") VALUES(");
14571457

14581458
/*
1459-
* remember attvals are 1 based
1459+
* Note: i is physical column number (counting from 0).
14601460
*/
14611461
needComma = false;
14621462
for (i = 0; i < natts; i++)
@@ -1467,12 +1467,9 @@ get_sql_insert(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
14671467
if (needComma)
14681468
appendStringInfo(str, ",");
14691469

1470-
if (tgt_pkattvals != NULL)
1471-
key = get_attnum_pk_pos(pkattnums, pknumatts, i);
1472-
else
1473-
key = -1;
1470+
key = get_attnum_pk_pos(pkattnums, pknumatts, i);
14741471

1475-
if (key > -1)
1472+
if (key >= 0)
14761473
val = pstrdup(tgt_pkattvals[key]);
14771474
else
14781475
val = SPI_getvalue(tuple, tupdesc, i + 1);
@@ -1503,7 +1500,6 @@ get_sql_delete(Relation rel, int *pkattnums, int pknumatts, char **tgt_pkattvals
15031500
int natts;
15041501
StringInfo str = makeStringInfo();
15051502
char *sql;
1506-
char *val = NULL;
15071503
int i;
15081504

15091505
/* 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
15231519
appendStringInfo(str, "%s",
15241520
quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));
15251521

1526-
if (tgt_pkattvals != NULL)
1527-
val = pstrdup(tgt_pkattvals[i]);
1528-
else
1529-
/* internal error */
1530-
elog(ERROR, "target key array must not be NULL");
1531-
1532-
if (val != NULL)
1533-
{
1534-
appendStringInfo(str, " = %s", quote_literal_cstr(val));
1535-
pfree(val);
1536-
}
1522+
if (tgt_pkattvals[i] != NULL)
1523+
appendStringInfo(str, " = %s",
1524+
quote_literal_cstr(tgt_pkattvals[i]));
15371525
else
15381526
appendStringInfo(str, " IS NULL");
15391527
}
@@ -1585,12 +1573,9 @@ get_sql_update(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
15851573
appendStringInfo(str, "%s = ",
15861574
quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));
15871575

1588-
if (tgt_pkattvals != NULL)
1589-
key = get_attnum_pk_pos(pkattnums, pknumatts, i);
1590-
else
1591-
key = -1;
1576+
key = get_attnum_pk_pos(pkattnums, pknumatts, i);
15921577

1593-
if (key > -1)
1578+
if (key >= 0)
15941579
val = pstrdup(tgt_pkattvals[key]);
15951580
else
15961581
val = SPI_getvalue(tuple, tupdesc, i + 1);
@@ -1617,16 +1602,10 @@ get_sql_update(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
16171602
appendStringInfo(str, "%s",
16181603
quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));
16191604

1620-
if (tgt_pkattvals != NULL)
1621-
val = pstrdup(tgt_pkattvals[i]);
1622-
else
1623-
val = SPI_getvalue(tuple, tupdesc, pkattnum + 1);
1605+
val = tgt_pkattvals[i];
16241606

16251607
if (val != NULL)
1626-
{
16271608
appendStringInfo(str, " = %s", quote_literal_cstr(val));
1628-
pfree(val);
1629-
}
16301609
else
16311610
appendStringInfo(str, " IS NULL");
16321611
}
@@ -1694,30 +1673,49 @@ get_tuple_of_interest(Relation rel, int *pkattnums, int pknumatts, char **src_pk
16941673
{
16951674
char *relname;
16961675
TupleDesc tupdesc;
1676+
int natts;
16971677
StringInfo str = makeStringInfo();
16981678
char *sql = NULL;
16991679
int ret;
17001680
HeapTuple tuple;
17011681
int i;
17021682
char *val = NULL;
17031683

1704-
/* get relation name including any needed schema prefix and quoting */
1705-
relname = generate_relation_name(rel);
1706-
1707-
tupdesc = rel->rd_att;
1708-
17091684
/*
17101685
* Connect to SPI manager
17111686
*/
17121687
if ((ret = SPI_connect()) < 0)
17131688
/* internal error */
17141689
elog(ERROR, "SPI connect failure - returned %d", ret);
17151690

1691+
/* get relation name including any needed schema prefix and quoting */
1692+
relname = generate_relation_name(rel);
1693+
1694+
tupdesc = rel->rd_att;
1695+
natts = tupdesc->natts;
1696+
17161697
/*
1717-
* Build sql statement to look up tuple of interest Use src_pkattvals
1718-
* as the criteria.
1698+
* Build sql statement to look up tuple of interest, ie, the one matching
1699+
* src_pkattvals. We used to use "SELECT *" here, but it's simpler to
1700+
* generate a result tuple that matches the table's physical structure,
1701+
* with NULLs for any dropped columns. Otherwise we have to deal with
1702+
* two different tupdescs and everything's very confusing.
17191703
*/
1720-
appendStringInfo(str, "SELECT * FROM %s WHERE ", relname);
1704+
appendStringInfoString(str, "SELECT ");
1705+
1706+
for (i = 0; i < natts; i++)
1707+
{
1708+
if (i > 0)
1709+
appendStringInfoString(str, ", ");
1710+
1711+
if (tupdesc->attrs[i]->attisdropped)
1712+
appendStringInfoString(str, "NULL");
1713+
else
1714+
appendStringInfoString(str,
1715+
quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));
1716+
}
1717+
1718+
appendStringInfo(str, " FROM %s WHERE ", relname);
17211719

17221720
for (i = 0; i < pknumatts; i++)
17231721
{

‎contrib/dblink/expected/dblink.out‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,3 +477,39 @@ SELECT dblink_disconnect('myconn');
477477
-- should get 'connection "myconn" not available' error
478478
SELECT dblink_disconnect('myconn');
479479
ERROR: connection "myconn" not available
480+
-- test dropped columns in dblink_build_sql_insert, dblink_build_sql_update
481+
CREATE TEMP TABLE test_dropped
482+
(
483+
col1 INT NOT NULL DEFAULT 111,
484+
id SERIAL PRIMARY KEY,
485+
col2 INT NOT NULL DEFAULT 112,
486+
col2b INT NOT NULL DEFAULT 113
487+
);
488+
NOTICE: CREATE TABLE will create implicit sequence "test_dropped_id_seq" for "serial" column "test_dropped.id"
489+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "test_dropped_pkey" for table "test_dropped"
490+
ALTER TABLE test_dropped DROP COLUMN col1;
491+
ALTER TABLE test_dropped DROP COLUMN col2;
492+
ALTER TABLE test_dropped ADD COLUMN col3 VARCHAR(10);
493+
ALTER TABLE test_dropped ADD COLUMN col4 INT;
494+
INSERT INTO test_dropped VALUES(default,default,'foo','42');
495+
SELECT dblink_build_sql_insert('test_dropped', '2', 1,
496+
ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
497+
dblink_build_sql_insert
498+
---------------------------------------------------------------------------
499+
INSERT INTO test_dropped(id,col2b,col3,col4) VALUES('2','113','foo','42')
500+
(1 row)
501+
502+
SELECT dblink_build_sql_update('test_dropped', '2', 1,
503+
ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
504+
dblink_build_sql_update
505+
-------------------------------------------------------------------------------------------
506+
UPDATE test_dropped SET id = '2', col2b = '113', col3 = 'foo', col4 = '42' WHERE id = '2'
507+
(1 row)
508+
509+
SELECT dblink_build_sql_delete('test_dropped', '2', 1,
510+
ARRAY['2'::TEXT]);
511+
dblink_build_sql_delete
512+
-----------------------------------------
513+
DELETE FROM test_dropped WHERE id = '2'
514+
(1 row)
515+

‎contrib/dblink/sql/dblink.sql‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,3 +244,28 @@ SELECT dblink_disconnect('myconn');
244244
-- close the named persistent connection again
245245
-- should get 'connection "myconn" not available' error
246246
SELECT dblink_disconnect('myconn');
247+
248+
-- test dropped columns in dblink_build_sql_insert, dblink_build_sql_update
249+
CREATE TEMP TABLE test_dropped
250+
(
251+
col1 INT NOT NULL DEFAULT 111,
252+
id SERIAL PRIMARY KEY,
253+
col2 INT NOT NULL DEFAULT 112,
254+
col2b INT NOT NULL DEFAULT 113
255+
);
256+
257+
ALTER TABLE test_dropped DROP COLUMN col1;
258+
ALTER TABLE test_dropped DROP COLUMN col2;
259+
ALTER TABLE test_dropped ADD COLUMN col3 VARCHAR(10);
260+
ALTER TABLE test_dropped ADD COLUMN col4 INT;
261+
262+
INSERT INTO test_dropped VALUES(default,default,'foo','42');
263+
264+
SELECT dblink_build_sql_insert('test_dropped', '2', 1,
265+
ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
266+
267+
SELECT dblink_build_sql_update('test_dropped', '2', 1,
268+
ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
269+
270+
SELECT dblink_build_sql_delete('test_dropped', '2', 1,
271+
ARRAY['2'::TEXT]);

0 commit comments

Comments
 (0)