pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Avoid inserting PlaceHolderVars in cases where pre-v16 PG did no 3+ messages / 1 participants [nested] [flat]
* pgsql: Avoid inserting PlaceHolderVars in cases where pre-v16 PG did no @ 2024-08-30 16:42 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2024-08-30 16:42 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Avoid inserting PlaceHolderVars in cases where pre-v16 PG did not. Commit 2489d76c4 removed some logic from pullup_replace_vars() that avoided wrapping a PlaceHolderVar around a pulled-up subquery output expression if the expression could be proven to go to NULL anyway (because it contained Vars or PHVs of the pulled-up relation and did not contain non-strict constructs). But removing that logic turns out to cause performance regressions in some cases, because the extra PHV blocks subexpression folding, and will do so even if outer-join reduction later turns it into a no-op with no phnullingrels bits. This can for example prevent an expression from being matched to an index. The reason for always adding a PHV was to ensure we had someplace to put the varnullingrels marker bits of the Var being replaced. However, it turns out we can optimize in exactly the same cases that the previous code did, because we can instead attach the needed varnullingrels bits to the contained Var(s)/PHV(s). This is not a complete solution --- it would be even better if we could remove PHVs after reducing them to no-ops. It doesn't look practical to back-patch such an improvement, but this change seems safe and at least gets rid of the performance-regression cases. Per complaint from Nikhil Raj. Back-patch to v16 where the problem appeared. Discussion: https://postgr.es/m/CAG1ps1xvnTZceKK24OUfMKLPvDP2vjT-d+F2AOCWbw_v3KeEgg@mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/cb8e50a4a09fe541e32cd54ea90a97f2924121a1 Modified Files -------------- src/backend/optimizer/prep/prepjointree.c | 66 +++++++++++++++++++++++++------ src/backend/rewrite/rewriteManip.c | 9 +++-- src/test/regress/expected/subselect.out | 29 ++++++++++++++ src/test/regress/sql/subselect.sql | 18 +++++++++ 4 files changed, 107 insertions(+), 15 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Avoid inserting PlaceHolderVars in cases where pre-v16 PG did no @ 2024-08-30 16:42 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2024-08-30 16:42 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Avoid inserting PlaceHolderVars in cases where pre-v16 PG did not. Commit 2489d76c4 removed some logic from pullup_replace_vars() that avoided wrapping a PlaceHolderVar around a pulled-up subquery output expression if the expression could be proven to go to NULL anyway (because it contained Vars or PHVs of the pulled-up relation and did not contain non-strict constructs). But removing that logic turns out to cause performance regressions in some cases, because the extra PHV blocks subexpression folding, and will do so even if outer-join reduction later turns it into a no-op with no phnullingrels bits. This can for example prevent an expression from being matched to an index. The reason for always adding a PHV was to ensure we had someplace to put the varnullingrels marker bits of the Var being replaced. However, it turns out we can optimize in exactly the same cases that the previous code did, because we can instead attach the needed varnullingrels bits to the contained Var(s)/PHV(s). This is not a complete solution --- it would be even better if we could remove PHVs after reducing them to no-ops. It doesn't look practical to back-patch such an improvement, but this change seems safe and at least gets rid of the performance-regression cases. Per complaint from Nikhil Raj. Back-patch to v16 where the problem appeared. Discussion: https://postgr.es/m/CAG1ps1xvnTZceKK24OUfMKLPvDP2vjT-d+F2AOCWbw_v3KeEgg@mail.gmail.com Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/b43110869fd82b549df5c6ab7ade476d6e414f37 Modified Files -------------- src/backend/optimizer/prep/prepjointree.c | 66 +++++++++++++++++++++++++------ src/backend/rewrite/rewriteManip.c | 9 +++-- src/test/regress/expected/subselect.out | 29 ++++++++++++++ src/test/regress/sql/subselect.sql | 18 +++++++++ 4 files changed, 107 insertions(+), 15 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Avoid inserting PlaceHolderVars in cases where pre-v16 PG did no @ 2024-08-30 16:42 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2024-08-30 16:42 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Avoid inserting PlaceHolderVars in cases where pre-v16 PG did not. Commit 2489d76c4 removed some logic from pullup_replace_vars() that avoided wrapping a PlaceHolderVar around a pulled-up subquery output expression if the expression could be proven to go to NULL anyway (because it contained Vars or PHVs of the pulled-up relation and did not contain non-strict constructs). But removing that logic turns out to cause performance regressions in some cases, because the extra PHV blocks subexpression folding, and will do so even if outer-join reduction later turns it into a no-op with no phnullingrels bits. This can for example prevent an expression from being matched to an index. The reason for always adding a PHV was to ensure we had someplace to put the varnullingrels marker bits of the Var being replaced. However, it turns out we can optimize in exactly the same cases that the previous code did, because we can instead attach the needed varnullingrels bits to the contained Var(s)/PHV(s). This is not a complete solution --- it would be even better if we could remove PHVs after reducing them to no-ops. It doesn't look practical to back-patch such an improvement, but this change seems safe and at least gets rid of the performance-regression cases. Per complaint from Nikhil Raj. Back-patch to v16 where the problem appeared. Discussion: https://postgr.es/m/CAG1ps1xvnTZceKK24OUfMKLPvDP2vjT-d+F2AOCWbw_v3KeEgg@mail.gmail.com Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/80d9c07a4a8c43955c6c818a125057f4315476a1 Modified Files -------------- src/backend/optimizer/prep/prepjointree.c | 66 +++++++++++++++++++++++++------ src/backend/rewrite/rewriteManip.c | 9 +++-- src/test/regress/expected/subselect.out | 29 ++++++++++++++ src/test/regress/sql/subselect.sql | 18 +++++++++ 4 files changed, 107 insertions(+), 15 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2024-08-30 16:42 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-08-30 16:42 pgsql: Avoid inserting PlaceHolderVars in cases where pre-v16 PG did no Tom Lane <tgl@sss.pgh.pa.us> 2024-08-30 16:42 pgsql: Avoid inserting PlaceHolderVars in cases where pre-v16 PG did no Tom Lane <tgl@sss.pgh.pa.us> 2024-08-30 16:42 pgsql: Avoid inserting PlaceHolderVars in cases where pre-v16 PG did no Tom Lane <tgl@sss.pgh.pa.us>
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