Skip to content
  • David Rowley's avatar
    065ce49a
    Fix issue with ORDER BY / DISTINCT aggregates and FILTER · 065ce49a
    David Rowley authored
    
    
    1349d279 added support so that aggregate functions with an ORDER BY or
    DISTINCT clause could make use of presorted inputs to avoid an implicit
    sort within nodeAgg.c.  That commit failed to consider that a FILTER
    clause may exist that filters rows before the aggregate function
    arguments are evaluated.  That can be problematic if an aggregate
    argument contains an expression which could error out during evaluation.
    It's perfectly valid to want to have a FILTER clause which eliminates
    such values, and with the pre-sorted path added in 1349d279, it was
    possible that the planner would produce a plan with a Sort node above
    the Aggregate to perform the sort on the aggregate's arguments long before
    the Aggregate node would filter out the non-matching values.
    
    Here we fix this by inspecting ORDER BY / DISTINCT aggregate functions
    which have a FILTER clause to see if the aggregate's arguments are
    anything more complex than a Var or a Const.  Evaluating these isn't
    going to cause an error.  If we find any non-Var, non-Const parameters
    then the planner will now opt to perform the sort in the Aggregate node
    for these aggregates, i.e. disable the presorted aggregate optimization.
    
    An alternative fix would have been to completely disallow the presorted
    optimization for Aggrefs with any FILTER clause, but that wasn't done as
    that could cause large performance regressions for queries that see
    significant gains from 1349d279 due to presorted results coming in from
    an Index Scan.
    
    Backpatch to 16, where 1349d279 was introduced
    
    Author: David Rowley <dgrowleyml@gmail.com>
    Reported-by: default avatarKaimeh <kkaimeh@gmail.com>
    Diagnosed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
    Discussion: https://postgr.es/m/CAK-%2BJz9J%3DQ06-M7cDJoPNeYbz5EZDqkjQbJnmRyQyzkbRGsYkA%40mail.gmail.com
    Backpatch-through: 16
    065ce49a
    Fix issue with ORDER BY / DISTINCT aggregates and FILTER
    David Rowley authored
    
    
    1349d279 added support so that aggregate functions with an ORDER BY or
    DISTINCT clause could make use of presorted inputs to avoid an implicit
    sort within nodeAgg.c.  That commit failed to consider that a FILTER
    clause may exist that filters rows before the aggregate function
    arguments are evaluated.  That can be problematic if an aggregate
    argument contains an expression which could error out during evaluation.
    It's perfectly valid to want to have a FILTER clause which eliminates
    such values, and with the pre-sorted path added in 1349d279, it was
    possible that the planner would produce a plan with a Sort node above
    the Aggregate to perform the sort on the aggregate's arguments long before
    the Aggregate node would filter out the non-matching values.
    
    Here we fix this by inspecting ORDER BY / DISTINCT aggregate functions
    which have a FILTER clause to see if the aggregate's arguments are
    anything more complex than a Var or a Const.  Evaluating these isn't
    going to cause an error.  If we find any non-Var, non-Const parameters
    then the planner will now opt to perform the sort in the Aggregate node
    for these aggregates, i.e. disable the presorted aggregate optimization.
    
    An alternative fix would have been to completely disallow the presorted
    optimization for Aggrefs with any FILTER clause, but that wasn't done as
    that could cause large performance regressions for queries that see
    significant gains from 1349d279 due to presorted results coming in from
    an Index Scan.
    
    Backpatch to 16, where 1349d279 was introduced
    
    Author: David Rowley <dgrowleyml@gmail.com>
    Reported-by: default avatarKaimeh <kkaimeh@gmail.com>
    Diagnosed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
    Discussion: https://postgr.es/m/CAK-%2BJz9J%3DQ06-M7cDJoPNeYbz5EZDqkjQbJnmRyQyzkbRGsYkA%40mail.gmail.com
    Backpatch-through: 16
Loading