delta/postgresql.git/src/backend/optimizer, branch master git.postgresql.org: git/postgresql.git Track tlist_vinfo.varnullingrels even in non-Assert builds. 2023-05-17T15:46:15+00:00 Tom Lane tgl@sss.pgh.pa.us 2023-05-17T15:46:15+00:00 69c430626b5eb44825a12e8bc90a59a90c4f5a03 Oversight in commit 867be9c07 (which should get reverted along with that, if we ever do revert it). Per buildfarm. 
Oversight in commit 867be9c07 (which should get reverted
along with that, if we ever do revert it).  Per buildfarm.
Fix some issues with improper placement of outer join clauses. 2023-05-17T15:14:04+00:00 Tom Lane tgl@sss.pgh.pa.us 2023-05-17T15:13:52+00:00 9df8f903eb6758be5a19e66cdf77e922e9329c31 After applying outer-join identity 3 in the forward direction, it was possible for the planner to mistakenly apply a qual clause from above the two outer joins at the now-lower join level. This can give the wrong answer, since a value that would get nulled by the now-upper join might not yet be null. To fix, when we perform such a transformation, consider that the now-lower join hasn't really completed the outer join it's nominally responsible for and thus its relid set should not include that OJ's relid (nor should its output Vars have that nullingrel bit set). Instead we add those bits when the now-upper join is performed. The existing rules for qual placement then suffice to prevent higher qual clauses from dropping below the now-upper join. There are a few complications from needing to consider transitive closures in case multiple pushdowns have happened, but all in all it's not a very complex patch. This is all new logic (from 2489d76c4) so no need to back-patch. The added test cases all have the same results as in v15. Tom Lane and Richard Guo Discussion: https://postgr.es/m/0b819232-4b50-f245-1c7d-c8c61bf41827@postgrespro.ru 
After applying outer-join identity 3 in the forward direction,
it was possible for the planner to mistakenly apply a qual clause
from above the two outer joins at the now-lower join level.
This can give the wrong answer, since a value that would get nulled
by the now-upper join might not yet be null.

To fix, when we perform such a transformation, consider that the
now-lower join hasn't really completed the outer join it's nominally
responsible for and thus its relid set should not include that OJ's
relid (nor should its output Vars have that nullingrel bit set).
Instead we add those bits when the now-upper join is performed.
The existing rules for qual placement then suffice to prevent
higher qual clauses from dropping below the now-upper join.
There are a few complications from needing to consider transitive
closures in case multiple pushdowns have happened, but all in all
it's not a very complex patch.

This is all new logic (from 2489d76c4) so no need to back-patch.
The added test cases all have the same results as in v15.

Tom Lane and Richard Guo

Discussion: https://postgr.es/m/0b819232-4b50-f245-1c7d-c8c61bf41827@postgrespro.ru
Convert nullingrels match checks from Asserts to test-and-elog. 2023-05-17T15:14:04+00:00 Tom Lane tgl@sss.pgh.pa.us 2023-05-17T14:50:35+00:00 867be9c0738bef591544d39985f886b7d8e99bf0 It seems like the code that these checks are backstopping may have a few bugs left in it. Use a test-and-elog so that the tests are performed even in non-assert builds, and so that we get something more informative than "server closed the connection" on failure. Committed separately with the idea that eventually we'll revert this. It might be awhile though. Discussion: https://postgr.es/m/3014965.1684293045@sss.pgh.pa.us 
It seems like the code that these checks are backstopping may have
a few bugs left in it.  Use a test-and-elog so that the tests are
performed even in non-assert builds, and so that we get something
more informative than "server closed the connection" on failure.

Committed separately with the idea that eventually we'll revert
this.  It might be awhile though.

Discussion: https://postgr.es/m/3014965.1684293045@sss.pgh.pa.us
Add back SQLValueFunction for SQL keywords 2023-05-17T01:19:17+00:00 Michael Paquier michael@paquier.xyz 2023-05-17T01:19:17+00:00 d8c3106bb60e4f87be595f241e173ba3c2b7aa2c This is equivalent to a revert of f193883 and fb32748, with the addition that the declaration of the SQLValueFunction node needs to gain a couple of node_attr for query jumbling. The performance impact of removing the function call inlining is proving to be too huge for some workloads where these are used. A worst-case test case of involving only simple SELECT queries with a SQL keyword is proving to lead to a reduction of 10% in TPS via pgbench and prepared queries on a high-end machine. None of the tests I ran back for this set of changes saw such a huge gap, but Alexander Lakhin and Andres Freund have found that this can be noticeable. Keeping the older performance would mean to do more inlining in the executor when using COERCE_SQL_SYNTAX for a function expression, similarly to what SQLValueFunction does. This requires more redesign work and there is little time until 16beta1 is released, so for now reverting the change is the best way forward, bringing back the previous performance. Bump catalog version. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/b32bed1b-0746-9b20-1472-4bdc9ca66d52@gmail.com 
This is equivalent to a revert of f193883 and fb32748, with the addition
that the declaration of the SQLValueFunction node needs to gain a couple
of node_attr for query jumbling.  The performance impact of removing the
function call inlining is proving to be too huge for some workloads
where these are used.  A worst-case test case of involving only simple
SELECT queries with a SQL keyword is proving to lead to a reduction of
10% in TPS via pgbench and prepared queries on a high-end machine.

None of the tests I ran back for this set of changes saw such a huge
gap, but Alexander Lakhin and Andres Freund have found that this can be
noticeable.  Keeping the older performance would mean to do more
inlining in the executor when using COERCE_SQL_SYNTAX for a function
expression, similarly to what SQLValueFunction does.  This requires more
redesign work and there is little time until 16beta1 is released, so for
now reverting the change is the best way forward, bringing back the
previous performance.

Bump catalog version.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/b32bed1b-0746-9b20-1472-4bdc9ca66d52@gmail.com
Undo faulty attempt at not relying on RINFO_IS_PUSHED_DOWN. 2023-05-11T17:44:25+00:00 Tom Lane tgl@sss.pgh.pa.us 2023-05-11T17:44:25+00:00 c8b881d21fd8e0214bc43280d6231cc2c6ef55ef I've had a bee in my bonnet for some time about getting rid of RestrictInfo.is_pushed_down, because it's squishily defined and requires not-inexpensive extra tests to use (cf RINFO_IS_PUSHED_DOWN). In commit 2489d76c4, I tried to make remove_rel_from_query() not depend on that macro; but the replacement test is buggy, as exposed by a report from Rushabh Lathia and Robert Haas. That change was pretty incidental to the main goal of 2489d76c4, so let's just revert it for now. (Getting rid of is_pushed_down is still far away, anyway.) Discussion: https://postgr.es/m/CA+TgmoYco=hmg+iX1CW9Y1_CzNoSL81J03wUG-d2_3=rue+L2A@mail.gmail.com 
I've had a bee in my bonnet for some time about getting rid of
RestrictInfo.is_pushed_down, because it's squishily defined and
requires not-inexpensive extra tests to use (cf RINFO_IS_PUSHED_DOWN).
In commit 2489d76c4, I tried to make remove_rel_from_query() not
depend on that macro; but the replacement test is buggy,
as exposed by a report from Rushabh Lathia and Robert Haas.
That change was pretty incidental to the main goal of 2489d76c4,
so let's just revert it for now.  (Getting rid of is_pushed_down
is still far away, anyway.)

Discussion: https://postgr.es/m/CA+TgmoYco=hmg+iX1CW9Y1_CzNoSL81J03wUG-d2_3=rue+L2A@mail.gmail.com
Handle RLS dependencies in inlined set-returning functions properly. 2023-05-08T14:12:44+00:00 Tom Lane tgl@sss.pgh.pa.us 2023-05-08T14:12:44+00:00 ca73753b090c33bc69ce299b4d7fff891a77b8ad If an SRF in the FROM clause references a table having row-level security policies, and we inline that SRF into the calling query, we neglected to mark the plan as potentially dependent on which role is executing it. This could lead to later executions in the same session returning or hiding rows that should have been hidden or returned instead. Our thanks to Wolfgang Walther for reporting this problem. Stephen Frost and Tom Lane Security: CVE-2023-2455 
If an SRF in the FROM clause references a table having row-level
security policies, and we inline that SRF into the calling query,
we neglected to mark the plan as potentially dependent on which
role is executing it.  This could lead to later executions in the
same session returning or hiding rows that should have been hidden
or returned instead.

Our thanks to Wolfgang Walther for reporting this problem.

Stephen Frost and Tom Lane

Security: CVE-2023-2455
Revert "Move PartitionPruneInfo out of plan nodes into PlannedStmt" 2023-05-04T10:09:59+00:00 Alvaro Herrera alvherre@alvh.no-ip.org 2023-05-04T10:09:59+00:00 5472743d9e8583638a897b47558066167cc14583 This reverts commit ec386948948c and its fixup 589bb816499e. This change was intended to support query planning avoiding acquisition of locks on partitions that were going to be pruned; however, the overall project took a different direction at [1] and this bit is no longer needed. Put things back the way they were as agreed in [2], to avoid unnecessary complexity. Discussion: [1] https://postgr.es/m/4191508.1674157166@sss.pgh.pa.us Discussion: [2] https://postgr.es/m/20230502175409.kcoirxczpdha26wt@alvherre.pgsql 
This reverts commit ec386948948c and its fixup 589bb816499e.

This change was intended to support query planning avoiding acquisition
of locks on partitions that were going to be pruned; however, the
overall project took a different direction at [1] and this bit is no
longer needed.  Put things back the way they were as agreed in [2], to
avoid unnecessary complexity.

Discussion: [1] https://postgr.es/m/4191508.1674157166@sss.pgh.pa.us
Discussion: [2] https://postgr.es/m/20230502175409.kcoirxczpdha26wt@alvherre.pgsql
Fix typos in comments 2023-05-02T03:23:08+00:00 Michael Paquier michael@paquier.xyz 2023-05-02T03:23:08+00:00 8961cb9a0315fa23458587b3df547ca8d8e00f85 The changes done in this commit impact comments with no direct user-visible changes, with fixes for incorrect function, variable or structure names. Author: Alexander Lakhin Discussion: https://postgr.es/m/e8c38840-596a-83d6-bd8d-cebc51111572@gmail.com 
The changes done in this commit impact comments with no direct
user-visible changes, with fixes for incorrect function, variable or
structure names.

Author: Alexander Lakhin
Discussion: https://postgr.es/m/e8c38840-596a-83d6-bd8d-cebc51111572@gmail.com
Fix various typos and incorrect/outdated name references 2023-04-19T01:50:33+00:00 David Rowley drowley@postgresql.org 2023-04-19T01:50:33+00:00 3f58a4e2960a9509036b7d94beab64a747dc59dc Author: Alexander Lakhin Discussion: https://postgr.es/m/699beab4-a6ca-92c9-f152-f559caf6dc25@gmail.com 
Author: Alexander Lakhin
Discussion: https://postgr.es/m/699beab4-a6ca-92c9-f152-f559caf6dc25@gmail.com
Fix some typos and some incorrectly duplicated words 2023-04-18T02:03:49+00:00 David Rowley drowley@postgresql.org 2023-04-18T02:03:30+00:00 eef231e8161eb8031a928fe48134ef25ab81fe5e Author: Justin Pryzby Reviewed-by: David Rowley Discussion: https://postgr.es/m/ZD3D1QxoccnN8A1V@telsasoft.com 
Author: Justin Pryzby
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/ZD3D1QxoccnN8A1V@telsasoft.com