agora inbox for pgsql-committers@postgresql.orghelp / color / mirror / Atom feed
pgsql: Fix qual pushdown past grouping with mismatched equivalence 3+ messages / 1 participants [nested] [flat]
* pgsql: Fix qual pushdown past grouping with mismatched equivalence @ 2026-07-06 07:16 Richard Guo <rguo@postgresql.org> 0 siblings, 0 replies; 3+ messages in thread From: Richard Guo @ 2026-07-06 07:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix qual pushdown past grouping with mismatched equivalence The planner has two optimizations that move a qual clause across a grouping boundary: subquery_planner transfers HAVING clauses to WHERE so they can be evaluated before aggregation, and qual_is_pushdown_safe pushes outer restriction clauses into a subquery past its DISTINCT, DISTINCT ON, window PARTITION BY, or set-operation grouping layer. Both produce wrong results when the moved clause's equivalence relation disagrees with the grouping's, since the clause then filters rows the grouping would have merged. The disagreement has two forms. A type may belong to multiple btree opfamilies whose equality operators disagree (e.g. record_ops vs record_image_ops); or the grouping may use a nondeterministic collation, where comparing the column under a different collation, or wrapping it in a function or operator, can distinguish values the collation considers equal. Because we cannot prove an arbitrary expression preserves that equality, a grouping column with a nondeterministic collation is safe to push only as a direct operand of a comparison under its own collation. Fix both call sites through a shared walker parameterized by a callback that maps each Var to the grouping equality operator for its column (or InvalidOid for non-grouping Vars). For HAVING, the callback recovers the SortGroupClause's eqop via the GROUP Var's varattno, which requires running before flatten_group_exprs while havingQual still contains GROUP Vars. For subquery pushdown, the callback recovers the eqop from subquery->distinctClause, a window's partitionClause, or any grouping node in the SetOperationStmt tree. The walker fires only when there is an equivalence boundary to cross, gated by either the existing UNSAFE_NOTIN_DISTINCTON_CLAUSE and UNSAFE_NOTIN_PARTITIONBY_CLAUSE flags or by a recursive check for any grouping node in the set-op tree. Back-patch to v18 only. The HAVING half relies on the RTE_GROUP mechanism introduced in v18 (commit 247dea89f), which is what lets us identify grouping expressions via GROUP Vars on pre-flatten havingQual. Pre-v18 branches lack that machinery, so a back-patch there would need a different approach. Given the absence of field reports of these bugs on back branches, the risk of carrying a different fix on stable branches is not justified. Author: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Thom Brown <thom@linux.com> Reviewed-by: Florin Irion <irionr@gmail.com> Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Reviewed-by: Chengpeng Yan <chengpeng_yan@outlook.com> Discussion: https://postgr.es/m/CAMbWs4-QLZpn3UVOpeG2fOxxhdnkDNMZ_3Zcm3dqJwRAphz68g@mail.gmail.com Backpatch-through: 18 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/44fb59fc605ea0eabd58029bb8dc2c2416c42c3c Modified Files -------------- src/backend/optimizer/path/allpaths.c | 179 ++++++++++++++++ src/backend/optimizer/plan/planner.c | 263 +++++++---------------- src/backend/optimizer/util/clauses.c | 268 +++++++++++++++++++++++ src/backend/utils/cache/lsyscache.c | 24 ++- src/include/optimizer/clauses.h | 14 ++ src/test/regress/expected/aggregates.out | 80 ++++++- src/test/regress/expected/collate.icu.utf8.out | 286 +++++++++++++++++++++++-- src/test/regress/expected/subselect.out | 214 ++++++++++++++++++ src/test/regress/sql/aggregates.sql | 46 +++- src/test/regress/sql/collate.icu.utf8.sql | 129 ++++++++++- src/test/regress/sql/subselect.sql | 105 +++++++++ src/tools/pgindent/typedefs.list | 4 +- 12 files changed, 1370 insertions(+), 242 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Fix qual pushdown past grouping with mismatched equivalence @ 2026-07-06 07:16 Richard Guo <rguo@postgresql.org> 0 siblings, 0 replies; 3+ messages in thread From: Richard Guo @ 2026-07-06 07:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix qual pushdown past grouping with mismatched equivalence The planner has two optimizations that move a qual clause across a grouping boundary: subquery_planner transfers HAVING clauses to WHERE so they can be evaluated before aggregation, and qual_is_pushdown_safe pushes outer restriction clauses into a subquery past its DISTINCT, DISTINCT ON, window PARTITION BY, or set-operation grouping layer. Both produce wrong results when the moved clause's equivalence relation disagrees with the grouping's, since the clause then filters rows the grouping would have merged. The disagreement has two forms. A type may belong to multiple btree opfamilies whose equality operators disagree (e.g. record_ops vs record_image_ops); or the grouping may use a nondeterministic collation, where comparing the column under a different collation, or wrapping it in a function or operator, can distinguish values the collation considers equal. Because we cannot prove an arbitrary expression preserves that equality, a grouping column with a nondeterministic collation is safe to push only as a direct operand of a comparison under its own collation. Fix both call sites through a shared walker parameterized by a callback that maps each Var to the grouping equality operator for its column (or InvalidOid for non-grouping Vars). For HAVING, the callback recovers the SortGroupClause's eqop via the GROUP Var's varattno, which requires running before flatten_group_exprs while havingQual still contains GROUP Vars. For subquery pushdown, the callback recovers the eqop from subquery->distinctClause, a window's partitionClause, or any grouping node in the SetOperationStmt tree. The walker fires only when there is an equivalence boundary to cross, gated by either the existing UNSAFE_NOTIN_DISTINCTON_CLAUSE and UNSAFE_NOTIN_PARTITIONBY_CLAUSE flags or by a recursive check for any grouping node in the set-op tree. Back-patch to v18 only. The HAVING half relies on the RTE_GROUP mechanism introduced in v18 (commit 247dea89f), which is what lets us identify grouping expressions via GROUP Vars on pre-flatten havingQual. Pre-v18 branches lack that machinery, so a back-patch there would need a different approach. Given the absence of field reports of these bugs on back branches, the risk of carrying a different fix on stable branches is not justified. Author: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Thom Brown <thom@linux.com> Reviewed-by: Florin Irion <irionr@gmail.com> Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Reviewed-by: Chengpeng Yan <chengpeng_yan@outlook.com> Discussion: https://postgr.es/m/CAMbWs4-QLZpn3UVOpeG2fOxxhdnkDNMZ_3Zcm3dqJwRAphz68g@mail.gmail.com Backpatch-through: 18 Branch ------ REL_19_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/98d5d7ee6419dc4b3894d7b4610a2e128f675052 Modified Files -------------- src/backend/optimizer/path/allpaths.c | 179 ++++++++++++++++ src/backend/optimizer/plan/planner.c | 263 +++++++---------------- src/backend/optimizer/util/clauses.c | 268 +++++++++++++++++++++++ src/backend/utils/cache/lsyscache.c | 24 ++- src/include/optimizer/clauses.h | 14 ++ src/test/regress/expected/aggregates.out | 80 ++++++- src/test/regress/expected/collate.icu.utf8.out | 286 +++++++++++++++++++++++-- src/test/regress/expected/subselect.out | 214 ++++++++++++++++++ src/test/regress/sql/aggregates.sql | 46 +++- src/test/regress/sql/collate.icu.utf8.sql | 129 ++++++++++- src/test/regress/sql/subselect.sql | 105 +++++++++ src/tools/pgindent/typedefs.list | 4 +- 12 files changed, 1370 insertions(+), 242 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Fix qual pushdown past grouping with mismatched equivalence @ 2026-07-06 07:16 Richard Guo <rguo@postgresql.org> 0 siblings, 0 replies; 3+ messages in thread From: Richard Guo @ 2026-07-06 07:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix qual pushdown past grouping with mismatched equivalence The planner has two optimizations that move a qual clause across a grouping boundary: subquery_planner transfers HAVING clauses to WHERE so they can be evaluated before aggregation, and qual_is_pushdown_safe pushes outer restriction clauses into a subquery past its DISTINCT, DISTINCT ON, window PARTITION BY, or set-operation grouping layer. Both produce wrong results when the moved clause's equivalence relation disagrees with the grouping's, since the clause then filters rows the grouping would have merged. The disagreement has two forms. A type may belong to multiple btree opfamilies whose equality operators disagree (e.g. record_ops vs record_image_ops); or the grouping may use a nondeterministic collation, where comparing the column under a different collation, or wrapping it in a function or operator, can distinguish values the collation considers equal. Because we cannot prove an arbitrary expression preserves that equality, a grouping column with a nondeterministic collation is safe to push only as a direct operand of a comparison under its own collation. Fix both call sites through a shared walker parameterized by a callback that maps each Var to the grouping equality operator for its column (or InvalidOid for non-grouping Vars). For HAVING, the callback recovers the SortGroupClause's eqop via the GROUP Var's varattno, which requires running before flatten_group_exprs while havingQual still contains GROUP Vars. For subquery pushdown, the callback recovers the eqop from subquery->distinctClause, a window's partitionClause, or any grouping node in the SetOperationStmt tree. The walker fires only when there is an equivalence boundary to cross, gated by either the existing UNSAFE_NOTIN_DISTINCTON_CLAUSE and UNSAFE_NOTIN_PARTITIONBY_CLAUSE flags or by a recursive check for any grouping node in the set-op tree. Back-patch to v18 only. The HAVING half relies on the RTE_GROUP mechanism introduced in v18 (commit 247dea89f), which is what lets us identify grouping expressions via GROUP Vars on pre-flatten havingQual. Pre-v18 branches lack that machinery, so a back-patch there would need a different approach. Given the absence of field reports of these bugs on back branches, the risk of carrying a different fix on stable branches is not justified. Author: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Thom Brown <thom@linux.com> Reviewed-by: Florin Irion <irionr@gmail.com> Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Reviewed-by: Chengpeng Yan <chengpeng_yan@outlook.com> Discussion: https://postgr.es/m/CAMbWs4-QLZpn3UVOpeG2fOxxhdnkDNMZ_3Zcm3dqJwRAphz68g@mail.gmail.com Backpatch-through: 18 Branch ------ REL_18_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/fe5d62951b3ccd382a530b790c3c2cb508b97c8c Modified Files -------------- src/backend/optimizer/path/allpaths.c | 179 ++++++++++++++++ src/backend/optimizer/plan/planner.c | 261 +++++++--------------- src/backend/optimizer/util/clauses.c | 268 +++++++++++++++++++++++ src/backend/utils/cache/lsyscache.c | 61 +++++- src/include/optimizer/clauses.h | 14 ++ src/include/utils/lsyscache.h | 1 + src/test/regress/expected/aggregates.out | 80 ++++++- src/test/regress/expected/collate.icu.utf8.out | 286 +++++++++++++++++++++++-- src/test/regress/expected/subselect.out | 214 ++++++++++++++++++ src/test/regress/sql/aggregates.sql | 46 +++- src/test/regress/sql/collate.icu.utf8.sql | 129 ++++++++++- src/test/regress/sql/subselect.sql | 105 +++++++++ src/tools/pgindent/typedefs.list | 4 +- 13 files changed, 1409 insertions(+), 239 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-07-06 07:16 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-07-06 07:16 pgsql: Fix qual pushdown past grouping with mismatched equivalence Richard Guo <rguo@postgresql.org> 2026-07-06 07:16 pgsql: Fix qual pushdown past grouping with mismatched equivalence Richard Guo <rguo@postgresql.org> 2026-07-06 07:16 pgsql: Fix qual pushdown past grouping with mismatched equivalence Richard Guo <rguo@postgresql.org>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox