Re: Implicit coercions need to be reined in

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';