diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index fd3c71e764..1ed5d8c07b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -280,7 +280,7 @@ static TupleTableSlot *agg_retrieve_hash_table(AggState *aggstate); static Datum GetAggInitVal(Datum textInitVal, Oid transtype); static void build_pertrans_for_aggref(AggStatePerTrans pertrans, AggState *aggstate, EState *estate, - Aggref *aggref, Oid aggtransfn, Oid aggtranstype, + Aggref *aggref, Oid transfn_oid, Oid aggtranstype, Oid aggserialfn, Oid aggdeserialfn, Datum initValue, bool initValueIsNull, Oid *inputTypes, int numArguments); @@ -2522,8 +2522,8 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) int existing_aggno; int existing_transno; List *same_input_transnos; - Oid inputTypes[FUNC_MAX_ARGS]; - int numArguments; + Oid transFnInputTypes[FUNC_MAX_ARGS]; + int numTransFnArgs; int numDirectArgs; HeapTuple aggTuple; Form_pg_aggregate aggform; @@ -2701,14 +2701,14 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) * could be different from the agg's declared input types, when the * agg accepts ANY or a polymorphic type. */ - numArguments = get_aggregate_argtypes(aggref, inputTypes); + numTransFnArgs = get_aggregate_argtypes(aggref, transFnInputTypes); /* Count the "direct" arguments, if any */ numDirectArgs = list_length(aggref->aggdirectargs); /* Detect how many arguments to pass to the finalfn */ if (aggform->aggfinalextra) - peragg->numFinalArgs = numArguments + 1; + peragg->numFinalArgs = numTransFnArgs + 1; else peragg->numFinalArgs = numDirectArgs + 1; @@ -2722,7 +2722,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) */ if (OidIsValid(finalfn_oid)) { - build_aggregate_finalfn_expr(inputTypes, + build_aggregate_finalfn_expr(transFnInputTypes, peragg->numFinalArgs, aggtranstype, aggref->aggtype, @@ -2777,11 +2777,69 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) else { pertrans = &pertransstates[++transno]; - build_pertrans_for_aggref(pertrans, aggstate, estate, - aggref, transfn_oid, aggtranstype, - serialfn_oid, deserialfn_oid, - initValue, initValueIsNull, - inputTypes, numArguments); + + if (DO_AGGSPLIT_COMBINE(aggstate->aggsplit)) + { + Oid combineFnInputTypes[] = { aggtranstype, aggtranstype }; + + /* + * When combining there's only one input, the to-be-combined + * added transition value from below (this node's transition + * value is counted separately). + */ + pertrans->numTransInputs = 1; + + build_pertrans_for_aggref(pertrans, aggstate, estate, + aggref, transfn_oid, aggtranstype, + serialfn_oid, deserialfn_oid, + initValue, initValueIsNull, + combineFnInputTypes, 2); + + /* + * Ensure that a combine function to combine INTERNAL states + * is not strict. This should have been checked during CREATE + * AGGREGATE, but the strict property could have been changed + * since then. + */ + if (pertrans->transfn.fn_strict && aggtranstype == INTERNALOID) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("combine function with transition type %s must not be declared STRICT", + format_type_be(aggtranstype)))); + } + else + { + /* Detect how many arguments to pass to the transfn */ + if (AGGKIND_IS_ORDERED_SET(aggref->aggkind)) + pertrans->numTransInputs = list_length(aggref->args); + else + pertrans->numTransInputs = numTransFnArgs; + + build_pertrans_for_aggref(pertrans, aggstate, estate, + aggref, transfn_oid, aggtranstype, + serialfn_oid, deserialfn_oid, + initValue, initValueIsNull, + transFnInputTypes, numTransFnArgs); + + /* + * If the transfn is strict and the initval is NULL, make sure input + * type and transtype are the same (or at least binary-compatible), so + * that it's OK to use the first aggregated input value as the initial + * transValue. This should have been checked at agg definition time, + * but we must check again in case the transfn's strictness property + * has been changed. + */ + if (pertrans->transfn.fn_strict && pertrans->initValueIsNull) + { + if (numTransFnArgs <= numDirectArgs || + !IsBinaryCoercible(transFnInputTypes[numDirectArgs], + aggtranstype)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("aggregate %u needs to have compatible input type and transition type", + aggref->aggfnoid))); + } + } peragg->transno = transno; } ReleaseSysCache(aggTuple); @@ -2869,7 +2927,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) * Build the state needed to calculate a state value for an aggregate. * * This initializes all the fields in 'pertrans'. 'aggref' is the aggregate - * to initialize the state for. 'aggtransfn', 'aggtranstype', and the rest + * to initialize the state for. 'transfn_oid', 'aggtranstype', and the rest * of the arguments could be calculated from 'aggref', but the caller has * calculated them already, so might as well pass them. */ @@ -2877,12 +2935,14 @@ static void build_pertrans_for_aggref(AggStatePerTrans pertrans, AggState *aggstate, EState *estate, Aggref *aggref, - Oid aggtransfn, Oid aggtranstype, + Oid transfn_oid, Oid aggtranstype, Oid aggserialfn, Oid aggdeserialfn, Datum initValue, bool initValueIsNull, Oid *inputTypes, int numArguments) { int numGroupingSets = Max(aggstate->maxsets, 1); + Expr *transfnexpr; + size_t numTransArgs; Expr *serialfnexpr = NULL; Expr *deserialfnexpr = NULL; ListCell *lc; @@ -2897,7 +2957,7 @@ build_pertrans_for_aggref(AggStatePerTrans pertrans, pertrans->aggref = aggref; pertrans->aggshared = false; pertrans->aggCollation = aggref->inputcollid; - pertrans->transfn_oid = aggtransfn; + pertrans->transfn_oid = transfn_oid; pertrans->serialfn_oid = aggserialfn; pertrans->deserialfn_oid = aggdeserialfn; pertrans->initValue = initValue; @@ -2911,111 +2971,34 @@ build_pertrans_for_aggref(AggStatePerTrans pertrans, pertrans->aggtranstype = aggtranstype; + /* account for the current transition state */ + numTransArgs = pertrans->numTransInputs + 1; + /* - * When combining states, we have no use at all for the aggregate - * function's transfn. Instead we use the combinefn. In this case, the - * transfn and transfn_oid fields of pertrans refer to the combine - * function rather than the transition function. + * Set up infrastructure for calling the transfn. Note that invtrans + * is not needed here. */ - if (DO_AGGSPLIT_COMBINE(aggstate->aggsplit)) - { - Expr *combinefnexpr; - size_t numTransArgs; - - /* - * When combining there's only one input, the to-be-combined added - * transition value from below (this node's transition value is - * counted separately). - */ - pertrans->numTransInputs = 1; - - /* account for the current transition state */ - numTransArgs = pertrans->numTransInputs + 1; - - build_aggregate_combinefn_expr(aggtranstype, - aggref->inputcollid, - aggtransfn, - &combinefnexpr); - fmgr_info(aggtransfn, &pertrans->transfn); - fmgr_info_set_expr((Node *) combinefnexpr, &pertrans->transfn); - - pertrans->transfn_fcinfo = - (FunctionCallInfo) palloc(SizeForFunctionCallInfo(2)); - InitFunctionCallInfoData(*pertrans->transfn_fcinfo, - &pertrans->transfn, - numTransArgs, - pertrans->aggCollation, - (void *) aggstate, NULL); - - /* - * Ensure that a combine function to combine INTERNAL states is not - * strict. This should have been checked during CREATE AGGREGATE, but - * the strict property could have been changed since then. - */ - if (pertrans->transfn.fn_strict && aggtranstype == INTERNALOID) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("combine function with transition type %s must not be declared STRICT", - format_type_be(aggtranstype)))); - } - else - { - Expr *transfnexpr; - size_t numTransArgs; - - /* Detect how many arguments to pass to the transfn */ - if (AGGKIND_IS_ORDERED_SET(aggref->aggkind)) - pertrans->numTransInputs = numInputs; - else - pertrans->numTransInputs = numArguments; - - /* account for the current transition state */ - numTransArgs = pertrans->numTransInputs + 1; + build_aggregate_transfn_expr(inputTypes, + numArguments, + numDirectArgs, + aggref->aggvariadic, + aggtranstype, + aggref->inputcollid, + transfn_oid, + InvalidOid, + &transfnexpr, + NULL); + fmgr_info(transfn_oid, &pertrans->transfn); + fmgr_info_set_expr((Node *) transfnexpr, &pertrans->transfn); - /* - * Set up infrastructure for calling the transfn. Note that invtrans - * is not needed here. - */ - build_aggregate_transfn_expr(inputTypes, - numArguments, - numDirectArgs, - aggref->aggvariadic, - aggtranstype, - aggref->inputcollid, - aggtransfn, - InvalidOid, - &transfnexpr, - NULL); - fmgr_info(aggtransfn, &pertrans->transfn); - fmgr_info_set_expr((Node *) transfnexpr, &pertrans->transfn); - - pertrans->transfn_fcinfo = + pertrans->transfn_fcinfo = (FunctionCallInfo) palloc(SizeForFunctionCallInfo(numTransArgs)); - InitFunctionCallInfoData(*pertrans->transfn_fcinfo, - &pertrans->transfn, - numTransArgs, - pertrans->aggCollation, - (void *) aggstate, NULL); + InitFunctionCallInfoData(*pertrans->transfn_fcinfo, + &pertrans->transfn, + numTransArgs, + pertrans->aggCollation, + (void *) aggstate, NULL); - /* - * If the transfn is strict and the initval is NULL, make sure input - * type and transtype are the same (or at least binary-compatible), so - * that it's OK to use the first aggregated input value as the initial - * transValue. This should have been checked at agg definition time, - * but we must check again in case the transfn's strictness property - * has been changed. - */ - if (pertrans->transfn.fn_strict && pertrans->initValueIsNull) - { - if (numArguments <= numDirectArgs || - !IsBinaryCoercible(inputTypes[numDirectArgs], - aggtranstype)) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("aggregate %u needs to have compatible input type and transition type", - aggref->aggfnoid))); - } - } /* get info about the state value's datatype */ get_typlenbyval(aggtranstype, @@ -3106,6 +3089,9 @@ build_pertrans_for_aggref(AggStatePerTrans pertrans, */ Assert(aggstate->aggstrategy != AGG_HASHED && aggstate->aggstrategy != AGG_MIXED); + /* ORDER BY aggregates are not supported with partial aggregation */ + Assert(!DO_AGGSPLIT_COMBINE(aggstate->aggsplit)); + /* If we have only one input, we need its len/byval info. */ if (numInputs == 1) { diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c index c745fcdd2b..821ee13e97 100644 --- a/src/backend/parser/parse_agg.c +++ b/src/backend/parser/parse_agg.c @@ -1975,35 +1975,6 @@ build_aggregate_transfn_expr(Oid *agg_input_types, } } -/* - * Like build_aggregate_transfn_expr, but creates an expression tree for the - * combine function of an aggregate, rather than the transition function. - */ -void -build_aggregate_combinefn_expr(Oid agg_state_type, - Oid agg_input_collation, - Oid combinefn_oid, - Expr **combinefnexpr) -{ - Node *argp; - List *args; - FuncExpr *fexpr; - - /* combinefn takes two arguments of the aggregate state type */ - argp = make_agg_arg(agg_state_type, agg_input_collation); - - args = list_make2(argp, argp); - - fexpr = makeFuncExpr(combinefn_oid, - agg_state_type, - args, - InvalidOid, - agg_input_collation, - COERCE_EXPLICIT_CALL); - /* combinefn is currently never treated as variadic */ - *combinefnexpr = (Expr *) fexpr; -} - /* * Like build_aggregate_transfn_expr, but creates an expression tree for the * serialization function of an aggregate. diff --git a/src/include/parser/parse_agg.h b/src/include/parser/parse_agg.h index 48fe9d7140..7d47624f2b 100644 --- a/src/include/parser/parse_agg.h +++ b/src/include/parser/parse_agg.h @@ -46,11 +46,6 @@ extern void build_aggregate_transfn_expr(Oid *agg_input_types, Expr **transfnexpr, Expr **invtransfnexpr); -extern void build_aggregate_combinefn_expr(Oid agg_state_type, - Oid agg_input_collation, - Oid combinefn_oid, - Expr **combinefnexpr); - extern void build_aggregate_serialfn_expr(Oid serialfn_oid, Expr **serialfnexpr);