Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 89c0a87

Browse files
tglsfdctglsfdc
committed
The original implementation of polymorphic aggregates didn't really get the
checking of argument compatibility right; although the problem is only exposed with multiple-input aggregates in which some arguments are polymorphic and some are not. Per bug #3852 from Sokolov Yura.
1 parent df62977 commit 89c0a87

File tree

9 files changed

+133
-57
lines changed

9 files changed

+133
-57
lines changed

‎src/backend/catalog/pg_aggregate.c

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/pg_aggregate.c,v 1.89 2008/01/01 19:45:48 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/pg_aggregate.c,v 1.90 2008/01/11 18:39:40 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -296,7 +296,6 @@ lookup_agg_function(List *fnName,
296296
FuncDetailCode fdresult;
297297
AclResult aclresult;
298298
int i;
299-
bool allPolyArgs = true;
300299

301300
/*
302301
* func_get_detail looks up the function in the catalogs, does
@@ -322,25 +321,15 @@ lookup_agg_function(List *fnName,
322321
func_signature_string(fnName, nargs, input_types))));
323322

324323
/*
325-
* If the given type(s) are all polymorphic, there's nothing we can check.
326-
* Otherwise, enforce consistency, and possibly refine the result type.
324+
* If there are any polymorphic types involved, enforce consistency, and
325+
* possibly refine the result type. It's OK if the result is still
326+
* polymorphic at this point, though.
327327
*/
328-
for (i = 0; i < nargs; i++)
329-
{
330-
if (!IsPolymorphicType(input_types[i]))
331-
{
332-
allPolyArgs = false;
333-
break;
334-
}
335-
}
336-
337-
if (!allPolyArgs)
338-
{
339-
*rettype = enforce_generic_type_consistency(input_types,
340-
true_oid_array,
341-
nargs,
342-
*rettype);
343-
}
328+
*rettype = enforce_generic_type_consistency(input_types,
329+
true_oid_array,
330+
nargs,
331+
*rettype,
332+
true);
344333

345334
/*
346335
* func_get_detail will find functions requiring run-time argument type

‎src/backend/executor/nodeAgg.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
* Portions Copyright (c) 1994, Regents of the University of California
6262
*
6363
* IDENTIFICATION
64-
* $PostgreSQL: pgsql/src/backend/executor/nodeAgg.c,v 1.155 2008/01/01 19:45:49 momjian Exp $
64+
* $PostgreSQL: pgsql/src/backend/executor/nodeAgg.c,v 1.156 2008/01/11 18:39:40 tgl Exp $
6565
*
6666
*-------------------------------------------------------------------------
6767
*/
@@ -1433,7 +1433,8 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
14331433
aggtranstype = enforce_generic_type_consistency(inputTypes,
14341434
declaredArgTypes,
14351435
agg_nargs,
1436-
aggtranstype);
1436+
aggtranstype,
1437+
false);
14371438
pfree(declaredArgTypes);
14381439
}
14391440

‎src/backend/optimizer/util/clauses.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.253 2008/01/01 19:45:50 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.254 2008/01/11 18:39:40 tgl Exp $
1212
*
1313
* HISTORY
1414
* AUTHOR DATE MAJOR EVENT
@@ -448,7 +448,8 @@ count_agg_clauses_walker(Node *node, AggClauseCounts *counts)
448448
aggtranstype = enforce_generic_type_consistency(inputTypes,
449449
declaredArgTypes,
450450
agg_nargs,
451-
aggtranstype);
451+
aggtranstype,
452+
false);
452453
pfree(declaredArgTypes);
453454
}
454455

‎src/backend/parser/parse_coerce.c

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/parser/parse_coerce.c,v 2.160 2008/01/01 19:45:50 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/parser/parse_coerce.c,v 2.161 2008/01/11 18:39:40 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -1255,22 +1255,27 @@ check_generic_type_consistency(Oid *actual_arg_types,
12551255
* we add the extra condition that the ANYELEMENT type must not be an array.
12561256
* (This is a no-op if used in combination with ANYARRAY or ANYENUM, but
12571257
* is an extra restriction if not.)
1258+
*
1259+
* When allow_poly is false, we are not expecting any of the actual_arg_types
1260+
* to be polymorphic, and we should not return a polymorphic result type
1261+
* either. When allow_poly is true, it is okay to have polymorphic "actual"
1262+
* arg types, and we can return ANYARRAY or ANYELEMENT as the result. (This
1263+
* case is currently used only to check compatibility of an aggregate's
1264+
* declaration with the underlying transfn.)
12581265
*/
12591266
Oid
12601267
enforce_generic_type_consistency(Oid *actual_arg_types,
12611268
Oid *declared_arg_types,
12621269
int nargs,
1263-
Oid rettype)
1270+
Oid rettype,
1271+
bool allow_poly)
12641272
{
12651273
int j;
12661274
bool have_generics = false;
12671275
bool have_unknowns = false;
12681276
Oid elem_typeid = InvalidOid;
12691277
Oid array_typeid = InvalidOid;
12701278
Oid array_typelem;
1271-
bool have_anyelement = (rettype == ANYELEMENTOID ||
1272-
rettype == ANYNONARRAYOID ||
1273-
rettype == ANYENUMOID);
12741279
bool have_anynonarray = (rettype == ANYNONARRAYOID);
12751280
bool have_anyenum = (rettype == ANYENUMOID);
12761281

@@ -1287,7 +1292,7 @@ enforce_generic_type_consistency(Oid *actual_arg_types,
12871292
decl_type == ANYNONARRAYOID ||
12881293
decl_type == ANYENUMOID)
12891294
{
1290-
have_generics = have_anyelement = true;
1295+
have_generics = true;
12911296
if (decl_type == ANYNONARRAYOID)
12921297
have_anynonarray = true;
12931298
else if (decl_type == ANYENUMOID)
@@ -1297,6 +1302,8 @@ enforce_generic_type_consistency(Oid *actual_arg_types,
12971302
have_unknowns = true;
12981303
continue;
12991304
}
1305+
if (allow_poly && decl_type == actual_type)
1306+
continue; /* no new information here */
13001307
if (OidIsValid(elem_typeid) && actual_type != elem_typeid)
13011308
ereport(ERROR,
13021309
(errcode(ERRCODE_DATATYPE_MISMATCH),
@@ -1314,6 +1321,8 @@ enforce_generic_type_consistency(Oid *actual_arg_types,
13141321
have_unknowns = true;
13151322
continue;
13161323
}
1324+
if (allow_poly && decl_type == actual_type)
1325+
continue; /* no new information here */
13171326
if (OidIsValid(array_typeid) && actual_type != array_typeid)
13181327
ereport(ERROR,
13191328
(errcode(ERRCODE_DATATYPE_MISMATCH),
@@ -1335,20 +1344,12 @@ enforce_generic_type_consistency(Oid *actual_arg_types,
13351344
/* Get the element type based on the array type, if we have one */
13361345
if (OidIsValid(array_typeid))
13371346
{
1338-
if (array_typeid == ANYARRAYOID && !have_anyelement)
1339-
{
1340-
/* Special case for ANYARRAY input: okay iff no ANYELEMENT */
1341-
array_typelem = InvalidOid;
1342-
}
1343-
else
1344-
{
1345-
array_typelem = get_element_type(array_typeid);
1346-
if (!OidIsValid(array_typelem))
1347-
ereport(ERROR,
1348-
(errcode(ERRCODE_DATATYPE_MISMATCH),
1349-
errmsg("argument declared \"anyarray\" is not an array but type %s",
1350-
format_type_be(array_typeid))));
1351-
}
1347+
array_typelem = get_element_type(array_typeid);
1348+
if (!OidIsValid(array_typelem))
1349+
ereport(ERROR,
1350+
(errcode(ERRCODE_DATATYPE_MISMATCH),
1351+
errmsg("argument declared \"anyarray\" is not an array but type %s",
1352+
format_type_be(array_typeid))));
13521353

13531354
if (!OidIsValid(elem_typeid))
13541355
{
@@ -1370,13 +1371,21 @@ enforce_generic_type_consistency(Oid *actual_arg_types,
13701371
}
13711372
else if (!OidIsValid(elem_typeid))
13721373
{
1373-
/* Only way to get here is if all the generic args are UNKNOWN */
1374-
ereport(ERROR,
1375-
(errcode(ERRCODE_DATATYPE_MISMATCH),
1376-
errmsg("could not determine polymorphic type because input has type \"unknown\"")));
1374+
if (allow_poly)
1375+
{
1376+
array_typeid = ANYARRAYOID;
1377+
elem_typeid = ANYELEMENTOID;
1378+
}
1379+
else
1380+
{
1381+
/* Only way to get here is if all the generic args are UNKNOWN */
1382+
ereport(ERROR,
1383+
(errcode(ERRCODE_DATATYPE_MISMATCH),
1384+
errmsg("could not determine polymorphic type because input has type \"unknown\"")));
1385+
}
13771386
}
13781387

1379-
if (have_anynonarray)
1388+
if (have_anynonarray && elem_typeid != ANYELEMENTOID)
13801389
{
13811390
/* require the element type to not be an array */
13821391
if (type_is_array(elem_typeid))
@@ -1386,7 +1395,7 @@ enforce_generic_type_consistency(Oid *actual_arg_types,
13861395
format_type_be(elem_typeid))));
13871396
}
13881397

1389-
if (have_anyenum)
1398+
if (have_anyenum && elem_typeid != ANYELEMENTOID)
13901399
{
13911400
/* require the element type to be an enum */
13921401
if (!type_is_enum(elem_typeid))

‎src/backend/parser/parse_func.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.200 2008/01/01 19:45:51 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.201 2008/01/11 18:39:41 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -235,7 +235,8 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
235235
rettype = enforce_generic_type_consistency(actual_arg_types,
236236
declared_arg_types,
237237
nargs,
238-
rettype);
238+
rettype,
239+
false);
239240

240241
/* perform the necessary typecasting of arguments */
241242
make_fn_arguments(pstate, fargs, actual_arg_types, declared_arg_types);

‎src/backend/parser/parse_oper.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/parser/parse_oper.c,v 1.100 2008/01/01 19:45:51 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/parser/parse_oper.c,v 1.101 2008/01/11 18:39:41 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -1006,7 +1006,8 @@ make_scalar_array_op(ParseState *pstate, List *opname,
10061006
rettype = enforce_generic_type_consistency(actual_arg_types,
10071007
declared_arg_types,
10081008
2,
1009-
opform->oprresult);
1009+
opform->oprresult,
1010+
false);
10101011

10111012
/*
10121013
* Check that operator result is boolean
@@ -1116,7 +1117,8 @@ make_op_expr(ParseState *pstate, Operator op,
11161117
rettype = enforce_generic_type_consistency(actual_arg_types,
11171118
declared_arg_types,
11181119
nargs,
1119-
opform->oprresult);
1120+
opform->oprresult,
1121+
false);
11201122

11211123
/* perform the necessary typecasting of arguments */
11221124
make_fn_arguments(pstate, args, actual_arg_types, declared_arg_types);

‎src/include/parser/parse_coerce.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/parser/parse_coerce.h,v 1.74 2008/01/01 19:45:58 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/parser/parse_coerce.h,v 1.75 2008/01/11 18:39:41 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -81,7 +81,8 @@ extern bool check_generic_type_consistency(Oid *actual_arg_types,
8181
extern Oid enforce_generic_type_consistency(Oid *actual_arg_types,
8282
Oid *declared_arg_types,
8383
int nargs,
84-
Oid rettype);
84+
Oid rettype,
85+
bool allow_poly);
8586
extern Oid resolve_generic_type(Oid declared_type,
8687
Oid context_actual_type,
8788
Oid context_declared_type);

‎src/test/regress/expected/polymorphism.out

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,3 +577,39 @@ select q2, sql_if(q2 > 0, q2, q2 + 1) from int8_tbl;
577577
-4567890123456789 | -4567890123456788
578578
(5 rows)
579579

580+
-- another kind of polymorphic aggregate
581+
create function add_group(grp anyarray, ad anyelement, size integer)
582+
returns anyarray
583+
as $$
584+
begin
585+
if grp is null then
586+
return array[ad];
587+
end if;
588+
if array_upper(grp, 1) < size then
589+
return grp || ad;
590+
end if;
591+
return grp;
592+
end;
593+
$$
594+
language plpgsql immutable;
595+
create aggregate build_group(anyelement, integer) (
596+
SFUNC = add_group,
597+
STYPE = anyarray
598+
);
599+
select build_group(q1,3) from int8_tbl;
600+
build_group
601+
----------------------------
602+
{123,123,4567890123456789}
603+
(1 row)
604+
605+
-- this should fail because stype isn't compatible with arg
606+
create aggregate build_group(int8, integer) (
607+
SFUNC = add_group,
608+
STYPE = int2[]
609+
);
610+
ERROR: function add_group(smallint[], bigint, integer) does not exist
611+
-- but we can make a non-poly agg from a poly sfunc if types are OK
612+
create aggregate build_group(int8, integer) (
613+
SFUNC = add_group,
614+
STYPE = int8[]
615+
);

‎src/test/regress/sql/polymorphism.sql

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,3 +390,39 @@ select case when $1 then $2 else $3 end $$ language sql;
390390
select f1, sql_if(f1 > 0, bleat(f1), bleat(f1 + 1)) from int4_tbl;
391391

392392
select q2, sql_if(q2 > 0, q2, q2 + 1) from int8_tbl;
393+
394+
-- another kind of polymorphic aggregate
395+
396+
create function add_group(grp anyarray, ad anyelement, size integer)
397+
returns anyarray
398+
as $$
399+
begin
400+
if grp is null then
401+
return array[ad];
402+
end if;
403+
if array_upper(grp, 1) < size then
404+
return grp || ad;
405+
end if;
406+
return grp;
407+
end;
408+
$$
409+
language plpgsql immutable;
410+
411+
create aggregate build_group(anyelement, integer) (
412+
SFUNC = add_group,
413+
STYPE = anyarray
414+
);
415+
416+
select build_group(q1,3) from int8_tbl;
417+
418+
-- this should fail because stype isn't compatible with arg
419+
create aggregate build_group(int8, integer) (
420+
SFUNC = add_group,
421+
STYPE = int2[]
422+
);
423+
424+
-- but we can make a non-poly agg from a poly sfunc if types are OK
425+
create aggregate build_group(int8, integer) (
426+
SFUNC = add_group,
427+
STYPE = int8[]
428+
);

0 commit comments

Comments
 (0)