8000 Fix dblink_build_sql_insert() and related functions to handle dropped · danielcode/postgres@221f4de · GitHub
[go: up one dir, main page]
Skip to content

Commit 221f4de

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 943676c commit 221f4de

File tree

3 files changed

+99
-38
lines changed

3 files changed

+99
-38
lines changed

‎contrib/dblink/dblink.c

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

16091609
/*
1610-
* remember attvals are 1 based
1610+
* Note: i is physical column number (counting from 0).
16111611
*/
16121612
needComma = false;
16131613
for (i = 0; i < natts; i++)
@@ -1618,12 +1618,9 @@ get_sql_insert(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
16181618
if (needComma)
16191619
appendStringInfo(str, ",");
16201620

1621-
if (tgt_pkattvals != NULL)
1622-
key = get_attnum_pk_pos(pkattnums, pknumatts, i);
1623-
else
1624-
key = -1;
1621+
key = get_attnum_pk_pos(pkattnums, pknumatts, i);
16251622

1626-
if (key > -1)
1623+
if (key >= 0)
16271624
val = pstrdup(tgt_pkattvals[key]);
16281625
else
16291626
val = SPI_getvalue(tuple, tupdesc, i + 1);
@@ -1654,7 +1651,6 @@ get_sql_delete(Relation rel, int *pkattnums, int pknumatts, char **tgt_pkattvals
16541651
int natts;
16551652
StringInfo str = makeStringInfo();
16561653
char *sql;
1657-
char *val = NULL;
16581654
int i;
16591655

16601656
/* get relation name including any needed schema prefix and quoting */
@@ -1674,17 +1670,9 @@ get_sql_delete(Relation rel, int *pkattnums, int pknumatts, char **tgt_pkattvals
16741670
appendStringInfo(str, "%s",
16751671
quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));
16761672

1677-
if (tgt_pkattvals != NULL)
1678-
val = pstrdup(tgt_pkattvals[i]);
1679-
else
1680-
/* internal error */
1681-
elog(ERROR, "target key array must not be NULL");
1682-
1683-
if (val != NULL)
1684-
{
1685-
appendStringInfo(str, " = %s", quote_literal_cstr(val));
1686-
pfree(val);
1687-
}
1673+
if (tgt_pkattvals[i] != NULL)
1674+
appendStringInfo(str, " = %s",
1675+
quote_literal_cstr(tgt_pkattvals[i]));
16881676
else
16891677
appendStringInfo(str, " IS NULL");
16901678
}
@@ -1736,12 +1724,9 @@ get_sql_update(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
17361724
appendStringInfo(str, "%s = ",
17371725
quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));
17381726

1739-
if (tgt_pkattvals != NULL)
1740-
key = get_attnum_pk_pos(pkattnums, pknumatts, i);
1741-
else
1742-
key = -1;
1727+
key = get_attnum_pk_pos(pkattnums, pknumatts, i);
17431728

1744-
if (key > -1)
1729+
if (key >= 0)
17451730
val = pstrdup(tgt_pkattvals[key]);
17461731
else
17471732
val = SPI_getvalue(tuple, tupdesc, i + 1);
@@ -1768,16 +1753,10 @@ get_sql_update(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals
17681753
appendStringInfo(str, "%s",
17691754
quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));
17701755

1771-
if (tgt_pkattvals != NULL)
1772-
val = pstrdup(tgt_pkattvals[i]);
1773-
else
1774-
val = SPI_getvalue(tuple, tupdesc, pkattnum + 1);
1756+
val = tgt_pkattvals[i];
17751757

17761758
if (val != NULL)
1777-
{
17781759
appendStringInfo(str, " = %s", quote_literal_cstr(val));
1779-
pfree(val);
1780-
}
17811760
else
17821761
appendStringInfo(str, " IS NULL");
17831762
}
@@ -1845,30 +1824,49 @@ get_tuple_of_interest(Relation rel, int *pkattnums, int pknumatts, char **src_pk
18451824
{
18461825
char *relname;
18471826
TupleDesc tupdesc;
1827+
int natts;
18481828
StringInfo str = makeStringInfo();
18491829
char *sql = NULL;
18501830
int ret;
18511831
HeapTuple tuple;
18521832
int i;
18531833
char *val = NULL;
18541834

1855-
/* get relation name including any needed schema prefix and quoting */
1856-
relname = generate_relation_name(rel);
1857-
1858-
tupdesc = rel->rd_att;
1859-
18601835
/*
18611836
* Connect to SPI manager
18621837
*/
18631838
if ((ret = SPI_connect()) < 0)
18641839
/* internal error */
18651840
elog(ERROR, "SPI connect failure - returned %d", ret);
18661841

1842+
/* get relation name including any needed schema prefix and quoting */
1843+
relname = generate_relation_name( F438 rel);
1844+
1845+
tupdesc = rel->rd_att;
1846+
natts = tupdesc->natts;
1847+
18671848
/*
1868-
* Build sql statement to look up tuple of interest Use src_pkattvals
1869-
* as the criteria.
1849+
* Build sql statement to look up tuple of interest, ie, the one matching
1850+
* src_pkattvals. We used to use "SELECT *" here, but it's simpler to
1851+
* generate a result tuple that matches the table's physical structure,
1852+
* with NULLs for any dropped columns. Otherwise we have to deal with
1853+
* two different tupdescs and everything's very confusing.
18701854
*/
1871-
appendStringInfo(str, "SELECT * FROM %s WHERE ", relname);
1855+
appendStringInfoString(str, "SELECT ");
1856+
1857+
for (i = 0; i < natts; i++)
1858+
{
1859+
if (i > 0)
1860+
appendStringInfoString(str, ", ");
1861+
1862+
if (tupdesc->attrs[i]->attisdropped)
1863+
appendStringInfoString(str, "NULL");
1864+
else
1865+
appendStringInfoString(str,
1866+
quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));
1867+
}
1868+
1869+
appendStringInfo(str, " FROM %s WHERE ", relname);
18721870

18731871
for (i = 0; i < pknumatts; i++)
18741872
{

‎contrib/dblink/expected/dblink.out

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,3 +624,40 @@ SELECT dblink_disconnect('myconn');
624624
-- should get 'connection "myconn" not available' error
625625
SELECT dblink_disconnect('myconn');
626626
ERROR: connection "myconn" not available
627+
-- test dropped columns in dblink_build_sql_insert, dblink_build_sql_update
628+
CREATE TEMP TABLE test_dropped
629+
(
630+
col1 INT NOT NULL DEFAULT 111,
631+
id SERIAL PRIMARY KEY,
632+
col2 INT NOT NULL DEFAULT 112,
633+
col2b INT NOT NULL DEFAULT 113
634+
);
635+
NOTICE: CREATE TABLE will create implicit sequence "test_dropped_id_seq" for serial column "test_dropped.id"
636+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "test_dropped_pkey" for table "test_dropped"
637+
INSERT INTO test_dropped VALUES(default);
638+
ALTER TABLE test_dropped
639+
DROP COLUMN col1,
640+
DROP COLUMN col2,
641+
ADD COLUMN col3 VARCHAR(10) NOT NULL DEFAULT 'foo',
642+
ADD COLUMN col4 INT NOT NULL DEFAULT 42;
643+
SELECT dblink_build_sql_insert('test_dropped', '2', 1,
644+
ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
645+
dblink_build_sql_insert
646+
---------------------------------------------------------------------------
647+
INSERT INTO test_dropped(id,col2b,col3,col4) VALUES('2','113','foo','42')
648+
(1 row)
649+
650+
SELECT dblink_build_sql_update('test_dropped', '2', 1,
651+
ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
652+
dblink_build_sql_update
653+
-------------------------------------------------------------------------------------------
654+
UPDATE test_dropped SET id = '2', col2b = '113', col3 = 'foo', col4 = '42' WHERE id = '2'
655+
(1 row)
656+
657+
SELECT dblink_build_sql_delete('test_dropped', '2', 1,
658+
ARRAY['2'::TEXT]);
659+
dblink_build_sql_delete
660+
-----------------------------------------
661+
DELETE FROM test_dropped WHERE id = '2'
662+
(1 row)
663+

‎contrib/dblink/sql/dblink.sql

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,3 +301,29 @@ SELECT dblink_disconnect('myconn');
301301
-- close the named persistent connection again
302302
-- should get 'connection "myconn" not available' error
303303
SELECT dblink_disconnect('myconn');
304+
305+
-- test dropped columns in dblink_build_sql_insert, dblink_build_sql_update
306+
CREATE TEMP TABLE test_dropped
307+
(
308+
col1 INT NOT NULL DEFAULT 111,
309+
id SERIAL PRIMARY KEY,
310+
col2 INT NOT NULL DEFAULT 112,
311+
col2b INT NOT NULL DEFAULT 113
312+
);
313+
314+
INSERT INTO test_dropped VALUES(default);
315+
316+
ALTER TABLE test_dropped
317+
DROP COLUMN col1,
318+
DROP COLUMN col2,
319+
ADD COLUMN col3 VARCHAR(10) NOT NULL DEFAULT 'foo',
320+
ADD COLUMN col4 INT NOT NULL DEFAULT 42;
321+
322+
SELECT dblink_build_sql_insert('test_dropped', '2', 1,
323+
ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
324+
325+
SELECT dblink_build_sql_update('test_dropped', '2', 1,
326+
ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
327+
328+
SELECT dblink_build_sql_delete('test_dropped', '2', 1,
329+
ARRAY['2'::TEXT]);

0 commit comments

Comments
 (0)
0