Re: Implicit coercions need to be reined in - Mailing list pgsql-hackers

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
>