pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Fix issue with ORDER BY / DISTINCT aggregates and FILTER 3+ messages / 1 participants [nested] [flat]
* pgsql: Fix issue with ORDER BY / DISTINCT aggregates and FILTER @ 2025-04-20 10:12 David Rowley <drowley@postgresql.org> 0 siblings, 0 replies; 3+ messages in thread From: David Rowley @ 2025-04-20 10:12 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix issue with ORDER BY / DISTINCT aggregates and FILTER 1349d2790 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 1349d2790, 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 1349d2790 due to presorted results coming in from an Index Scan. Backpatch to 16, where 1349d2790 was introduced Author: David Rowley <dgrowleyml@gmail.com> Reported-by: Kaimeh <kkaimeh@gmail.com> Diagnosed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAK-%2BJz9J%3DQ06-M7cDJoPNeYbz5EZDqkjQbJnmRyQyzkbRGsYkA%40mail.gmail.com Backpatch-through: 16 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/d47f922246b54f0290811951e0b73a3d6110437c Modified Files -------------- src/backend/optimizer/plan/planner.c | 51 +++++++++++++++++++++++++++++--- src/test/regress/expected/aggregates.out | 37 +++++++++++++++++++++++ src/test/regress/expected/sqljson.out | 24 +++++++-------- src/test/regress/sql/aggregates.sql | 19 ++++++++++++ 4 files changed, 115 insertions(+), 16 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Fix issue with ORDER BY / DISTINCT aggregates and FILTER @ 2025-04-20 10:12 David Rowley <drowley@postgresql.org> 0 siblings, 0 replies; 3+ messages in thread From: David Rowley @ 2025-04-20 10:12 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix issue with ORDER BY / DISTINCT aggregates and FILTER 1349d2790 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 1349d2790, 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 1349d2790 due to presorted results coming in from an Index Scan. Backpatch to 16, where 1349d2790 was introduced Author: David Rowley <dgrowleyml@gmail.com> Reported-by: Kaimeh <kkaimeh@gmail.com> Diagnosed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAK-%2BJz9J%3DQ06-M7cDJoPNeYbz5EZDqkjQbJnmRyQyzkbRGsYkA%40mail.gmail.com Backpatch-through: 16 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/065ce49a10e0722df809935504bc7814e94a04dc Modified Files -------------- src/backend/optimizer/plan/planner.c | 51 +++++++++++++++++++++++++++++--- src/test/regress/expected/aggregates.out | 37 +++++++++++++++++++++++ src/test/regress/expected/sqljson.out | 24 +++++++-------- src/test/regress/sql/aggregates.sql | 19 ++++++++++++ 4 files changed, 115 insertions(+), 16 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Fix issue with ORDER BY / DISTINCT aggregates and FILTER @ 2025-04-20 10:13 David Rowley <drowley@postgresql.org> 0 siblings, 0 replies; 3+ messages in thread From: David Rowley @ 2025-04-20 10:13 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix issue with ORDER BY / DISTINCT aggregates and FILTER 1349d2790 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 1349d2790, 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 1349d2790 due to presorted results coming in from an Index Scan. Backpatch to 16, where 1349d2790 was introduced Author: David Rowley <dgrowleyml@gmail.com> Reported-by: Kaimeh <kkaimeh@gmail.com> Diagnosed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAK-%2BJz9J%3DQ06-M7cDJoPNeYbz5EZDqkjQbJnmRyQyzkbRGsYkA%40mail.gmail.com Backpatch-through: 16 Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/887a23237150be514df30b11084e6738ed5f93e3 Modified Files -------------- src/backend/optimizer/plan/planner.c | 51 +++++++++++++++++++++++++++++--- src/test/regress/expected/aggregates.out | 37 +++++++++++++++++++++++ src/test/regress/expected/sqljson.out | 24 +++++++-------- src/test/regress/sql/aggregates.sql | 19 ++++++++++++ 4 files changed, 115 insertions(+), 16 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2025-04-20 10:13 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-04-20 10:12 pgsql: Fix issue with ORDER BY / DISTINCT aggregates and FILTER David Rowley <drowley@postgresql.org> 2025-04-20 10:12 pgsql: Fix issue with ORDER BY / DISTINCT aggregates and FILTER David Rowley <drowley@postgresql.org> 2025-04-20 10:13 pgsql: Fix issue with ORDER BY / DISTINCT aggregates and FILTER David Rowley <drowley@postgresql.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox