• Tom Lane's avatar
    Fix handling of argument and result datatypes for partial aggregation. · 915b703e
    Tom Lane authored
    When doing partial aggregation, the args list of the upper (combining)
    Aggref node is replaced by a Var representing the output of the partial
    aggregation steps, which has either the aggregate's transition data type
    or a serialized representation of that.  However, nodeAgg.c blindly
    continued to use the args list as an indication of the user-level argument
    types.  This broke resolution of polymorphic transition datatypes at
    executor startup (though it accidentally failed to fail for the ANYARRAY
    case, which is likely the only one anyone had tested).  Moreover, the
    constructed FuncExpr passed to the finalfunc contained completely wrong
    information, which would have led to bogus answers or crashes for any case
    where the finalfunc examined that information (which is only likely to be
    with polymorphic aggregates using a non-polymorphic transition type).
    
    As an independent bug, apply_partialaggref_adjustment neglected to resolve
    a polymorphic transition datatype before assigning it as the output type
    of the lower-level Aggref node.  This again accidentally failed to fail
    for ANYARRAY but would be unlikely to work in other cases.
    
    To fix the first problem, record the user-level argument types in a
    separate OID-list field of Aggref, and look to that rather than the args
    list when asking what the argument types were.  (It turns out to be
    convenient to include any "direct" arguments in this list too, although
    those are not currently subject to being overwritten.)
    
    Rather than adding yet another resolve_aggregate_transtype() call to fix
    the second problem, add an aggtranstype field to Aggref, and store the
    resolved transition type OID there when the planner first computes it.
    (By doing this in the planner and not the parser, we can allow the
    aggregate's transition type to change from time to time, although no DDL
    support yet exists for that.)  This saves nothing of consequence for
    simple non-polymorphic aggregates, but for polymorphic transition types
    we save a catalog lookup during executor startup as well as several
    planner lookups that are new in 9.6 due to parallel query planning.
    
    In passing, fix an error that was introduced into count_agg_clauses_walker
    some time ago: it was applying exprTypmod() to something that wasn't an
    expression node at all, but a TargetEntry.  exprTypmod silently returned
    -1 so that there was not an obvious failure, but this broke the intended
    sensitivity of aggregate space consumption estimates to the typmod of
    varchar and similar data types.  This part needs to be back-patched.
    
    Catversion bump due to change of stored Aggref nodes.
    
    Discussion: <8229.1466109074@sss.pgh.pa.us>
    915b703e
parse_func.c 62.7 KB