Re: Implicit coercions need to be reined in

Tom,

My feeling is that this change as currently scoped will break a lot of
existing apps. Especially the case where people are using where clauses
of the form: bigintcolumn = '999' to get a query to use the index on
a column of type bigint.

thanks,
--Barry

Tom Lane wrote:
> Awhile back I suggested adding a boolean column to pg_proc to control
> which type coercion functions could be invoked implicitly, and which
> would need an explicit cast:
> http://archives.postgresql.org/pgsql-hackers/2001-11/msg00803.php
> There is a relevant bug report #484 showing the dangers of too many
> implicit coercion paths:
> http://archives.postgresql.org/pgsql-bugs/2001-10/msg00108.php
>
> I have added such a column as part of the pg_proc changes I'm currently
> doing to migrate aggregates into pg_proc. So it's now time to debate
> the nitty-gritty: exactly which coercion functions should not be
> implicitly invokable anymore?
>
> My first-cut attempt at this is shown by the two printouts below.
> The first cut does not allow any implicit coercions to text from types
> that are not in the text category, which seems a necessary rule to me
> --- the above-cited bug report shows why free coercions to text are
> dangerous. However, it turns out that several of the regression
> tests fail with this rule; see the regression diffs below.
>
> Should I consider these regression tests wrong, and correct them?
> If not, how can we limit implicit coercions to text enough to avoid
> the problems illustrated by bug #484?
>
> Another interesting point is that I allowed implicit coercions from
> float8 to numeric; this is necessary to avoid breaking cases like
> insert into foo(numeric_col) values(12.34);
> since the constant will be initially typed as float8. However, because
> I didn't allow the reverse coercion implicitly, this makes numeric
> "more preferred" than float8. Thus, for example,
> select '12.34'::numeric + 12.34;
> which draws a can't-resolve-operator error in 7.2, is resolved as
> numeric addition with these changes. Is this a good thing, or not?
> We could preserve the can't-resolve behavior by marking numeric->float8
> as an allowed implicit coercion, but that seems ugly. I'm not sure we
> can do a whole lot better without some more wide-ranging revisions of
> the way we handle untyped numeric literals (as in past proposals to
> invent an UNKNOWNNUMERIC pseudo-type).
>
> Also, does anyone have any other nits to pick with this classification
> of which coercions are implicitly okay? I've started with a fairly
> tough approach of disallowing most implicit coercions, but perhaps this
> goes too far.
>
> regards, tom lane
>
> Coercions allowed implicitly:
>
> oid | result | input | prosrc
> ------+-------------+-------------+-----------------------
> 860 | bpchar | char | char_bpchar
> 408 | bpchar | name | name_bpchar
> 861 | char | bpchar | bpchar_char
> 944 | char | text | text_char
> 312 | float4 | float8 | dtof
> 236 | float4 | int2 | i2tof
> 318 | float4 | int4 | i4tof
> 311 | float8 | float4 | ftod
> 235 | float8 | int2 | i2tod
> 316 | float8 | int4 | i4tod
> 482 | float8 | int8 | i8tod
> 314 | int2 | int4 | i4toi2
> 714 | int2 | int8 | int82
> 313 | int4 | int2 | i2toi4
> 480 | int4 | int8 | int84
> 754 | int8 | int2 | int28
> 481 | int8 | int4 | int48
> 1177 | interval | reltime | reltime_interval
> 1370 | interval | time | time_interval
> 409 | name | bpchar | bpchar_name
> 407 | name | text | text_name
> 1400 | name | varchar | text_name
> 1742 | numeric | float4 | float4_numeric
> 1743 | numeric | float8 | float8_numeric
> 1782 | numeric | int2 | int2_numeric
> 1740 | numeric | int4 | int4_numeric
> 1781 | numeric | int8 | int8_numeric
> 946 | text | char | char_text
> 406 | text | name | name_text
> 2046 | time | timetz | timetz_time
> 2023 | timestamp | abstime | abstime_timestamp
> 2024 | timestamp | date | date_timestamp
> 2027 | timestamp | timestamptz | timestamptz_timestamp
> 1173 | timestamptz | abstime | abstime_timestamptz
> 1174 | timestamptz | date | date_timestamptz
> 2028 | timestamptz | timestamp | timestamp_timestamptz
> 2047 | timetz | time | time_timetz
> 1401 | varchar | name | name_text
> (38 rows)
>
> Coercions that will require explicit CAST, ::type, or typename(x) syntax
> (NB: in 7.2 all of these would have been allowed implicitly):
>
> oid | result | input | prosrc
> ------+-------------+-------------+------------------------------------------
> 2030 | abstime | timestamp | timestamp_abstime
> 1180 | abstime | timestamptz | timestamptz_abstime
> 1480 | box | circle | circle_box
> 1446 | box | polygon | poly_box
> 1714 | cidr | text | text_cidr
> 1479 | circle | box | box_circle
> 1474 | circle | polygon | poly_circle
> 1179 | date | abstime | abstime_date
> 748 | date | text | text_date
> 2029 | date | timestamp | timestamp_date
> 1178 | date | timestamptz | timestamptz_date
> 1745 | float4 | numeric | numeric_float4
> 839 | float4 | text | text_float4
> 1746 | float8 | numeric | numeric_float8
> 838 | float8 | text | text_float8
> 1713 | inet | text | text_inet
> 238 | int2 | float4 | ftoi2
> 237 | int2 | float8 | dtoi2
> 1783 | int2 | numeric | numeric_int2
> 818 | int2 | text | text_int2
> 319 | int4 | float4 | ftoi4
> 317 | int4 | float8 | dtoi4
> 1744 | int4 | numeric | numeric_int4
> 819 | int4 | text | text_int4
> 483 | int8 | float8 | dtoi8
> 1779 | int8 | numeric | numeric_int8
> 1289 | int8 | text | text_int8
> 1263 | interval | text | text_interval
> 1541 | lseg | box | box_diagonal
> 767 | macaddr | text | text_macaddr
> 817 | oid | text | text_oid
> 1447 | path | polygon | poly_path
> 1534 | point | box | box_center
> 1416 | point | circle | circle_center
> 1532 | point | lseg | lseg_center
> 1533 | point | path | path_center
> 1540 | point | polygon | poly_center
> 1448 | polygon | box | box_poly
> 1544 | polygon | circle | select polygon(12, $1)
> 1449 | polygon | path | path_poly
> 1200 | reltime | int4 | int4reltime
> 1194 | reltime | interval | interval_reltime
> 749 | text | date | date_text
> 841 | text | float4 | float4_text
> 840 | text | float8 | float8_text
> 730 | text | inet | network_show
> 113 | text | int2 | int2_text
> 112 | text | int4 | int4_text
> 1288 | text | int8 | int8_text
> 1193 | text | interval | interval_text
> 752 | text | macaddr | macaddr_text
> 114 | text | oid | oid_text
> 948 | text | time | time_text
> 2034 | text | timestamp | timestamp_text
> 1192 | text | timestamptz | timestamptz_text
> 939 | text | timetz | timetz_text
> 1364 | time | abstime | select time(cast($1 as timestamp without time zone))
> 1419 | time | interval | interval_time
> 837 | time | text | text_time
> 1316 | time | timestamp | timestamp_time
> 2022 | timestamp | text | text_timestamp
> 1191 | timestamptz | text | text_timestamptz
> 938 | timetz | text | text_timetz
> 1388 | timetz | timestamptz | timestamptz_timetz
> 1619 | varchar | int4 | int4_text
> 1623 | varchar | int8 | int8_text
> (66 rows)
>
>
> Regression failures with this set of choices (I've edited the output to
> remove diffs that are merely consequences of the actual failures):
>
> *** ./expected/char.out Mon May 21 12:54:46 2001
> --- ./results/char.out Wed Apr 10 11:48:16 2002
> ***************
> *** 18,23 ****
> --- 18,25 ----
> -- any of the following three input formats are acceptable
> INSERT INTO CHAR_TBL (f1) VALUES ('1');
> INSERT INTO CHAR_TBL (f1) VALUES (2);
> + ERROR: column "f1" is of type 'character' but expression is of type 'integer'
> + You will need to rewrite or cast the expression
> INSERT INTO CHAR_TBL (f1) VALUES ('3');
> -- zero-length char
> INSERT INTO CHAR_TBL (f1) VALUES ('');
>
> *** ./expected/varchar.out Mon May 21 12:54:46 2001
> --- ./results/varchar.out Wed Apr 10 11:48:17 2002
> ***************
> *** 7,12 ****
> --- 7,14 ----
> -- any of the following three input formats are acceptable
> INSERT INTO VARCHAR_TBL (f1) VALUES ('1');
> INSERT INTO VARCHAR_TBL (f1) VALUES (2);
> + ERROR: column "f1" is of type 'character varying' but expression is of type 'integer'
> + You will need to rewrite or cast the expression
> INSERT INTO VARCHAR_TBL (f1) VALUES ('3');
> -- zero-length char
> INSERT INTO VARCHAR_TBL (f1) VALUES ('');
>
> *** ./expected/strings.out Fri Jun 1 13:49:17 2001
> --- ./results/strings.out Wed Apr 10 11:49:29 2002
> ***************
> *** 137,147 ****
> (1 row)
>
> SELECT POSITION(5 IN '1234567890') = '5' AS "5";
> ! 5
> ! ---
> ! t
> ! (1 row)
> !
> --
> -- test LIKE
> -- Be sure to form every test as a LIKE/NOT LIKE pair.
> --- 137,145 ----
> (1 row)
>
> SELECT POSITION(5 IN '1234567890') = '5' AS "5";
> ! ERROR: Function 'pg_catalog.position(unknown, int4)' does not exist
> ! Unable to identify a function that satisfies the given argument types
> ! You may need to add explicit typecasts
> --
> -- test LIKE
> -- Be sure to form every test as a LIKE/NOT LIKE pair.
>
> *** ./expected/alter_table.out Fri Apr 5 12:03:45 2002
> --- ./results/alter_table.out Wed Apr 10 11:51:06 2002
> ***************
> *** 363,374 ****
> CREATE TEMP TABLE FKTABLE (ftest1 varchar);
> ALTER TABLE FKTABLE ADD FOREIGN KEY(ftest1) references pktable;
> NOTICE: ALTER TABLE will create implicit trigger(s) for FOREIGN KEY check(s)
> -- As should this
> ALTER TABLE FKTABLE ADD FOREIGN KEY(ftest1) references pktable(ptest1);
> NOTICE: ALTER TABLE will create implicit trigger(s) for FOREIGN KEY check(s)
> DROP TABLE pktable;
> - NOTICE: DROP TABLE implicitly drops referential integrity trigger from table "fktable"
> - NOTICE: DROP TABLE implicitly drops referential integrity trigger from table "fktable"
> DROP TABLE fktable;
> CREATE TEMP TABLE PKTABLE (ptest1 int, ptest2 text,
> PRIMARY KEY(ptest1, ptest2));
> --- 363,376 ----
> CREATE TEMP TABLE FKTABLE (ftest1 varchar);
> ALTER TABLE FKTABLE ADD FOREIGN KEY(ftest1) references pktable;
> NOTICE: ALTER TABLE will create implicit trigger(s) for FOREIGN KEY check(s)
> + ERROR: Unable to identify an operator '=' for types 'character varying' and 'integer'
> + You will have to retype this query using an explicit cast
> -- As should this
> ALTER TABLE FKTABLE ADD FOREIGN KEY(ftest1) references pktable(ptest1);
> NOTICE: ALTER TABLE will create implicit trigger(s) for FOREIGN KEY check(s)
> + ERROR: Unable to identify an operator '=' for types 'character varying' and 'integer'
> + You will have to retype this query using an explicit cast
> DROP TABLE pktable;
> DROP TABLE fktable;
> CREATE TEMP TABLE PKTABLE (ptest1 int, ptest2 text,
> PRIMARY KEY(ptest1, ptest2));
>
> *** ./expected/rules.out Thu Mar 21 10:24:35 2002
> --- ./results/rules.out Wed Apr 10 11:51:11 2002
> ***************
> *** 1026,1037 ****
> 'Al Bundy',
> 'epoch'::text
> );
> UPDATE shoelace_data SET sl_avail = 6 WHERE sl_name = 'sl7';
> SELECT * FROM shoelace_log;
> sl_name | sl_avail | log_who | log_when
> ! ------------+----------+----------+--------------------------
> ! sl7 | 6 | Al Bundy | Thu Jan 01 00:00:00 1970
> ! (1 row)
>
> CREATE RULE shoelace_ins AS ON INSERT TO shoelace
> DO INSTEAD
> --- 1026,1038 ----
> 'Al Bundy',
> 'epoch'::text
> );
> + ERROR: column "log_when" is of type 'timestamp without time zone' but expression is of type 'text'
> + You will need to rewrite or cast the expression
> UPDATE shoelace_data SET sl_avail = 6 WHERE sl_name = 'sl7';
> SELECT * FROM shoelace_log;
> sl_name | sl_avail | log_who | log_when
> ! ---------+----------+---------+----------
> ! (0 rows)
>
> CREATE RULE shoelace_ins AS ON INSERT TO shoelace
> DO INSTEAD
>
> *** ./expected/foreign_key.out Wed Mar 6 01:10:56 2002
> --- ./results/foreign_key.out Wed Apr 10 11:51:17 2002
> ***************
> *** 733,747 ****
> -- because varchar=int does exist
> CREATE TABLE FKTABLE (ftest1 varchar REFERENCES pktable);
> NOTICE: CREATE TABLE will create implicit trigger(s) for FOREIGN KEY check(s)
> DROP TABLE FKTABLE;
> ! NOTICE: DROP TABLE implicitly drops referential integrity trigger from table "pktable"
> ! NOTICE: DROP TABLE implicitly drops referential integrity trigger from table "pktable"
> -- As should this
> CREATE TABLE FKTABLE (ftest1 varchar REFERENCES pktable(ptest1));
> NOTICE: CREATE TABLE will create implicit trigger(s) for FOREIGN KEY check(s)
> DROP TABLE FKTABLE;
> ! NOTICE: DROP TABLE implicitly drops referential integrity trigger from table "pktable"
> ! NOTICE: DROP TABLE implicitly drops referential integrity trigger from table "pktable"
> DROP TABLE PKTABLE;
> -- Two columns, two tables
> CREATE TABLE PKTABLE (ptest1 int, ptest2 text, PRIMARY KEY(ptest1, ptest2));
> --- 733,749 ----
> -- because varchar=int does exist
> CREATE TABLE FKTABLE (ftest1 varchar REFERENCES pktable);
> NOTICE: CREATE TABLE will create implicit trigger(s) for FOREIGN KEY check(s)
> + ERROR: Unable to identify an operator '=' for types 'character varying' and 'integer'
> + You will have to retype this query using an explicit cast
> DROP TABLE FKTABLE;
> ! ERROR: table "fktable" does not exist
> -- As should this
> CREATE TABLE FKTABLE (ftest1 varchar REFERENCES pktable(ptest1));
> NOTICE: CREATE TABLE will create implicit trigger(s) for FOREIGN KEY check(s)
> + ERROR: Unable to identify an operator '=' for types 'character varying' and 'integer'
> + You will have to retype this query using an explicit cast
> DROP TABLE FKTABLE;
> ! ERROR: table "fktable" does not exist
> DROP TABLE PKTABLE;
> -- Two columns, two tables
> CREATE TABLE PKTABLE (ptest1 int, ptest2 text, PRIMARY KEY(ptest1, ptest2));
>
> *** ./expected/domain.out Wed Mar 20 13:34:37 2002
> --- ./results/domain.out Wed Apr 10 11:51:23 2002
> ***************
> *** 111,116 ****
> --- 111,118 ----
> create domain ddef2 oid DEFAULT '12';
> -- Type mixing, function returns int8
> create domain ddef3 text DEFAULT 5;
> + ERROR: Column "ddef3" is of type text but default expression is of type integer
> + You will need to rewrite or cast the expression
> create sequence ddef4_seq;
> create domain ddef4 int4 DEFAULT nextval(cast('ddef4_seq' as text));
> create domain ddef5 numeric(8,2) NOT NULL DEFAULT '12.12';
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>