pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: When replanning a plpgsql "simple expression", check it's still 6+ messages / 1 participants [nested] [flat]
* pgsql: When replanning a plpgsql "simple expression", check it's still @ 2024-06-13 17:38 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-06-13 17:38 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org When replanning a plpgsql "simple expression", check it's still simple. The previous coding here assumed that we didn't need to recheck any of the querytree tests made in exec_simple_check_plan(). I think we supposed that those properties were fully determined by the syntax of the source text and hence couldn't change. That is true for most of them, but at least hasTargetSRFs and hasAggs can change by dint of forcibly dropping an originally-referenced function and recreating it with new properties. That leads to "unexpected plan node type" or similar failures. These tests are pretty cheap compared to the cost of replanning, so rather than sweat over exactly which properties need to be rechecked, let's just recheck them all. Hence, factor out those tests into a new function exec_is_simple_query(), and rearrange callers as needed. A second problem in the same area was that if we failed during replanning or during exec_save_simple_expr(), we'd potentially leave behind now-dangling pointers to the old simple expression, potentially resulting in crashes later. To fix, clear those pointers before replanning. The v12 code looks quite different in this area but still has the bug about needing to recheck query simplicity. I chose to back-patch all of the plpgsql_simple.sql test script, which formerly didn't exist in this branch. Per bug #18497 from Nikita Kalinin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18497-fe93b6da82ce31d4@postgresql.org Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/6dfac24401b7143ad5c75f991c18105e1267f88e Modified Files -------------- src/pl/plpgsql/src/expected/plpgsql_simple.out | 29 +++++ src/pl/plpgsql/src/pl_exec.c | 143 +++++++++++++++---------- src/pl/plpgsql/src/sql/plpgsql_simple.sql | 22 ++++ 3 files changed, 138 insertions(+), 56 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: When replanning a plpgsql "simple expression", check it's still @ 2024-06-13 17:38 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-06-13 17:38 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org When replanning a plpgsql "simple expression", check it's still simple. The previous coding here assumed that we didn't need to recheck any of the querytree tests made in exec_simple_check_plan(). I think we supposed that those properties were fully determined by the syntax of the source text and hence couldn't change. That is true for most of them, but at least hasTargetSRFs and hasAggs can change by dint of forcibly dropping an originally-referenced function and recreating it with new properties. That leads to "unexpected plan node type" or similar failures. These tests are pretty cheap compared to the cost of replanning, so rather than sweat over exactly which properties need to be rechecked, let's just recheck them all. Hence, factor out those tests into a new function exec_is_simple_query(), and rearrange callers as needed. A second problem in the same area was that if we failed during replanning or during exec_save_simple_expr(), we'd potentially leave behind now-dangling pointers to the old simple expression, potentially resulting in crashes later. To fix, clear those pointers before replanning. The v12 code looks quite different in this area but still has the bug about needing to recheck query simplicity. I chose to back-patch all of the plpgsql_simple.sql test script, which formerly didn't exist in this branch. Per bug #18497 from Nikita Kalinin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18497-fe93b6da82ce31d4@postgresql.org Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/82a931d3d2fe6c97c022399605413179b6c3c0aa Modified Files -------------- src/pl/plpgsql/src/expected/plpgsql_simple.out | 29 +++++ src/pl/plpgsql/src/pl_exec.c | 143 +++++++++++++++---------- src/pl/plpgsql/src/sql/plpgsql_simple.sql | 22 ++++ 3 files changed, 138 insertions(+), 56 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: When replanning a plpgsql "simple expression", check it's still @ 2024-06-13 17:38 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-06-13 17:38 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org When replanning a plpgsql "simple expression", check it's still simple. The previous coding here assumed that we didn't need to recheck any of the querytree tests made in exec_simple_check_plan(). I think we supposed that those properties were fully determined by the syntax of the source text and hence couldn't change. That is true for most of them, but at least hasTargetSRFs and hasAggs can change by dint of forcibly dropping an originally-referenced function and recreating it with new properties. That leads to "unexpected plan node type" or similar failures. These tests are pretty cheap compared to the cost of replanning, so rather than sweat over exactly which properties need to be rechecked, let's just recheck them all. Hence, factor out those tests into a new function exec_is_simple_query(), and rearrange callers as needed. A second problem in the same area was that if we failed during replanning or during exec_save_simple_expr(), we'd potentially leave behind now-dangling pointers to the old simple expression, potentially resulting in crashes later. To fix, clear those pointers before replanning. The v12 code looks quite different in this area but still has the bug about needing to recheck query simplicity. I chose to back-patch all of the plpgsql_simple.sql test script, which formerly didn't exist in this branch. Per bug #18497 from Nikita Kalinin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18497-fe93b6da82ce31d4@postgresql.org Branch ------ REL_15_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/bf552b1b2d8bc777e7831b7d2dbbd68bdd824c67 Modified Files -------------- src/pl/plpgsql/src/expected/plpgsql_simple.out | 29 +++++ src/pl/plpgsql/src/pl_exec.c | 143 +++++++++++++++---------- src/pl/plpgsql/src/sql/plpgsql_simple.sql | 22 ++++ 3 files changed, 138 insertions(+), 56 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: When replanning a plpgsql "simple expression", check it's still @ 2024-06-13 17:38 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-06-13 17:38 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org When replanning a plpgsql "simple expression", check it's still simple. The previous coding here assumed that we didn't need to recheck any of the querytree tests made in exec_simple_check_plan(). I think we supposed that those properties were fully determined by the syntax of the source text and hence couldn't change. That is true for most of them, but at least hasTargetSRFs and hasAggs can change by dint of forcibly dropping an originally-referenced function and recreating it with new properties. That leads to "unexpected plan node type" or similar failures. These tests are pretty cheap compared to the cost of replanning, so rather than sweat over exactly which properties need to be rechecked, let's just recheck them all. Hence, factor out those tests into a new function exec_is_simple_query(), and rearrange callers as needed. A second problem in the same area was that if we failed during replanning or during exec_save_simple_expr(), we'd potentially leave behind now-dangling pointers to the old simple expression, potentially resulting in crashes later. To fix, clear those pointers before replanning. The v12 code looks quite different in this area but still has the bug about needing to recheck query simplicity. I chose to back-patch all of the plpgsql_simple.sql test script, which formerly didn't exist in this branch. Per bug #18497 from Nikita Kalinin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18497-fe93b6da82ce31d4@postgresql.org Branch ------ REL_14_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/1450db793f643991e4684fa8b5cee6ded51ab870 Modified Files -------------- src/pl/plpgsql/src/expected/plpgsql_simple.out | 29 +++++ src/pl/plpgsql/src/pl_exec.c | 143 +++++++++++++++---------- src/pl/plpgsql/src/sql/plpgsql_simple.sql | 22 ++++ 3 files changed, 138 insertions(+), 56 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: When replanning a plpgsql "simple expression", check it's still @ 2024-06-13 17:38 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-06-13 17:38 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org When replanning a plpgsql "simple expression", check it's still simple. The previous coding here assumed that we didn't need to recheck any of the querytree tests made in exec_simple_check_plan(). I think we supposed that those properties were fully determined by the syntax of the source text and hence couldn't change. That is true for most of them, but at least hasTargetSRFs and hasAggs can change by dint of forcibly dropping an originally-referenced function and recreating it with new properties. That leads to "unexpected plan node type" or similar failures. These tests are pretty cheap compared to the cost of replanning, so rather than sweat over exactly which properties need to be rechecked, let's just recheck them all. Hence, factor out those tests into a new function exec_is_simple_query(), and rearrange callers as needed. A second problem in the same area was that if we failed during replanning or during exec_save_simple_expr(), we'd potentially leave behind now-dangling pointers to the old simple expression, potentially resulting in crashes later. To fix, clear those pointers before replanning. The v12 code looks quite different in this area but still has the bug about needing to recheck query simplicity. I chose to back-patch all of the plpgsql_simple.sql test script, which formerly didn't exist in this branch. Per bug #18497 from Nikita Kalinin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18497-fe93b6da82ce31d4@postgresql.org Branch ------ REL_13_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/1fa46dba53e6161d9120187e1a57dfe7b6aa73a3 Modified Files -------------- src/pl/plpgsql/src/expected/plpgsql_simple.out | 29 ++++++ src/pl/plpgsql/src/pl_exec.c | 139 +++++++++++++++---------- src/pl/plpgsql/src/sql/plpgsql_simple.sql | 22 ++++ 3 files changed, 137 insertions(+), 53 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: When replanning a plpgsql "simple expression", check it's still @ 2024-06-13 17:38 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-06-13 17:38 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org When replanning a plpgsql "simple expression", check it's still simple. The previous coding here assumed that we didn't need to recheck any of the querytree tests made in exec_simple_check_plan(). I think we supposed that those properties were fully determined by the syntax of the source text and hence couldn't change. That is true for most of them, but at least hasTargetSRFs and hasAggs can change by dint of forcibly dropping an originally-referenced function and recreating it with new properties. That leads to "unexpected plan node type" or similar failures. These tests are pretty cheap compared to the cost of replanning, so rather than sweat over exactly which properties need to be rechecked, let's just recheck them all. Hence, factor out those tests into a new function exec_is_simple_query(), and rearrange callers as needed. A second problem in the same area was that if we failed during replanning or during exec_save_simple_expr(), we'd potentially leave behind now-dangling pointers to the old simple expression, potentially resulting in crashes later. To fix, clear those pointers before replanning. The v12 code looks quite different in this area but still has the bug about needing to recheck query simplicity. I chose to back-patch all of the plpgsql_simple.sql test script, which formerly didn't exist in this branch. Per bug #18497 from Nikita Kalinin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18497-fe93b6da82ce31d4@postgresql.org Branch ------ REL_12_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/ec210914c751e715913405e25d734f859f79a47a Modified Files -------------- src/pl/plpgsql/src/Makefile | 2 +- src/pl/plpgsql/src/expected/plpgsql_simple.out | 120 +++++++++++++++++++++++++ src/pl/plpgsql/src/pl_exec.c | 89 ++++++++++++------ src/pl/plpgsql/src/sql/plpgsql_simple.sql | 104 +++++++++++++++++++++ 4 files changed, 285 insertions(+), 30 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-06-13 17:38 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-06-13 17:38 pgsql: When replanning a plpgsql "simple expression", check it's still Tom Lane <tgl@sss.pgh.pa.us> 2024-06-13 17:38 pgsql: When replanning a plpgsql "simple expression", check it's still Tom Lane <tgl@sss.pgh.pa.us> 2024-06-13 17:38 pgsql: When replanning a plpgsql "simple expression", check it's still Tom Lane <tgl@sss.pgh.pa.us> 2024-06-13 17:38 pgsql: When replanning a plpgsql "simple expression", check it's still Tom Lane <tgl@sss.pgh.pa.us> 2024-06-13 17:38 pgsql: When replanning a plpgsql "simple expression", check it's still Tom Lane <tgl@sss.pgh.pa.us> 2024-06-13 17:38 pgsql: When replanning a plpgsql "simple expression", check it's still 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