Hi
I am sending a proof concept. Current implementation is not suboptimal - I
wrote this code for demonstration of current issues, and checking possible
side effects of changes in this patch.
The basic problem is strong restrictive implementation of polymorphic types
- now these types doesn't allow any cast although it is possible. It can be
changed relatively simply I though (after we implemented variadic
functions).
CREATE OR REPLACE FUNCTION public.foo1(anyelement, anyelement)
RETURNS anyelement
LANGUAGE sql
AS $function$
SELECT $1 + $2;
$function$
CREATE OR REPLACE FUNCTION public.foo2(anyelement, anyelement)
RETURNS anyarray
LANGUAGE sql
AS $function$
SELECT ARRAY[$1, $2]
$function$
Now, polymorphic functions don't allow some natively expected calls:
postgres=# select foo1(1,1);
foo1
------
2
(1 row)
postgres=# select foo1(1,1.1);
ERROR: function foo1(integer, numeric) does not exist
LINE 1: select foo1(1,1.1);
^
HINT: No function matches the given name and argument types. You might
need to add explicit type casts.
postgres=# select foo2(1,1);
foo2
-------
{1,1}
(1 row)
postgres=# select foo2(1,1.1);
ERROR: function foo2(integer, numeric) does not exist
LINE 1: select foo2(1,1.1);
^
HINT: No function matches the given name and argument types. You might
need to add explicit type casts.
CREATE OR REPLACE FUNCTION public.foo3(VARIADIC anyarray)
RETURNS anyelement
LANGUAGE sql
AS $function$
SELECT min(v) FROM unnest($1) g(v)
$function$
postgres=# SELECT foo3(1,2,3);
foo3
------
1
(1 row)
postgres=# SELECT foo3(1,2,3.1);
ERROR: function foo3(integer, integer, numeric) does not exist
LINE 1: SELECT foo3(1,2,3.1);
^
HINT: No function matches the given name and argument types. You might
need to add explicit type casts.
Some our functions like COALESCE are not too restrictive and allow to use
types from same category.
postgres=# select coalesce(1,1.1);
coalesce
----------
1
(1 row)
With attached patch the polymorphic functions use same mechanism as our
buildin functions. It is applied on ANYARRAY, ANYELEMENT types only.
postgres=# select foo1(1,1.1), foo2(1,1.1), foo3(1.1,2,3.1);
foo1 | foo2 | foo3
------+---------+------
2.1 | {1,1.1} | 1.1
(1 row)
Comments, notices, ... ?
Regards
Pavel
2014-11-24 20:52 GMT+01:00 Pavel Stehule <pavel.steh...@gmail.com>:
> Hello
>
> now a functions with more than one polymorphic arguments are relative
> fragile due missing casting to most common type. Some our "functions" like
> "coalesce" can do it, so it is surprising for our users.
>
> our custom polymorphic function foo(anyelement, anyelement) working well
> for
>
> foo(10,20) or foo(10.1, 20.1), but not for foo(10, 20.1)
>
> I am thinking, so we can add a searching most common type stage without
> breaking to backing compatibility.
>
> What do you think about it?
>
> Regards
>
> Pavel
>
commit 4eb8c9bf9e5b2d20d69e8e68fa34e760537347c2
Author: Pavel Stehule <pavel.steh...@gooddata.com>
Date: Sun Mar 8 19:28:49 2015 +0100
proof concept
diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index a4e494b..92e63de 100644
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -1261,6 +1261,100 @@ select_common_type(ParseState *pstate, List *exprs, const char *context,
}
/*
+ * select_common_type_from_vector()
+ * Determine the common supertype of vector of Oids.
+ *
+ * Similar to select_common_type() but simplified for polymorphics
+ * type processing. When there are no supertype, then returns InvalidOid,
+ * when noerror is true, or raise exception when noerror is false.
+ */
+Oid
+select_common_type_from_vector(int nargs, Oid *typeids, bool noerror)
+{
+ int i = 0;
+ Oid ptype;
+ TYPCATEGORY pcategory;
+ bool pispreferred;
+
+ Assert(nargs > 0);
+ ptype = typeids[0];
+
+ /* fast leave when all types are same */
+ if (ptype != UNKNOWNOID)
+ {
+ for (i = 1; i < nargs; i++)
+ {
+ if (ptype != typeids[i])
+ break;
+ }
+
+ if (i == nargs)
+ return ptype;
+ }
+
+ /*
+ * Nope, so set up for the full algorithm. Note that at this point, lc
+ * points to the first list item with type different from pexpr's; we need
+ * not re-examine any items the previous loop advanced over.
+ */
+ ptype = getBaseType(ptype);
+ get_type_category_preferred(ptype, &pcategory, &pispreferred);
+
+ for (; i < nargs; i++)
+ {
+ Oid ntype = getBaseType(typeids[i]);
+
+ /* move on to next one if no new information... */
+ if (ntype != UNKNOWNOID && ntype != ptype)
+ {
+ TYPCATEGORY ncategory;
+ bool nispreferred;
+
+ get_type_category_preferred(ntype, &ncategory, &nispreferred);
+
+ if (ptype == UNKNOWNOID)
+ {
+ /* so far, only unknowns so take anything... */
+ ptype = ntype;
+ pcategory = ncategory;
+ pispreferred = nispreferred;
+ }
+ else if (ncategory != pcategory)
+ {
+ if (noerror)
+ return InvalidOid;
+
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("types %s and %s cannot be matched",
+ format_type_be(ptype),
+ format_type_be(ntype))));
+ }
+ else if (!pispreferred &&
+ can_coerce_type(1, &ptype, &ntype, COERCION_IMPLICIT) &&
+ !can_coerce_type(1, &ntype, &ptype, COERCION_IMPLICIT))
+ {
+ /*
+ * take new type if can coerce to it implicitly but not the
+ * other way; but if we have a preferred type, stay on it.
+ */
+ ptype = ntype;
+ pcategory = ncategory;
+ pispreferred = nispreferred;
+ }
+ }
+ }
+
+ /*
+ * Be consistent with select_common_type()
+ */
+ if (ptype == UNKNOWNOID)
+ ptype = TEXTOID;
+
+ return ptype;
+}
+
+/*
* coerce_to_common_type()
* Coerce an expression to the given type.
*
@@ -1301,14 +1395,14 @@ coerce_to_common_type(ParseState *pstate, Node *node,
*
* The argument consistency rules are:
*
- * 1) All arguments declared ANYELEMENT must have the same datatype.
- * 2) All arguments declared ANYARRAY must have the same datatype,
+ * 1) All arguments declared ANYELEMENT must have common supertype.
+ * 2) All arguments declared ANYARRAY must have the common supertype,
* which must be a varlena array type.
* 3) All arguments declared ANYRANGE must have the same datatype,
* which must be a range type.
* 4) If there are arguments of both ANYELEMENT and ANYARRAY, make sure the
* actual ANYELEMENT datatype is in fact the element type for the actual
- * ANYARRAY datatype.
+ * ANYARRAY datatype (can be supertype for both)
* 5) Similarly, if there are arguments of both ANYELEMENT and ANYRANGE,
* make sure the actual ANYELEMENT datatype is in fact the subtype for
* the actual ANYRANGE type.
@@ -1349,13 +1443,16 @@ check_generic_type_consistency(Oid *actual_arg_types,
{
int j;
Oid elem_typeid = InvalidOid;
- Oid array_typeid = InvalidOid;
- Oid array_typelem;
Oid range_typeid = InvalidOid;
+ Oid enum_typeid = InvalidOid;
Oid range_typelem;
bool have_anyelement = false;
+ bool have_anyarray = false;
bool have_anynonarray = false;
bool have_anyenum = false;
+ Oid actual_types[FUNC_MAX_ARGS];
+ int n_actual_types = 0;
+ bool anyarrayarg_is_used = false;
/*
* Loop through the arguments to see if we have any that are polymorphic.
@@ -1367,28 +1464,58 @@ check_generic_type_consistency(Oid *actual_arg_types,
Oid actual_type = actual_arg_types[j];
if (decl_type == ANYELEMENTOID ||
- decl_type == ANYNONARRAYOID ||
- decl_type == ANYENUMOID)
+ decl_type == ANYNONARRAYOID)
{
have_anyelement = true;
if (decl_type == ANYNONARRAYOID)
have_anynonarray = true;
- else if (decl_type == ANYENUMOID)
- have_anyenum = true;
+
+ /* Store actual_type to buffer, reduce repetation */
+ if ((n_actual_types > 0 && actual_types[n_actual_types - 1] != actual_type)
+ || (n_actual_types == 0))
+ actual_types[n_actual_types++] = actual_type;
+ }
+ else if (decl_type == ANYENUMOID)
+ {
+ /*
+ * ToDo: possible bug in other parts of Postgres
+ * Probably it should to work like anyelement. So broken
+ * regress tests should be fixed somewhere else.
+ */
+
+ /* don't try to work with common super type with enums */
+ have_anyenum = true;
+
if (actual_type == UNKNOWNOID)
continue;
- if (OidIsValid(elem_typeid) && actual_type != elem_typeid)
+ actual_type = getBaseType(actual_type); /* flatten domains */
+ if (OidIsValid(enum_typeid) && actual_type != enum_typeid)
return false;
- elem_typeid = actual_type;
+ enum_typeid = actual_type;
}
else if (decl_type == ANYARRAYOID)
{
- if (actual_type == UNKNOWNOID)
+ have_anyarray = true;
+
+ if (actual_type == ANYARRAYOID)
+ {
+ anyarrayarg_is_used = true;
continue;
- actual_type = getBaseType(actual_type); /* flatten domains */
- if (OidIsValid(array_typeid) && actual_type != array_typeid)
- return false;
- array_typeid = actual_type;
+ }
+
+ if (actual_type != UNKNOWNOID)
+ {
+ actual_type = getBaseType(actual_type); /* flatten domains */
+ actual_type = get_element_type(actual_type);
+
+ if (!OidIsValid(actual_type))
+ return false; /* should be an array, but isn't */
+ }
+
+ /* Store actual_type to buffer, reduce repetation */
+ if ((n_actual_types > 0 && actual_types[n_actual_types - 1] != actual_type)
+ || (n_actual_types == 0))
+ actual_types[n_actual_types++] = actual_type;
}
else if (decl_type == ANYRANGEOID)
{
@@ -1401,32 +1528,33 @@ check_generic_type_consistency(Oid *actual_arg_types,
}
}
- /* Get the element type based on the array type, if we have one */
- if (OidIsValid(array_typeid))
+ /*
+ * Get common super type for ANYELEMENT, ANYARRAY types. Cannot to
+ * raise error here due block to search other function alternatives.
+ */
+ if (have_anyelement || have_anyarray)
{
- if (array_typeid == ANYARRAYOID)
- {
- /* Special case for ANYARRAY input: okay iff no ANYELEMENT */
- if (have_anyelement)
- return false;
- return true;
- }
-
- array_typelem = get_element_type(array_typeid);
- if (!OidIsValid(array_typelem))
- return false; /* should be an array, but isn't */
+ Oid supertyp = select_common_type_from_vector(n_actual_types, actual_types, true);
+ if (!OidIsValid(supertyp))
+ return false;
- if (!OidIsValid(elem_typeid))
+ if (have_anyelement)
+ elem_typeid = supertyp;
+ if (have_anyarray)
{
- /*
- * if we don't have an element type yet, use the one we just got
- */
- elem_typeid = array_typelem;
- }
- else if (array_typelem != elem_typeid)
- {
- /* otherwise, they better match */
- return false;
+ if (anyarrayarg_is_used)
+ {
+ if (have_anyelement)
+ return false;
+
+ /* Special case for ANYARRAY input: okay iff no ANYELEMENT */
+ return true;
+ }
+ else
+ {
+ if (!OidIsValid(elem_typeid))
+ elem_typeid = supertyp;
+ }
}
}
@@ -1461,7 +1589,7 @@ check_generic_type_consistency(Oid *actual_arg_types,
if (have_anyenum)
{
/* require the element type to be an enum */
- if (!type_is_enum(elem_typeid))
+ if (!type_is_enum(enum_typeid))
return false;
}
@@ -1552,13 +1680,16 @@ enforce_generic_type_consistency(Oid *actual_arg_types,
Oid elem_typeid = InvalidOid;
Oid array_typeid = InvalidOid;
Oid range_typeid = InvalidOid;
- Oid array_typelem;
Oid range_typelem;
bool have_anyelement = (rettype == ANYELEMENTOID ||
rettype == ANYNONARRAYOID ||
rettype == ANYENUMOID);
bool have_anynonarray = (rettype == ANYNONARRAYOID);
bool have_anyenum = (rettype == ANYENUMOID);
+ Oid actual_types[FUNC_MAX_ARGS];
+ int n_actual_types = 0;
+ bool anyarrayarg_is_used = false;
+ bool have_anyarray = (rettype == ANYARRAYOID);
/*
* Loop through the arguments to see if we have any that are polymorphic.
@@ -1578,41 +1709,48 @@ enforce_generic_type_consistency(Oid *actual_arg_types,
have_anynonarray = true;
else if (decl_type == ANYENUMOID)
have_anyenum = true;
+
if (actual_type == UNKNOWNOID)
- {
have_unknowns = true;
- continue;
- }
- if (allow_poly && decl_type == actual_type)
+ else if (allow_poly && decl_type == actual_type)
continue; /* no new information here */
- if (OidIsValid(elem_typeid) && actual_type != elem_typeid)
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("arguments declared \"anyelement\" are not all alike"),
- errdetail("%s versus %s",
- format_type_be(elem_typeid),
- format_type_be(actual_type))));
- elem_typeid = actual_type;
+
+ /* Store actual_type to buffer, reduce repetation */
+ if ((n_actual_types > 0 && actual_types[n_actual_types - 1] != actual_type)
+ || (n_actual_types == 0))
+ actual_types[n_actual_types++] = actual_type;
}
else if (decl_type == ANYARRAYOID)
{
have_generics = true;
+ have_anyarray = true;
if (actual_type == UNKNOWNOID)
- {
have_unknowns = true;
+ else if (allow_poly && decl_type == actual_type)
+ continue; /* no new information here */
+
+ if (actual_type == ANYARRAYOID)
+ {
+ anyarrayarg_is_used = true;
continue;
}
- if (allow_poly && decl_type == actual_type)
- continue; /* no new information here */
- actual_type = getBaseType(actual_type); /* flatten domains */
- if (OidIsValid(array_typeid) && actual_type != array_typeid)
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("arguments declared \"anyarray\" are not all alike"),
- errdetail("%s versus %s",
- format_type_be(array_typeid),
- format_type_be(actual_type))));
- array_typeid = actual_type;
+
+ if (actual_type != UNKNOWNOID)
+ {
+ actual_type = getBaseType(actual_type); /* flatten domains */
+ actual_type = get_element_type(actual_type);
+
+ if (!OidIsValid(actual_type))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("argument declared \"anyarray\" is not an array but type %s",
+ format_type_be(actual_type))));
+ }
+
+ /* Store actual_type to buffer, reduce repetation */
+ if ((n_actual_types > 0 && actual_types[n_actual_types - 1] != actual_type)
+ || (n_actual_types == 0))
+ actual_types[n_actual_types++] = actual_type;
}
else if (decl_type == ANYRANGEOID)
{
@@ -1643,40 +1781,32 @@ enforce_generic_type_consistency(Oid *actual_arg_types,
if (!have_generics)
return rettype;
- /* Get the element type based on the array type, if we have one */
- if (OidIsValid(array_typeid))
+ if (anyarrayarg_is_used)
{
- if (array_typeid == ANYARRAYOID && !have_anyelement)
- {
- /* Special case for ANYARRAY input: okay iff no ANYELEMENT */
- array_typelem = ANYELEMENTOID;
- }
- else
+ array_typeid = ANYARRAYOID;
+ elem_typeid = ANYELEMENTOID;
+ }
+ else if ((have_anyelement || have_anyarray) && n_actual_types > 0)
+ {
+ Oid supertyp;
+
+ /* returns supertyp or raise error */
+ supertyp = select_common_type_from_vector(n_actual_types, actual_types, false);
+
+ Assert(OidIsValid(supertyp));
+
+ if (have_anyelement)
+ elem_typeid = supertyp;
+
+ if (have_anyarray)
{
- array_typelem = get_element_type(array_typeid);
- if (!OidIsValid(array_typelem))
+ array_typeid = get_array_type(supertyp);
+ if (!OidIsValid(array_typeid))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("argument declared \"anyarray\" is not an array but type %s",
- format_type_be(array_typeid))));
- }
-
- if (!OidIsValid(elem_typeid))
- {
- /*
- * if we don't have an element type yet, use the one we just got
- */
- elem_typeid = array_typelem;
- }
- else if (array_typelem != elem_typeid)
- {
- /* otherwise, they better match */
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("argument declared \"anyarray\" is not consistent with argument declared \"anyelement\""),
- errdetail("%s versus %s",
- format_type_be(array_typeid),
- format_type_be(elem_typeid))));
+ errmsg("There are no array type for type %s",
+ format_type_be(supertyp))));
+ elem_typeid = supertyp;
}
}
@@ -1757,15 +1887,11 @@ enforce_generic_type_consistency(Oid *actual_arg_types,
/*
* If we had any unknown inputs, re-scan to assign correct types
*/
- if (have_unknowns)
+ if (have_unknowns || have_anyelement || have_anyarray)
{
for (j = 0; j < nargs; j++)
{
Oid decl_type = declared_arg_types[j];
- Oid actual_type = actual_arg_types[j];
-
- if (actual_type != UNKNOWNOID)
- continue;
if (decl_type == ANYELEMENTOID ||
decl_type == ANYNONARRAYOID ||
diff --git a/src/include/parser/parse_coerce.h b/src/include/parser/parse_coerce.h
index ec0ee14..06fcd3b 100644
--- a/src/include/parser/parse_coerce.h
+++ b/src/include/parser/parse_coerce.h
@@ -64,6 +64,7 @@ extern int parser_coercion_errposition(ParseState *pstate,
extern Oid select_common_type(ParseState *pstate, List *exprs,
const char *context, Node **which_expr);
+extern Oid select_common_type_from_vector(int ntypes, Oid *typeids, bool noerror);
extern Node *coerce_to_common_type(ParseState *pstate, Node *node,
Oid targetTypeId,
const char *context);
diff --git a/src/test/regress/expected/polymorphism.out b/src/test/regress/expected/polymorphism.out
index 27b2879..74ea2b5 100644
--- a/src/test/regress/expected/polymorphism.out
+++ b/src/test/regress/expected/polymorphism.out
@@ -629,7 +629,7 @@ create aggregate build_group(int8, integer) (
SFUNC = add_group,
STYPE = int2[]
);
-ERROR: function add_group(smallint[], bigint, integer) does not exist
+ERROR: function add_group(bigint[], bigint, integer) requires run-time type coercion
-- but we can make a non-poly agg from a poly sfunc if types are OK
create aggregate build_group(int8, integer) (
SFUNC = add_group,
@@ -1389,3 +1389,66 @@ View definition:
drop view dfview;
drop function dfunc(anyelement, anyelement, bool);
+create function foo1(anyelement, anyelement)
+returns anyelement as $$
+ select $1 + $2;
+$$ language sql;
+select foo1(1,2);
+ foo1
+------
+ 3
+(1 row)
+
+select foo1(1.1,2);
+ foo1
+------
+ 3.1
+(1 row)
+
+create function foo2(anyelement, anyelement)
+returns anyarray as $$
+ select ARRAY[$1, $2]
+$$ language sql;
+select foo2(1,2);
+ foo2
+-------
+ {1,2}
+(1 row)
+
+select foo2(1.1,2);
+ foo2
+---------
+ {1.1,2}
+(1 row)
+
+select foo2(2,1.1);
+ foo2
+---------
+ {2,1.1}
+(1 row)
+
+create function foo3(variadic anyarray)
+returns anyelement as $$
+ select min(v) from unnest($1) g(v)
+$$ language sql;
+select foo3(1,2,3,4);
+ foo3
+------
+ 1
+(1 row)
+
+select foo3(1.1,2,3,4);
+ foo3
+------
+ 1.1
+(1 row)
+
+select foo3(1,2,3,3.3);
+ foo3
+------
+ 1
+(1 row)
+
+drop function foo1(anyelement, anyelement);
+drop function foo2(anyelement, anyelement);
+drop function foo3(anyarray);
diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index 7991e99..c29962d 100644
--- a/src/test/regress/expected/rangefuncs.out
+++ b/src/test/regress/expected/rangefuncs.out
@@ -1514,7 +1514,11 @@ SELECT dup(22);
(1 row)
SELECT dup('xyz'); -- fails
-ERROR: could not determine polymorphic type because input has type "unknown"
+ dup
+-------------------
+ (xyz,"{xyz,xyz}")
+(1 row)
+
SELECT dup('xyz'::text);
dup
-------------------
diff --git a/src/test/regress/sql/polymorphism.sql b/src/test/regress/sql/polymorphism.sql
index 3d8dd1e..303a0e5 100644
--- a/src/test/regress/sql/polymorphism.sql
+++ b/src/test/regress/sql/polymorphism.sql
@@ -761,3 +761,33 @@ select * from dfview;
drop view dfview;
drop function dfunc(anyelement, anyelement, bool);
+
+create function foo1(anyelement, anyelement)
+returns anyelement as $$
+ select $1 + $2;
+$$ language sql;
+
+select foo1(1,2);
+select foo1(1.1,2);
+
+create function foo2(anyelement, anyelement)
+returns anyarray as $$
+ select ARRAY[$1, $2]
+$$ language sql;
+
+select foo2(1,2);
+select foo2(1.1,2);
+select foo2(2,1.1);
+
+create function foo3(variadic anyarray)
+returns anyelement as $$
+ select min(v) from unnest($1) g(v)
+$$ language sql;
+
+select foo3(1,2,3,4);
+select foo3(1.1,2,3,4);
+select foo3(1,2,3,3.3);
+
+drop function foo1(anyelement, anyelement);
+drop function foo2(anyelement, anyelement);
+drop function foo3(anyarray);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
