agora inbox for pgsql-committers@postgresql.orghelp / color / mirror / Atom feed
pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr 7+ messages / 1 participants [nested] [flat]
* pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr @ 2026-07-28 20:09 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2026-07-28 20:09 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix planner's nullability/strictness logic for ScalarArrayOpExpr. find_nonnullable_rels and find_nonnullable_vars mistakenly treated a ScalarArrayOpExpr that could return FALSE as strict, but that's okay only at top level of a qual expression; further down, we've got to insist on a guaranteed-NULL result. The result was that we could draw mistaken conclusions about whether outer joins can be simplified, if the decision hinged on a non-top-level ScalarArrayOpExpr with a potentially-empty array argument. I believe this error dates to commit 72a070a36, which taught find_nonnullable_rels to descend into non-top-level parts of qual expressions. is_strict_saop (added earlier by 72153c058) already had enough intelligence to do the case correctly, but it wasn't passed the proper flag, ie "top_level" needs to be passed for "falseOK". e006a24ad copied that mistake into find_nonnullable_vars. Later, over-eager refactoring in commit 2f153ddfd broke contain_nonstrict_functions' handling of ScalarArrayOpExpr by treating it as though it were no different from an OpExpr. It is, because we must also prove the array is non-empty before concluding that the expression is strict. This could result in misclassifying an expression as strict when it is not, leading to assorted planning mistakes such as inlining a SQL function that shouldn't be inlined. We can almost fix this by just re-adding the previous handling of ScalarArrayOpExpr in that function, but doing only that would lead to also calling check_functions_in_node() and thus redundantly checking the operator's strictness. Avoid that by turning the if-series into an else-if chain, as it arguably should have been all along. The reason these errors have escaped detection for decades is that they are exposed only in arcane corner cases. ScalarArrayOpExpr with an empty array isn't typical usage, and even when that's possible several other conditions apply before the planner can reach a mistaken conclusion. While it's possible to build test cases demonstrating these mistakes, I (tgl) judged them too indirect and special-purpose to justify consuming regression test cycles forevermore. Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAJTYsWV3vqRJmST-gv1NsXEef-zOnjVJpYS910aBaiuMij4nFg@mail.gmail.com Discussion: https://postgr.es/m/CAJTYsWWcLGmz0f8_QPP_Liq-fc7-geiFSCdqoq3XGeRHPPsWeA@mail.gmail.com Backpatch-through: 14 Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/6fcee189bc5cd62d53b584fe2741723667ac44b0 Modified Files -------------- src/backend/optimizer/util/clauses.c | 74 ++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 33 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr @ 2026-07-28 20:09 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2026-07-28 20:09 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix planner's nullability/strictness logic for ScalarArrayOpExpr. find_nonnullable_rels and find_nonnullable_vars mistakenly treated a ScalarArrayOpExpr that could return FALSE as strict, but that's okay only at top level of a qual expression; further down, we've got to insist on a guaranteed-NULL result. The result was that we could draw mistaken conclusions about whether outer joins can be simplified, if the decision hinged on a non-top-level ScalarArrayOpExpr with a potentially-empty array argument. I believe this error dates to commit 72a070a36, which taught find_nonnullable_rels to descend into non-top-level parts of qual expressions. is_strict_saop (added earlier by 72153c058) already had enough intelligence to do the case correctly, but it wasn't passed the proper flag, ie "top_level" needs to be passed for "falseOK". e006a24ad copied that mistake into find_nonnullable_vars. Later, over-eager refactoring in commit 2f153ddfd broke contain_nonstrict_functions' handling of ScalarArrayOpExpr by treating it as though it were no different from an OpExpr. It is, because we must also prove the array is non-empty before concluding that the expression is strict. This could result in misclassifying an expression as strict when it is not, leading to assorted planning mistakes such as inlining a SQL function that shouldn't be inlined. We can almost fix this by just re-adding the previous handling of ScalarArrayOpExpr in that function, but doing only that would lead to also calling check_functions_in_node() and thus redundantly checking the operator's strictness. Avoid that by turning the if-series into an else-if chain, as it arguably should have been all along. The reason these errors have escaped detection for decades is that they are exposed only in arcane corner cases. ScalarArrayOpExpr with an empty array isn't typical usage, and even when that's possible several other conditions apply before the planner can reach a mistaken conclusion. While it's possible to build test cases demonstrating these mistakes, I (tgl) judged them too indirect and special-purpose to justify consuming regression test cycles forevermore. Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAJTYsWV3vqRJmST-gv1NsXEef-zOnjVJpYS910aBaiuMij4nFg@mail.gmail.com Discussion: https://postgr.es/m/CAJTYsWWcLGmz0f8_QPP_Liq-fc7-geiFSCdqoq3XGeRHPPsWeA@mail.gmail.com Backpatch-through: 14 Branch ------ REL_15_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/53470ffba99a3590469a118b8cc82d7ab080e071 Modified Files -------------- src/backend/optimizer/util/clauses.c | 72 ++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 32 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr @ 2026-07-28 20:09 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2026-07-28 20:09 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix planner's nullability/strictness logic for ScalarArrayOpExpr. find_nonnullable_rels and find_nonnullable_vars mistakenly treated a ScalarArrayOpExpr that could return FALSE as strict, but that's okay only at top level of a qual expression; further down, we've got to insist on a guaranteed-NULL result. The result was that we could draw mistaken conclusions about whether outer joins can be simplified, if the decision hinged on a non-top-level ScalarArrayOpExpr with a potentially-empty array argument. I believe this error dates to commit 72a070a36, which taught find_nonnullable_rels to descend into non-top-level parts of qual expressions. is_strict_saop (added earlier by 72153c058) already had enough intelligence to do the case correctly, but it wasn't passed the proper flag, ie "top_level" needs to be passed for "falseOK". e006a24ad copied that mistake into find_nonnullable_vars. Later, over-eager refactoring in commit 2f153ddfd broke contain_nonstrict_functions' handling of ScalarArrayOpExpr by treating it as though it were no different from an OpExpr. It is, because we must also prove the array is non-empty before concluding that the expression is strict. This could result in misclassifying an expression as strict when it is not, leading to assorted planning mistakes such as inlining a SQL function that shouldn't be inlined. We can almost fix this by just re-adding the previous handling of ScalarArrayOpExpr in that function, but doing only that would lead to also calling check_functions_in_node() and thus redundantly checking the operator's strictness. Avoid that by turning the if-series into an else-if chain, as it arguably should have been all along. The reason these errors have escaped detection for decades is that they are exposed only in arcane corner cases. ScalarArrayOpExpr with an empty array isn't typical usage, and even when that's possible several other conditions apply before the planner can reach a mistaken conclusion. While it's possible to build test cases demonstrating these mistakes, I (tgl) judged them too indirect and special-purpose to justify consuming regression test cycles forevermore. Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAJTYsWV3vqRJmST-gv1NsXEef-zOnjVJpYS910aBaiuMij4nFg@mail.gmail.com Discussion: https://postgr.es/m/CAJTYsWWcLGmz0f8_QPP_Liq-fc7-geiFSCdqoq3XGeRHPPsWeA@mail.gmail.com Backpatch-through: 14 Branch ------ REL_14_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/13b627a3e1811577e830c8b0a4a2cc095b22ac47 Modified Files -------------- src/backend/optimizer/util/clauses.c | 72 ++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 32 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr @ 2026-07-28 20:09 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2026-07-28 20:09 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix planner's nullability/strictness logic for ScalarArrayOpExpr. find_nonnullable_rels and find_nonnullable_vars mistakenly treated a ScalarArrayOpExpr that could return FALSE as strict, but that's okay only at top level of a qual expression; further down, we've got to insist on a guaranteed-NULL result. The result was that we could draw mistaken conclusions about whether outer joins can be simplified, if the decision hinged on a non-top-level ScalarArrayOpExpr with a potentially-empty array argument. I believe this error dates to commit 72a070a36, which taught find_nonnullable_rels to descend into non-top-level parts of qual expressions. is_strict_saop (added earlier by 72153c058) already had enough intelligence to do the case correctly, but it wasn't passed the proper flag, ie "top_level" needs to be passed for "falseOK". e006a24ad copied that mistake into find_nonnullable_vars. Later, over-eager refactoring in commit 2f153ddfd broke contain_nonstrict_functions' handling of ScalarArrayOpExpr by treating it as though it were no different from an OpExpr. It is, because we must also prove the array is non-empty before concluding that the expression is strict. This could result in misclassifying an expression as strict when it is not, leading to assorted planning mistakes such as inlining a SQL function that shouldn't be inlined. We can almost fix this by just re-adding the previous handling of ScalarArrayOpExpr in that function, but doing only that would lead to also calling check_functions_in_node() and thus redundantly checking the operator's strictness. Avoid that by turning the if-series into an else-if chain, as it arguably should have been all along. The reason these errors have escaped detection for decades is that they are exposed only in arcane corner cases. ScalarArrayOpExpr with an empty array isn't typical usage, and even when that's possible several other conditions apply before the planner can reach a mistaken conclusion. While it's possible to build test cases demonstrating these mistakes, I (tgl) judged them too indirect and special-purpose to justify consuming regression test cycles forevermore. Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAJTYsWV3vqRJmST-gv1NsXEef-zOnjVJpYS910aBaiuMij4nFg@mail.gmail.com Discussion: https://postgr.es/m/CAJTYsWWcLGmz0f8_QPP_Liq-fc7-geiFSCdqoq3XGeRHPPsWeA@mail.gmail.com Backpatch-through: 14 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/239eabda41e39de73c376000ba74bbeb8fe32a5c Modified Files -------------- src/backend/optimizer/util/clauses.c | 74 ++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 33 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr @ 2026-07-28 20:09 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2026-07-28 20:09 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix planner's nullability/strictness logic for ScalarArrayOpExpr. find_nonnullable_rels and find_nonnullable_vars mistakenly treated a ScalarArrayOpExpr that could return FALSE as strict, but that's okay only at top level of a qual expression; further down, we've got to insist on a guaranteed-NULL result. The result was that we could draw mistaken conclusions about whether outer joins can be simplified, if the decision hinged on a non-top-level ScalarArrayOpExpr with a potentially-empty array argument. I believe this error dates to commit 72a070a36, which taught find_nonnullable_rels to descend into non-top-level parts of qual expressions. is_strict_saop (added earlier by 72153c058) already had enough intelligence to do the case correctly, but it wasn't passed the proper flag, ie "top_level" needs to be passed for "falseOK". e006a24ad copied that mistake into find_nonnullable_vars. Later, over-eager refactoring in commit 2f153ddfd broke contain_nonstrict_functions' handling of ScalarArrayOpExpr by treating it as though it were no different from an OpExpr. It is, because we must also prove the array is non-empty before concluding that the expression is strict. This could result in misclassifying an expression as strict when it is not, leading to assorted planning mistakes such as inlining a SQL function that shouldn't be inlined. We can almost fix this by just re-adding the previous handling of ScalarArrayOpExpr in that function, but doing only that would lead to also calling check_functions_in_node() and thus redundantly checking the operator's strictness. Avoid that by turning the if-series into an else-if chain, as it arguably should have been all along. The reason these errors have escaped detection for decades is that they are exposed only in arcane corner cases. ScalarArrayOpExpr with an empty array isn't typical usage, and even when that's possible several other conditions apply before the planner can reach a mistaken conclusion. While it's possible to build test cases demonstrating these mistakes, I (tgl) judged them too indirect and special-purpose to justify consuming regression test cycles forevermore. Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAJTYsWV3vqRJmST-gv1NsXEef-zOnjVJpYS910aBaiuMij4nFg@mail.gmail.com Discussion: https://postgr.es/m/CAJTYsWWcLGmz0f8_QPP_Liq-fc7-geiFSCdqoq3XGeRHPPsWeA@mail.gmail.com Backpatch-through: 14 Branch ------ REL_19_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/9740c68ff7aa5d5a2d40c3ff0172467b38a17df8 Modified Files -------------- src/backend/optimizer/util/clauses.c | 74 ++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 33 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr @ 2026-07-28 20:09 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2026-07-28 20:09 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix planner's nullability/strictness logic for ScalarArrayOpExpr. find_nonnullable_rels and find_nonnullable_vars mistakenly treated a ScalarArrayOpExpr that could return FALSE as strict, but that's okay only at top level of a qual expression; further down, we've got to insist on a guaranteed-NULL result. The result was that we could draw mistaken conclusions about whether outer joins can be simplified, if the decision hinged on a non-top-level ScalarArrayOpExpr with a potentially-empty array argument. I believe this error dates to commit 72a070a36, which taught find_nonnullable_rels to descend into non-top-level parts of qual expressions. is_strict_saop (added earlier by 72153c058) already had enough intelligence to do the case correctly, but it wasn't passed the proper flag, ie "top_level" needs to be passed for "falseOK". e006a24ad copied that mistake into find_nonnullable_vars. Later, over-eager refactoring in commit 2f153ddfd broke contain_nonstrict_functions' handling of ScalarArrayOpExpr by treating it as though it were no different from an OpExpr. It is, because we must also prove the array is non-empty before concluding that the expression is strict. This could result in misclassifying an expression as strict when it is not, leading to assorted planning mistakes such as inlining a SQL function that shouldn't be inlined. We can almost fix this by just re-adding the previous handling of ScalarArrayOpExpr in that function, but doing only that would lead to also calling check_functions_in_node() and thus redundantly checking the operator's strictness. Avoid that by turning the if-series into an else-if chain, as it arguably should have been all along. The reason these errors have escaped detection for decades is that they are exposed only in arcane corner cases. ScalarArrayOpExpr with an empty array isn't typical usage, and even when that's possible several other conditions apply before the planner can reach a mistaken conclusion. While it's possible to build test cases demonstrating these mistakes, I (tgl) judged them too indirect and special-purpose to justify consuming regression test cycles forevermore. Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAJTYsWV3vqRJmST-gv1NsXEef-zOnjVJpYS910aBaiuMij4nFg@mail.gmail.com Discussion: https://postgr.es/m/CAJTYsWWcLGmz0f8_QPP_Liq-fc7-geiFSCdqoq3XGeRHPPsWeA@mail.gmail.com Backpatch-through: 14 Branch ------ REL_18_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/277122036c3382c5ab47034a180fde1176728c43 Modified Files -------------- src/backend/optimizer/util/clauses.c | 74 ++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 33 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr @ 2026-07-28 20:09 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2026-07-28 20:09 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix planner's nullability/strictness logic for ScalarArrayOpExpr. find_nonnullable_rels and find_nonnullable_vars mistakenly treated a ScalarArrayOpExpr that could return FALSE as strict, but that's okay only at top level of a qual expression; further down, we've got to insist on a guaranteed-NULL result. The result was that we could draw mistaken conclusions about whether outer joins can be simplified, if the decision hinged on a non-top-level ScalarArrayOpExpr with a potentially-empty array argument. I believe this error dates to commit 72a070a36, which taught find_nonnullable_rels to descend into non-top-level parts of qual expressions. is_strict_saop (added earlier by 72153c058) already had enough intelligence to do the case correctly, but it wasn't passed the proper flag, ie "top_level" needs to be passed for "falseOK". e006a24ad copied that mistake into find_nonnullable_vars. Later, over-eager refactoring in commit 2f153ddfd broke contain_nonstrict_functions' handling of ScalarArrayOpExpr by treating it as though it were no different from an OpExpr. It is, because we must also prove the array is non-empty before concluding that the expression is strict. This could result in misclassifying an expression as strict when it is not, leading to assorted planning mistakes such as inlining a SQL function that shouldn't be inlined. We can almost fix this by just re-adding the previous handling of ScalarArrayOpExpr in that function, but doing only that would lead to also calling check_functions_in_node() and thus redundantly checking the operator's strictness. Avoid that by turning the if-series into an else-if chain, as it arguably should have been all along. The reason these errors have escaped detection for decades is that they are exposed only in arcane corner cases. ScalarArrayOpExpr with an empty array isn't typical usage, and even when that's possible several other conditions apply before the planner can reach a mistaken conclusion. While it's possible to build test cases demonstrating these mistakes, I (tgl) judged them too indirect and special-purpose to justify consuming regression test cycles forevermore. Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAJTYsWV3vqRJmST-gv1NsXEef-zOnjVJpYS910aBaiuMij4nFg@mail.gmail.com Discussion: https://postgr.es/m/CAJTYsWWcLGmz0f8_QPP_Liq-fc7-geiFSCdqoq3XGeRHPPsWeA@mail.gmail.com Backpatch-through: 14 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/26d6b7dc9ec815f54472c7b08b0b83098007e361 Modified Files -------------- src/backend/optimizer/util/clauses.c | 74 ++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 33 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-07-28 20:09 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-07-28 20:09 pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr Tom Lane <tgl@sss.pgh.pa.us> 2026-07-28 20:09 pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr Tom Lane <tgl@sss.pgh.pa.us> 2026-07-28 20:09 pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr Tom Lane <tgl@sss.pgh.pa.us> 2026-07-28 20:09 pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr Tom Lane <tgl@sss.pgh.pa.us> 2026-07-28 20:09 pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr Tom Lane <tgl@sss.pgh.pa.us> 2026-07-28 20:09 pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr Tom Lane <tgl@sss.pgh.pa.us> 2026-07-28 20:09 pgsql: Fix planner's nullability/strictness logic for ScalarArrayOpExpr Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox