From 543d22fc7423747afd59fe7214f2ddf6259efc62 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 7 Nov 2010 13:03:19 -0500 Subject: [PATCH] Prevent invoking I/O conversion casts via functional/attribute notation. PG 8.4 added a built-in feature for casting pretty much any data type to string types (text, varchar, etc). We allowed this to work in any of the historically-allowed syntaxes: CAST(x AS text), x::text, text(x), or x.text. However, multiple complaints have shown that it's too easy to invoke such casts unintentionally in the latter two styles, particularly field selection. To cure the problem with the narrowest possible change of behavior, disallow use of I/O conversion casts from composite types to string types via functional/attribute syntax. The new functionality is still available via cast syntax. In passing, document the equivalence of functional and attribute syntax in a more visible place. --- doc/src/sgml/ref/create_cast.sgml | 27 +++++++++++++++----- doc/src/sgml/syntax.sgml | 13 ++++++++++ doc/src/sgml/xfunc.sgml | 8 +++++- src/backend/parser/parse_func.c | 28 ++++++++++++++++++--- src/test/regress/expected/rowtypes.out | 46 ++++++++++++++++++++++++++++++++++ src/test/regress/sql/rowtypes.sql | 15 +++++++++++ 6 files changed, 126 insertions(+), 11 deletions(-) diff --git a/doc/src/sgml/ref/create_cast.sgml b/doc/src/sgml/ref/create_cast.sgml index 5e74efdcac..d1c540da8a 100644 --- a/doc/src/sgml/ref/create_cast.sgml +++ b/doc/src/sgml/ref/create_cast.sgml @@ -38,7 +38,7 @@ CREATE CAST (source_type AS target_type< CREATE CAST defines a new cast. A cast specifies how to perform a conversion between - two data types. For example: + two data types. For example, SELECT CAST(42 AS float8); @@ -64,10 +64,13 @@ SELECT CAST(42 AS float8); - You can define a cast as an I/O conversion cast using + You can define a cast as an I/O conversion cast by using the WITH INOUT syntax. An I/O conversion cast is performed by invoking the output function of the source data type, and - passing the result to the input function of the target data type. + passing the resulting string to the input function of the target data type. + In many common cases, this feature avoids the need to write a separate + cast function for conversion. An I/O conversion cast acts the same as + a regular function-based cast; only the implementation is different. @@ -218,7 +221,7 @@ SELECT CAST ( 2 AS numeric ) + 4.0; Indicates that the cast is an I/O conversion cast, performed by invoking the output function of the source data type, and passing the - result to the input function of the target data type. + resulting string to the input function of the target data type. @@ -278,9 +281,9 @@ SELECT CAST ( 2 AS numeric ) + 4.0; When a cast has different source and target types and a function that takes more than one argument, it - represents converting from one type to another and applying a length + supports converting from one type to another and applying a length coercion in a single step. When no such entry is available, coercion - to a type that uses a type modifier involves two steps, one to + to a type that uses a type modifier involves two cast steps, one to convert between data types and a second to apply the modifier. @@ -366,6 +369,18 @@ SELECT CAST ( 2 AS numeric ) + 4.0; syntax. + + + + There's an exception to the exception, too: I/O conversion casts from + composite types to string types cannot be invoked using functional + syntax, but must be written in explicit cast syntax (either + CAST or :: notation). This exception was added + because after the introduction of automatically-provided I/O conversion + casts, it was found too easy to accidentally invoke such a cast when + a function or column reference was intended. + + diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml index e8e0da96b9..2ebea7cf34 100644 --- a/doc/src/sgml/syntax.sgml +++ b/doc/src/sgml/syntax.sgml @@ -1522,6 +1522,19 @@ sqrt(2) The arguments can optionally have names attached. See for details. + + + + A function that takes a single argument of composite type can + optionally be called using field-selection syntax, and conversely + field selection can be written in functional style. That is, the + notations col(table) and table.col are + interchangeable. This behavior is not SQL-standard but is provided + in PostgreSQL because it allows use of functions to + emulate computed fields. For more information see + . + + diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index 9fb2be4aec..e79c1f2923 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -271,7 +271,7 @@ $$ LANGUAGE SQL; - + <acronym>SQL</acronym> Functions on Composite Types @@ -492,6 +492,12 @@ SELECT emp.name, emp.double_salary FROM emp; double_salary isn't a real column of the table. (You can also emulate computed fields with views.) + + + Because of this behavior, it's unwise to give a function that takes + a single composite-type argument the same name as any of the fields of + that composite type. + diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index b50bce4487..9bb100e0c1 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -985,8 +985,13 @@ func_get_detail(List *funcname, * can't write "foo[] (something)" as a function call. In theory * someone might want to invoke it as "_foo (something)" but we have * never supported that historically, so we can insist that people - * write it as a normal cast instead. Lack of historical support is - * also the reason for not considering composite-type casts here. + * write it as a normal cast instead. + * + * We also reject the specific case of COERCEVIAIO for a composite + * source type and a string-category target type. This is a case that + * find_coercion_pathway() allows by default, but experience has shown + * that it's too commonly invoked by mistake. So, again, insist that + * people use cast syntax if they want to do that. * * NB: it's important that this code does not exceed what coerce_type * can do, because the caller will try to apply coerce_type if we @@ -1017,8 +1022,23 @@ func_get_detail(List *funcname, cpathtype = find_coercion_pathway(targetType, sourceType, COERCION_EXPLICIT, &cfuncid); - iscoercion = (cpathtype == COERCION_PATH_RELABELTYPE || - cpathtype == COERCION_PATH_COERCEVIAIO); + switch (cpathtype) + { + case COERCION_PATH_RELABELTYPE: + iscoercion = true; + break; + case COERCION_PATH_COERCEVIAIO: + if ((sourceType == RECORDOID || + ISCOMPLEX(sourceType)) && + TypeCategory(targetType) == TYPCATEGORY_STRING) + iscoercion = false; + else + iscoercion = true; + break; + default: + iscoercion = false; + break; + } } if (iscoercion) diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out index a21f7b8c06..e5cd71421c 100644 --- a/src/test/regress/expected/rowtypes.out +++ b/src/test/regress/expected/rowtypes.out @@ -324,3 +324,49 @@ select * from price; (3 rows) rollback; +-- +-- We allow I/O conversion casts from composite types to strings to be +-- invoked via cast syntax, but not functional syntax. This is because +-- the latter is too prone to be invoked unintentionally. +-- +select cast (fullname as text) from fullname; + fullname +---------- +(0 rows) + +select fullname::text from fullname; + fullname +---------- +(0 rows) + +select text(fullname) from fullname; -- error +ERROR: function text(fullname) does not exist +LINE 1: select text(fullname) from fullname; + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +select fullname.text from fullname; -- error +ERROR: column fullname.text does not exist +LINE 1: select fullname.text from fullname; + ^ +-- same, but RECORD instead of named composite type: +select cast (row('Jim', 'Beam') as text); + row +------------ + (Jim,Beam) +(1 row) + +select (row('Jim', 'Beam'))::text; + row +------------ + (Jim,Beam) +(1 row) + +select text(row('Jim', 'Beam')); -- error +ERROR: function text(record) does not exist +LINE 1: select text(row('Jim', 'Beam')); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +select (row('Jim', 'Beam')).text; -- error +ERROR: could not identify column "text" in record data type +LINE 1: select (row('Jim', 'Beam')).text; + ^ diff --git a/src/test/regress/sql/rowtypes.sql b/src/test/regress/sql/rowtypes.sql index e5a77f79f6..9041df147f 100644 --- a/src/test/regress/sql/rowtypes.sql +++ b/src/test/regress/sql/rowtypes.sql @@ -157,3 +157,18 @@ UPDATE price select * from price; rollback; + +-- +-- We allow I/O conversion casts from composite types to strings to be +-- invoked via cast syntax, but not functional syntax. This is because +-- the latter is too prone to be invoked unintentionally. +-- +select cast (fullname as text) from fullname; +select fullname::text from fullname; +select text(fullname) from fullname; -- error +select fullname.text from fullname; -- error +-- same, but RECORD instead of named composite type: +select cast (row('Jim', 'Beam') as text); +select (row('Jim', 'Beam'))::text; +select text(row('Jim', 'Beam')); -- error +select (row('Jim', 'Beam')).text; -- error -- 2.11.0