Thread: contrib/dblink regression test failure fix
Attached is a patch to fix the current regression test failure for contrib/dblink. The failure was induced by recent changes to cast behavior. If possible, please apply before creating the beta2 tarball. Thanks, Joe Index: contrib/dblink/expected/dblink.out =================================================================== RCS file: /opt/src/cvs/pgsql-server/contrib/dblink/expected/dblink.out,v retrieving revision 1.2 diff -c -r1.2 dblink.out *** contrib/dblink/expected/dblink.out 14 Sep 2002 22:00:59 -0000 1.2 --- contrib/dblink/expected/dblink.out 23 Sep 2002 16:39:40 -0000 *************** *** 56,62 **** -- 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"}'); dblink_build_sql_insert ----------------------------------------------------------- INSERT INTO foo(f1,f2,f3) VALUES('99','xyz','{a0,b0,c0}') --- 56,62 ---- -- 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::int2,'{"0", "a"}','{"99", "xyz"}'); dblink_build_sql_insert ----------------------------------------------------------- INSERT INTO foo(f1,f2,f3) VALUES('99','xyz','{a0,b0,c0}') *************** *** 64,77 **** -- 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"}'); dblink_build_sql_update ---------------------------------------------------------------------------------------- UPDATE foo SET f1 = '99', f2 = 'xyz', f3 = '{a0,b0,c0}' WHERE f1 = '99' AND f2 = 'xyz' (1 row) -- build a delete statement based on a local tuple, ! select dblink_build_sql_delete('foo','1 2',2,'{"0", "a"}'); dblink_build_sql_delete --------------------------------------------- DELETE FROM foo WHERE f1 = '0' AND f2 = 'a' --- 64,77 ---- -- 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::int2,'{"0", "a"}','{"99", "xyz"}'); dblink_build_sql_update ---------------------------------------------------------------------------------------- UPDATE foo SET f1 = '99', f2 = 'xyz', f3 = '{a0,b0,c0}' WHERE f1 = '99' AND f2 = 'xyz' (1 row) -- build a delete statement based on a local tuple, ! select dblink_build_sql_delete('foo','1 2',2::int2,'{"0", "a"}'); dblink_build_sql_delete --------------------------------------------- DELETE FROM foo WHERE f1 = '0' AND f2 = 'a' Index: contrib/dblink/sql/dblink.sql =================================================================== RCS file: /opt/src/cvs/pgsql-server/contrib/dblink/sql/dblink.sql,v retrieving revision 1.2 diff -c -r1.2 dblink.sql *** contrib/dblink/sql/dblink.sql 14 Sep 2002 22:00:59 -0000 1.2 --- contrib/dblink/sql/dblink.sql 23 Sep 2002 16:39:09 -0000 *************** *** 48,61 **** -- 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"}'); -- 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"}'); -- build a delete statement based on a local tuple, ! select dblink_build_sql_delete('foo','1 2',2,'{"0", "a"}'); -- -- Connect back to the regression database and define the functions. --- 48,61 ---- -- 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::int2,'{"0", "a"}','{"99", "xyz"}'); -- 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::int2,'{"0", "a"}','{"99", "xyz"}'); -- build a delete statement based on a local tuple, ! select dblink_build_sql_delete('foo','1 2',2::int2,'{"0", "a"}'); -- -- Connect back to the regression database and define the functions.
Joe Conway <mail@joeconway.com> writes: > Attached is a patch to fix the current regression test failure for > contrib/dblink. The failure was induced by recent changes to cast > behavior. Actually, I was going to suggest modifying the declaration of the dblink function(s) to take integer instead of int2. Is there a really good reason why they take int2? If not, there's little point in putting up with a notational headache throughout the 7.3 cycle. The problem should go away again in 7.4, when we tweak the parser to initially type "2" as "2::int2" ... but for now the path of least resistance would seem to be avoiding declaring functions to take int2. regards, tom lane
Tom Lane wrote: > Actually, I was going to suggest modifying the declaration of the > dblink function(s) to take integer instead of int2. Is there a > really good reason why they take int2? If not, there's little point > in putting up with a notational headache throughout the 7.3 cycle. > > The problem should go away again in 7.4, when we tweak the parser to > initially type "2" as "2::int2" ... but for now the path of least > resistance would seem to be avoiding declaring functions to take int2. Good point. I only used int2 to be consistent with pg_class.relnatts. Here is a revised patch. This uses an int4 input with a bit of checking to be sure the given value fits in an int2 variable. This seemed the easiest and safest approach. Please apply if there are no objections. Thanks, Joe Index: contrib/dblink/dblink.c =================================================================== RCS file: /opt/src/cvs/pgsql-server/contrib/dblink/dblink.c,v retrieving revision 1.14 diff -c -r1.14 dblink.c *** contrib/dblink/dblink.c 5 Sep 2002 00:56:35 -0000 1.14 --- contrib/dblink/dblink.c 23 Sep 2002 17:31:38 -0000 *************** *** 1002,1007 **** --- 1002,1010 ---- } + #ifndef SHRT_MAX + #define SHRT_MAX (0x7FFF) + #endif /* * dblink_build_sql_insert * *************** *** 1028,1034 **** Oid relid; text *relname_text; int16 *pkattnums; ! int16 pknumatts; char **src_pkattvals; char **tgt_pkattvals; ArrayType *src_pkattvals_arry; --- 1031,1038 ---- Oid relid; text *relname_text; int16 *pkattnums; ! int pknumatts_tmp; ! int16 pknumatts = 0; char **src_pkattvals; char **tgt_pkattvals; ArrayType *src_pkattvals_arry; *************** *** 1057,1063 **** elog(ERROR, "dblink_build_sql_insert: relation does not exist"); pkattnums = (int16 *) PG_GETARG_POINTER(1); ! pknumatts = PG_GETARG_INT16(2); /* * There should be at least one key attribute --- 1061,1071 ---- elog(ERROR, "dblink_build_sql_insert: relation does not exist"); pkattnums = (int16 *) PG_GETARG_POINTER(1); ! pknumatts_tmp = PG_GETARG_INT32(2); ! if (pknumatts_tmp <= SHRT_MAX) ! pknumatts = pknumatts_tmp; ! else ! elog(ERROR, "Bad input value for pknumatts; too large"); /* * There should be at least one key attribute *************** *** 1167,1173 **** Oid relid; text *relname_text; int16 *pkattnums; ! int16 pknumatts; char **tgt_pkattvals; ArrayType *tgt_pkattvals_arry; int tgt_ndim; --- 1175,1182 ---- Oid relid; text *relname_text; int16 *pkattnums; ! int pknumatts_tmp; ! int16 pknumatts = 0; char **tgt_pkattvals; ArrayType *tgt_pkattvals_arry; int tgt_ndim; *************** *** 1191,1197 **** elog(ERROR, "dblink_build_sql_delete: relation does not exist"); pkattnums = (int16 *) PG_GETARG_POINTER(1); ! pknumatts = PG_GETARG_INT16(2); /* * There should be at least one key attribute --- 1200,1210 ---- elog(ERROR, "dblink_build_sql_delete: relation does not exist"); pkattnums = (int16 *) PG_GETARG_POINTER(1); ! pknumatts_tmp = PG_GETARG_INT32(2); ! if (pknumatts_tmp <= SHRT_MAX) ! pknumatts = pknumatts_tmp; ! else ! elog(ERROR, "Bad input value for pknumatts; too large"); /* * There should be at least one key attribute *************** *** 1274,1280 **** Oid relid; text *relname_text; int16 *pkattnums; ! int16 pknumatts; char **src_pkattvals; char **tgt_pkattvals; ArrayType *src_pkattvals_arry; --- 1287,1294 ---- Oid relid; text *relname_text; int16 *pkattnums; ! int pknumatts_tmp; ! int16 pknumatts = 0; char **src_pkattvals; char **tgt_pkattvals; ArrayType *src_pkattvals_arry; *************** *** 1303,1309 **** elog(ERROR, "dblink_build_sql_update: relation does not exist"); pkattnums = (int16 *) PG_GETARG_POINTER(1); ! pknumatts = PG_GETARG_INT16(2); /* * There should be one source array key values for each key attnum --- 1317,1327 ---- elog(ERROR, "dblink_build_sql_update: relation does not exist"); pkattnums = (int16 *) PG_GETARG_POINTER(1); ! pknumatts_tmp = PG_GETARG_INT32(2); ! if (pknumatts_tmp <= SHRT_MAX) ! pknumatts = pknumatts_tmp; ! else ! elog(ERROR, "Bad input value for pknumatts; too large"); /* * There should be one source array key values for each key attnum Index: contrib/dblink/dblink.sql.in =================================================================== RCS file: /opt/src/cvs/pgsql-server/contrib/dblink/dblink.sql.in,v retrieving revision 1.5 diff -c -r1.5 dblink.sql.in *** contrib/dblink/dblink.sql.in 14 Sep 2002 20:28:54 -0000 1.5 --- contrib/dblink/dblink.sql.in 23 Sep 2002 17:32:53 -0000 *************** *** 57,71 **** AS 'MODULE_PATHNAME','dblink_get_pkey' LANGUAGE 'c' WITH (isstrict); ! CREATE OR REPLACE FUNCTION dblink_build_sql_insert (text, int2vector, int2, _text, _text) RETURNS text AS 'MODULE_PATHNAME','dblink_build_sql_insert' LANGUAGE 'c' WITH (isstrict); ! CREATE OR REPLACE FUNCTION dblink_build_sql_delete (text, int2vector, int2, _text) RETURNS text AS 'MODULE_PATHNAME','dblink_build_sql_delete' LANGUAGE 'c' WITH (isstrict); ! CREATE OR REPLACE FUNCTION dblink_build_sql_update (text, int2vector, int2, _text, _text) RETURNS text AS 'MODULE_PATHNAME','dblink_build_sql_update' LANGUAGE 'c' WITH (isstrict); --- 57,71 ---- AS 'MODULE_PATHNAME','dblink_get_pkey' LANGUAGE 'c' WITH (isstrict); ! CREATE OR REPLACE FUNCTION dblink_build_sql_insert (text, int2vector, int4, _text, _text) RETURNS text AS 'MODULE_PATHNAME','dblink_build_sql_insert' LANGUAGE 'c' WITH (isstrict); ! CREATE OR REPLACE FUNCTION dblink_build_sql_delete (text, int2vector, int4, _text) RETURNS text AS 'MODULE_PATHNAME','dblink_build_sql_delete' LANGUAGE 'c' WITH (isstrict); ! CREATE OR REPLACE FUNCTION dblink_build_sql_update (text, int2vector, int4, _text, _text) RETURNS text AS 'MODULE_PATHNAME','dblink_build_sql_update' LANGUAGE 'c' WITH (isstrict); *************** *** 82,89 **** GRANT EXECUTE ON FUNCTION dblink_exec (text,text) TO PUBLIC; GRANT EXECUTE ON FUNCTION dblink_exec (text) TO PUBLIC; GRANT EXECUTE ON FUNCTION dblink_get_pkey (text) TO PUBLIC; ! GRANT EXECUTE ON FUNCTION dblink_build_sql_insert (text, int2vector, int2, _text, _text) TO PUBLIC; ! GRANT EXECUTE ON FUNCTION dblink_build_sql_delete (text, int2vector, int2, _text) TO PUBLIC; ! GRANT EXECUTE ON FUNCTION dblink_build_sql_update (text, int2vector, int2, _text, _text) TO PUBLIC; GRANT EXECUTE ON FUNCTION dblink_current_query () TO PUBLIC; --- 82,89 ---- GRANT EXECUTE ON FUNCTION dblink_exec (text,text) TO PUBLIC; GRANT EXECUTE ON FUNCTION dblink_exec (text) TO PUBLIC; GRANT EXECUTE ON FUNCTION dblink_get_pkey (text) TO PUBLIC; ! GRANT EXECUTE ON FUNCTION dblink_build_sql_insert (text, int2vector, int4, _text, _text) TO PUBLIC; ! GRANT EXECUTE ON FUNCTION dblink_build_sql_delete (text, int2vector, int4, _text) TO PUBLIC; ! GRANT EXECUTE ON FUNCTION dblink_build_sql_update (text, int2vector, int4, _text, _text) TO PUBLIC; GRANT EXECUTE ON FUNCTION dblink_current_query () TO PUBLIC;
Joe Conway <mail@joeconway.com> writes: > Here is a revised patch. This uses an int4 input with a bit of checking to be > sure the given value fits in an int2 variable. This seemed the easiest and > safest approach. Please apply if there are no objections. Okay, patch applied --- I figured we'd better fast-track this into beta2, since otherwise people might have a problem upgrading later. regards, tom lane