On 02/03/2010 10:18 AM, Tom Lane wrote: > Joe Conway <m...@joeconway.com> writes: >> The problem exists with all three dblink_build_sql_* functions. Here is >> a more complete patch. If there are no objections I'll apply this to >> HEAD and look at back-patching -- these functions have hardly been >> touched since inception. > > Do you really need to copy the relation tupdesc when you only are going > to make a one-time check of the number of attributes? This coding would > make some sense if you intended to use the tupdesc again later in the > functions, but it appears you don't. > > Also, what about cases where the relation contains dropped columns --- > it's not obvious whether this test is correct in that case.
Good input, as always. Here's another whack at it. Thanks, Joe
Index: dblink.c
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.87
diff -c -r1.87 dblink.c
*** dblink.c 24 Jan 2010 22:19:38 -0000 1.87
--- dblink.c 3 Feb 2010 19:23:51 -0000
***************
*** 101,106 ****
--- 101,107 ----
static void dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail);
static char *get_connect_string(const char *servername);
static char *escape_param_str(const char *from);
+ static int get_nondropped_natts(Oid relid);
/* Global */
static remoteConn *pconn = NULL;
***************
*** 1262,1267 ****
--- 1263,1269 ----
int src_nitems;
int tgt_nitems;
char *sql;
+ int nondropped_natts;
/*
* Convert relname to rel OID.
***************
*** 1290,1295 ****
--- 1292,1306 ----
"attributes too large")));
/*
+ * ensure we don't ask for more pk attributes than we have
+ * non-dropped columns
+ */
+ nondropped_natts = get_nondropped_natts(relid);
+ if (pknumatts > nondropped_natts)
+ ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("number of primary key fields exceeds number of specified relation attributes")));
+
+ /*
* Source array is made up of key values that will be used to locate the
* tuple of interest from the local system.
*/
***************
*** 1354,1359 ****
--- 1365,1371 ----
int2vector *pkattnums = (int2vector *) PG_GETARG_POINTER(1);
int32 pknumatts_tmp = PG_GETARG_INT32(2);
ArrayType *tgt_pkattvals_arry = PG_GETARG_ARRAYTYPE_P(3);
+ int nondropped_natts;
Oid relid;
int16 pknumatts = 0;
char **tgt_pkattvals;
***************
*** 1387,1392 ****
--- 1399,1413 ----
"attributes too large")));
/*
+ * ensure we don't ask for more pk attributes than we have
+ * non-dropped columns
+ */
+ nondropped_natts = get_nondropped_natts(relid);
+ if (pknumatts > nondropped_natts)
+ ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("number of primary key fields exceeds number of specified relation attributes")));
+
+ /*
* Target array is made up of key values that will be used to build the
* SQL string for use on the remote system.
*/
***************
*** 1441,1446 ****
--- 1462,1468 ----
int32 pknumatts_tmp = PG_GETARG_INT32(2);
ArrayType *src_pkattvals_arry = PG_GETARG_ARRAYTYPE_P(3);
ArrayType *tgt_pkattvals_arry = PG_GETARG_ARRAYTYPE_P(4);
+ int nondropped_natts;
Oid relid;
int16 pknumatts = 0;
char **src_pkattvals;
***************
*** 1476,1481 ****
--- 1498,1512 ----
"attributes too large")));
/*
+ * ensure we don't ask for more pk attributes than we have
+ * non-dropped columns
+ */
+ nondropped_natts = get_nondropped_natts(relid);
+ if (pknumatts > nondropped_natts)
+ ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("number of primary key fields exceeds number of specified relation attributes")));
+
+ /*
* Source array is made up of key values that will be used to locate the
* tuple of interest from the local system.
*/
***************
*** 2442,2444 ****
--- 2473,2500 ----
return buf->data;
}
+
+ static int
+ get_nondropped_natts(Oid relid)
+ {
+ int nondropped_natts = 0;
+ TupleDesc tupdesc;
+ Relation rel;
+ int natts;
+ int i;
+
+ rel = relation_open(relid, AccessShareLock);
+ tupdesc = rel->rd_att;
+ natts = tupdesc->natts;
+
+ for (i = 0; i < natts; i++)
+ {
+ if (tupdesc->attrs[i]->attisdropped)
+ continue;
+ nondropped_natts++;
+ }
+
+ relation_close(rel, AccessShareLock);
+ return nondropped_natts;
+ }
+
Index: expected/dblink.out
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/expected/dblink.out,v
retrieving revision 1.27
diff -c -r1.27 dblink.out
*** expected/dblink.out 22 Nov 2009 05:20:38 -0000 1.27
--- expected/dblink.out 3 Feb 2010 18:01:25 -0000
***************
*** 39,44 ****
--- 39,47 ----
INSERT INTO foo(f1,f2,f3) VALUES('99','xyz','{a0,b0,c0}')
(1 row)
+ -- too many pk fields, should fail
+ SELECT dblink_build_sql_insert('foo','1 2 3 4',4,'{"0", "a", "{a0,b0,c0}"}','{"99", "xyz", "{za0,zb0,zc0}"}');
+ ERROR: number of primary key fields exceeds number of specified relation attributes
-- build an update statement based on a local tuple,
-- replacing the primary key values with new ones
SELECT dblink_build_sql_update('foo','1 2',2,'{"0", "a"}','{"99", "xyz"}');
***************
*** 47,52 ****
--- 50,58 ----
UPDATE foo SET f1 = '99', f2 = 'xyz', f3 = '{a0,b0,c0}' WHERE f1 = '99' AND f2 = 'xyz'
(1 row)
+ -- too many pk fields, should fail
+ SELECT dblink_build_sql_update('foo','1 2 3 4',4,'{"0", "a", "{a0,b0,c0}"}','{"99", "xyz", "{za0,zb0,zc0}"}');
+ ERROR: number of primary key fields exceeds number of specified relation attributes
-- build a delete statement based on a local tuple,
SELECT dblink_build_sql_delete('foo','1 2',2,'{"0", "a"}');
dblink_build_sql_delete
***************
*** 54,59 ****
--- 60,68 ----
DELETE FROM foo WHERE f1 = '0' AND f2 = 'a'
(1 row)
+ -- too many pk fields, should fail
+ SELECT dblink_build_sql_delete('foo','1 2 3 4',4,'{"0", "a", "{a0,b0,c0}"}');
+ ERROR: number of primary key fields exceeds number of specified relation attributes
-- retest using a quoted and schema qualified table
CREATE SCHEMA "MySchema";
CREATE TABLE "MySchema"."Foo"(f1 int, f2 text, f3 text[], primary key (f1,f2));
Index: sql/dblink.sql
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/sql/dblink.sql,v
retrieving revision 1.22
diff -c -r1.22 dblink.sql
*** sql/dblink.sql 5 Aug 2009 16:11:07 -0000 1.22
--- sql/dblink.sql 3 Feb 2010 17:59:11 -0000
***************
*** 34,46 ****
--- 34,52 ----
-- build an insert statement based on a local tuple,
-- replacing the primary key values with new ones
SELECT dblink_build_sql_insert('foo','1 2',2,'{"0", "a"}','{"99", "xyz"}');
+ -- too many pk fields, should fail
+ SELECT dblink_build_sql_insert('foo','1 2 3 4',4,'{"0", "a", "{a0,b0,c0}"}','{"99", "xyz", "{za0,zb0,zc0}"}');
-- build an update statement based on a local tuple,
-- replacing the primary key values with new ones
SELECT dblink_build_sql_update('foo','1 2',2,'{"0", "a"}','{"99", "xyz"}');
+ -- too many pk fields, should fail
+ SELECT dblink_build_sql_update('foo','1 2 3 4',4,'{"0", "a", "{a0,b0,c0}"}','{"99", "xyz", "{za0,zb0,zc0}"}');
-- build a delete statement based on a local tuple,
SELECT dblink_build_sql_delete('foo','1 2',2,'{"0", "a"}');
+ -- too many pk fields, should fail
+ SELECT dblink_build_sql_delete('foo','1 2 3 4',4,'{"0", "a", "{a0,b0,c0}"}');
-- retest using a quoted and schema qualified table
CREATE SCHEMA "MySchema";
signature.asc
Description: OpenPGP digital signature
