pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Fix possible issue of a WindowFunc being in the wrong WindowClau 3+ messages / 1 participants [nested] [flat]
* pgsql: Fix possible issue of a WindowFunc being in the wrong WindowClau @ 2026-01-26 10:46 David Rowley <drowley@postgresql.org> 0 siblings, 0 replies; 3+ messages in thread From: David Rowley @ 2026-01-26 10:46 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix possible issue of a WindowFunc being in the wrong WindowClause ed1a88dda made it so WindowClauses can be merged when all window functions belonging to the WindowClause can equally well use some other WindowClause without any behavioral changes. When that optimization applies, the WindowFunc's "winref" gets adjusted to reference the new WindowClause. That commit does not work well with the deduplication logic in find_window_functions(), which only added the WindowFunc to the list when there wasn't already an identical WindowFunc in the list. That deduplication logic meant that the duplicate WindowFunc wouldn't get the "winref" changed when optimize_window_clauses() was able to swap the WindowFunc to another WindowClause. This could lead to the following error in the unlikely event that the deduplication code did something and the duplicate WindowFunc happened to be moved into another WindowClause. ERROR: WindowFunc with winref 2 assigned to WindowAgg with winref 1 As it turns out, the deduplication logic in find_window_functions() is pretty bogus. It might have done something when added, as that code predates b8d7f053c, which changed how projections work. As it turns out, at least now we *will* evaluate the duplicate WindowFuncs. All that the deduplication code seems to do today is assist in underestimating the WindowAggPath costs due to not counting the evaluation costs of duplicate WindowFuncs. Ideally the fix would be to remove the deduplication code, but that could result in changes to the plan costs, as duplicate WindowFuncs would then be costed. Instead, let's play it safe and shift the deduplication code so it runs after the other processing in optimize_window_clauses(). Backpatch only as far as v16 as there doesn't seem to be any other harm done by the WindowFunc deduplication code before then. This issue was fixed in master by 7027dd499. Reported-by: Meng Zhang <mza117jc@gmail.com> Author: Meng Zhang <mza117jc@gmail.com> Author: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/CAErYLFAuxmW0UVdgrz7iiuNrxGQnFK_OP9hBD5CUzRgjrVrz=Q@mail.gmail.com Backpatch-through: 16 Branch ------ REL_18_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/ccde5be6869c13ad88259d300c684900d7c1eb8c Modified Files -------------- src/backend/optimizer/plan/planner.c | 35 +++++++++++++++++++++++++++++++++++ src/backend/optimizer/util/clauses.c | 11 ++++------- 2 files changed, 39 insertions(+), 7 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Fix possible issue of a WindowFunc being in the wrong WindowClau @ 2026-01-26 10:47 David Rowley <drowley@postgresql.org> 0 siblings, 0 replies; 3+ messages in thread From: David Rowley @ 2026-01-26 10:47 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix possible issue of a WindowFunc being in the wrong WindowClause ed1a88dda made it so WindowClauses can be merged when all window functions belonging to the WindowClause can equally well use some other WindowClause without any behavioral changes. When that optimization applies, the WindowFunc's "winref" gets adjusted to reference the new WindowClause. That commit does not work well with the deduplication logic in find_window_functions(), which only added the WindowFunc to the list when there wasn't already an identical WindowFunc in the list. That deduplication logic meant that the duplicate WindowFunc wouldn't get the "winref" changed when optimize_window_clauses() was able to swap the WindowFunc to another WindowClause. This could lead to the following error in the unlikely event that the deduplication code did something and the duplicate WindowFunc happened to be moved into another WindowClause. ERROR: WindowFunc with winref 2 assigned to WindowAgg with winref 1 As it turns out, the deduplication logic in find_window_functions() is pretty bogus. It might have done something when added, as that code predates b8d7f053c, which changed how projections work. As it turns out, at least now we *will* evaluate the duplicate WindowFuncs. All that the deduplication code seems to do today is assist in underestimating the WindowAggPath costs due to not counting the evaluation costs of duplicate WindowFuncs. Ideally the fix would be to remove the deduplication code, but that could result in changes to the plan costs, as duplicate WindowFuncs would then be costed. Instead, let's play it safe and shift the deduplication code so it runs after the other processing in optimize_window_clauses(). Backpatch only as far as v16 as there doesn't seem to be any other harm done by the WindowFunc deduplication code before then. This issue was fixed in master by 7027dd499. Reported-by: Meng Zhang <mza117jc@gmail.com> Author: Meng Zhang <mza117jc@gmail.com> Author: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/CAErYLFAuxmW0UVdgrz7iiuNrxGQnFK_OP9hBD5CUzRgjrVrz=Q@mail.gmail.com Backpatch-through: 16 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/cae8127411670becc195f46af723025ddd6e135d Modified Files -------------- src/backend/optimizer/plan/planner.c | 35 +++++++++++++++++++++++++++++++++++ src/backend/optimizer/util/clauses.c | 11 ++++------- 2 files changed, 39 insertions(+), 7 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Fix possible issue of a WindowFunc being in the wrong WindowClau @ 2026-01-26 10:48 David Rowley <drowley@postgresql.org> 0 siblings, 0 replies; 3+ messages in thread From: David Rowley @ 2026-01-26 10:48 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix possible issue of a WindowFunc being in the wrong WindowClause ed1a88dda made it so WindowClauses can be merged when all window functions belonging to the WindowClause can equally well use some other WindowClause without any behavioral changes. When that optimization applies, the WindowFunc's "winref" gets adjusted to reference the new WindowClause. That commit does not work well with the deduplication logic in find_window_functions(), which only added the WindowFunc to the list when there wasn't already an identical WindowFunc in the list. That deduplication logic meant that the duplicate WindowFunc wouldn't get the "winref" changed when optimize_window_clauses() was able to swap the WindowFunc to another WindowClause. This could lead to the following error in the unlikely event that the deduplication code did something and the duplicate WindowFunc happened to be moved into another WindowClause. ERROR: WindowFunc with winref 2 assigned to WindowAgg with winref 1 As it turns out, the deduplication logic in find_window_functions() is pretty bogus. It might have done something when added, as that code predates b8d7f053c, which changed how projections work. As it turns out, at least now we *will* evaluate the duplicate WindowFuncs. All that the deduplication code seems to do today is assist in underestimating the WindowAggPath costs due to not counting the evaluation costs of duplicate WindowFuncs. Ideally the fix would be to remove the deduplication code, but that could result in changes to the plan costs, as duplicate WindowFuncs would then be costed. Instead, let's play it safe and shift the deduplication code so it runs after the other processing in optimize_window_clauses(). Backpatch only as far as v16 as there doesn't seem to be any other harm done by the WindowFunc deduplication code before then. This issue was fixed in master by 7027dd499. Reported-by: Meng Zhang <mza117jc@gmail.com> Author: Meng Zhang <mza117jc@gmail.com> Author: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/CAErYLFAuxmW0UVdgrz7iiuNrxGQnFK_OP9hBD5CUzRgjrVrz=Q@mail.gmail.com Backpatch-through: 16 Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/4297a35196de281e551d500fc71c7033f253cb98 Modified Files -------------- src/backend/optimizer/plan/planner.c | 35 +++++++++++++++++++++++++++++++++++ src/backend/optimizer/util/clauses.c | 11 ++++------- 2 files changed, 39 insertions(+), 7 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-01-26 10:48 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-01-26 10:46 pgsql: Fix possible issue of a WindowFunc being in the wrong WindowClau David Rowley <drowley@postgresql.org> 2026-01-26 10:47 pgsql: Fix possible issue of a WindowFunc being in the wrong WindowClau David Rowley <drowley@postgresql.org> 2026-01-26 10:48 pgsql: Fix possible issue of a WindowFunc being in the wrong WindowClau 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