Thread: pgsql: Do assorted mop-up in the planner.
Do assorted mop-up in the planner. Remove RestrictInfo.nullable_relids, along with a good deal of infrastructure that calculated it. One use-case for it was in join_clause_is_movable_to, but we can now replace that usage with a check to see if the clause's relids include any outer join that can null the target relation. The other use-case was in join_clause_is_movable_into, but that test can just be dropped entirely now that the clause's relids include outer joins. Furthermore, join_clause_is_movable_into should now be accurate enough that it will accept anything returned by generate_join_implied_equalities, so we can restore the Assert that was diked out in commit 95f4e59c3. Remove the outerjoin_delayed mechanism. We needed this before to prevent quals from getting evaluated below outer joins that should null some of their vars. Now that we consider varnullingrels while placing quals, that's taken care of automatically, so throw the whole thing away. Teach remove_useless_result_rtes to also remove useless FromExprs. Having done that, the delay_upper_joins flag serves no purpose any more and we can remove it, largely reverting 11086f2f2. Use constant TRUE for "dummy" clauses when throwing back outer joins. This improves on a hack I introduced in commit 6a6522529. If we have a left-join clause l.x = r.y, and a WHERE clause l.x = constant, we generate r.y = constant and then don't really have a need for the join clause. But we must throw the join clause back anyway after marking it redundant, so that the join search heuristics won't think this is a clauseless join and avoid it. That was a kluge introduced under time pressure, and after looking at it I thought of a better way: let's just introduce constant-TRUE "join clauses" instead, and get rid of them at the end. This improves the generated plans for such cases by not having to test a redundant join clause. We can also get rid of the ugly hack used to mark such clauses as redundant for selectivity estimation. Patch by me; thanks to Richard Guo for review. Discussion: https://postgr.es/m/830269.1656693747@sss.pgh.pa.us Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/b448f1c8d83f8b65e2f0080c556ee21a7076da25 Modified Files -------------- contrib/postgres_fdw/postgres_fdw.c | 2 - src/backend/optimizer/path/allpaths.c | 1 - src/backend/optimizer/path/clausesel.c | 6 - src/backend/optimizer/path/costsize.c | 2 - src/backend/optimizer/path/equivclass.c | 168 ++++---------- src/backend/optimizer/path/joinrels.c | 1 - src/backend/optimizer/path/pathkeys.c | 40 +--- src/backend/optimizer/plan/analyzejoins.c | 16 +- src/backend/optimizer/plan/initsplan.c | 371 ++++++------------------------ src/backend/optimizer/plan/planner.c | 7 +- src/backend/optimizer/prep/prepjointree.c | 112 +++++++-- src/backend/optimizer/util/appendinfo.c | 3 - src/backend/optimizer/util/inherit.c | 7 +- src/backend/optimizer/util/orclauses.c | 12 +- src/backend/optimizer/util/placeholder.c | 97 -------- src/backend/optimizer/util/relnode.c | 21 +- src/backend/optimizer/util/restrictinfo.c | 123 +++++----- src/include/nodes/pathnodes.h | 48 +--- src/include/optimizer/paths.h | 3 +- src/include/optimizer/placeholder.h | 2 - src/include/optimizer/planmain.h | 2 - src/include/optimizer/restrictinfo.h | 6 +- src/test/regress/expected/join.out | 34 ++- src/test/regress/sql/join.sql | 4 +- 24 files changed, 325 insertions(+), 763 deletions(-)
Hi Tom,
On Tue, 31 Jan 2023 at 05:43, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Do assorted mop-up in the planner.
This commit is causing occasional Asserts in my testing.
SQL / Backtrace / triaging below.
Program terminated with signal SIGABRT, Aborted.
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x00007f6d091d0859 in __GI_abort () at abort.c:79
#2 0x000055bf99f9d962 in ExceptionalCondition
(conditionName=0x55bf9a153280 "bms_is_subset(rinfo->required_relids,
both_input_relids)", fileName=0x55bf9a152faf "relnode.c",
lineNumber=1314) at assert.c:66
#3 0x000055bf99cc3689 in subbuild_joinrel_restrictlist
(root=0x55bf9b35ec08, joinrel=0x55bf9b36bc38,
input_rel=0x55bf9b367420, both_input_relids=0x55bf9b36c7f8,
new_restrictlist=0x0)
at relnode.c:1314
#4 0x000055bf99cc33f9 in build_joinrel_restrictlist
(root=0x55bf9b35ec08, joinrel=0x55bf9b36bc38,
outer_rel=0x55bf9b367420, inner_rel=0x55bf9b3660e0) at relnode.c:1232
#5 0x000055bf99cc2730 in build_join_rel (root=0x55bf9b35ec08,
joinrelids=0x55bf9b36b2a8, outer_rel=0x55bf9b367420,
inner_rel=0x55bf9b3660e0, sjinfo=0x55bf9b368068,
restrictlist_ptr=0x7ffd89ee7560) at relnode.c:771
#6 0x000055bf99c5eedf in make_join_rel (root=0x55bf9b35ec08,
rel1=0x55bf9b367420, rel2=0x55bf9b3660e0) at joinrels.c:756
#7 0x000055bf99c5e3bd in make_rels_by_clause_joins
(root=0x55bf9b35ec08, old_rel=0x55bf9b367420,
other_rels_list=0x55bf9b36b408, other_rels=0x55bf9b36b420) at
joinrels.c:312
#8 0x000055bf99c5dedd in join_search_one_level (root=0x55bf9b35ec08,
level=3) at joinrels.c:123
#9 0x000055bf99c3fbc7 in standard_join_search (root=0x55bf9b35ec08,
levels_needed=3, initial_rels=0x55bf9b36b408) at allpaths.c:3387
#10 0x000055bf99c3fb34 in make_rel_from_joinlist (root=0x55bf9b35ec08,
joinlist=0x55bf9b367f18) at allpaths.c:3318
#11 0x000055bf99c3aabd in make_one_rel (root=0x55bf9b35ec08,
joinlist=0x55bf9b367f18) at allpaths.c:211
#12 0x000055bf99c7aa5e in query_planner (root=0x55bf9b35ec08,
qp_callback=0x55bf99c8112d <standard_qp_callback>,
qp_extra=0x7ffd89ee79b0) at planmain.c:278
#13 0x000055bf99c7d374 in grouping_planner (root=0x55bf9b35ec08,
tuple_fraction=0) at planner.c:1488
#14 0x000055bf99c7ca2f in subquery_planner (glob=0x55bf9b340e18,
parse=0x55bf9b26f5b8, parent_root=0x0, hasRecursion=false,
tuple_fraction=0) at planner.c:1057
#15 0x000055bf99c7b137 in standard_planner (parse=0x55bf9b26f5b8,
query_string=0x55bf9b26de28 "SELECT pg_catalog.avg(NULL::NUMERIC)
OVER (ORDER BY ref_3.schemaname) AS c2\nFROM (SELECT) AS subq_0\n
LEFT JOIN pg_catalog.pg_statio_all_sequences AS ref_3 ON NULL;",
cursorOptions=2048, boundParams=0x0) at planner.c:411
SQL:
SELECT pg_catalog.avg(NULL::NUMERIC) OVER (ORDER BY ref_3.schemaname) AS c2
FROM (SELECT) AS subq_0
LEFT JOIN pg_catalog.pg_statio_all_sequences AS ref_3 ON NULL;
Checking (fb1a59de0c~10) - 8c1cd726c5d997d5d170505ec15a2dc1dfe81d6a - Crash
Checking (fb1a59de0c~11) - 54e72b66ed1a55c2fa558bc60d534bdc61dbb62f - Crash
Checking (fb1a59de0c~12) - 3bef56e11650a33f70adeb6dd442bc2b48bb9b72 - Crash
Checking (fb1a59de0c~13) - b448f1c8d83f8b65e2f0080c556ee21a7076da25 - Crash
Checking (fb1a59de0c~14) - 2489d76c4906f4461a364ca8ad7e0751ead8aa0d - Success
Checking (fb1a59de0c~15) - ec7e053a98f39a9e3c7e6d35f0d2e83933882399 - Success
Thanks to SQLSmith / SQLReduce.
-
Robins Tharakan
Amazon Web Services
Robins Tharakan <tharakan@gmail.com> writes:
> This commit is causing occasional Asserts in my testing.
Thanks! I'm tired now, but will have a look tomorrow.
regards, tom lane
Thanks for taking a look. To add, although a slightly different signature, it looks like Bug #17769 is also related to this commit b448f1c8d83f. https://www.postgresql.org/message-id/17769-e4f7a5c9d84a80a7%40postgresql.org ======= SQL SELECT FROM pg_catalog.pg_statio_all_tables AS ref_0, LATERAL (SELECT WHERE ref_0.schemaname = ref_0.relname) AS subq_0; Checking (0736fc1ceb~16) - 3bef56e11650a33f70adeb6dd442bc2b48bb9b72 - Crash Checking (0736fc1ceb~17) - b448f1c8d83f8b65e2f0080c556ee21a7076da25 - Crash Checking (0736fc1ceb~18) - 2489d76c4906f4461a364ca8ad7e0751ead8aa0d - Success Checking (0736fc1ceb~19) - ec7e053a98f39a9e3c7e6d35f0d2e83933882399 - Success Checking (0736fc1ceb~20) - fe9e658f4d7fbc12d2b6a74c4ee90c73e53d68ef - Success - robins On Thu, 2 Feb 2023 at 12:23, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Robins Tharakan <tharakan@gmail.com> writes: > > This commit is causing occasional Asserts in my testing. > > Thanks! I'm tired now, but will have a look tomorrow. > > regards, tom lane
Hi Tom,
I'll probably switch off testing master for a day (and probably stop
posting more on this thread), but thought I'd post one last
(seemingly different) signature around the same commit, before
I wrap up for the day.
On Thu, 2 Feb 2023 at 23:25, Robins Tharakan <tharakan@gmail.com> wrote:
>
> To add, although a slightly different signature, it looks like Bug #17769 is
> also related to this commit b448f1c8d83f.
Core was generated by `postgres: 117d2604c2@master@sqith: ubuntu
postgres 127.0.'.
Program terminated with signal SIGABRT, Aborted.
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x00007f7934a6f859 in __GI_abort () at abort.c:79
#2 0x000055bea71e1dfe in ExceptionalCondition (
conditionName=0x55bea7398420 "nrm_match == NRM_SUBSET ?
bms_is_subset(phv->phnullingrels, subphv->phnullingrels) : nrm_match
== NRM_SUPERSET ? bms_is_subset(subphv->phnullingrels, phv->p
hnullingrels) : bms_equal(subphv->phnullingr"...,
fileName=0x55bea7397e4f "setrefs.c",
lineNumber=2845) at assert.c:66
#3 0x000055bea6ed2088 in search_indexed_tlist_for_phv (phv=0x55bea8845868,
itlist=0x55bea884d3a0, newvarno=-2, nrm_match=NRM_SUPERSET) at
setrefs.c:2845
#4 0x000055bea6ed24ec in fix_join_expr_mutator (node=0x55bea8845868,
context=0x7ffcabf84820) at setrefs.c:3057
#5 0x000055bea6e0f1ea in expression_tree_mutator_impl (node=0x55bea8845928,
mutator=0x55bea6ed2317 <fix_join_expr_mutator>, context=0x7ffcabf84820)
at nodeFuncs.c:3137
#6 0x000055bea6ed269b in fix_join_expr_mutator (node=0x55bea8845928,
context=0x7ffcabf84820) at setrefs.c:3104
#7 0x000055bea6e0e055 in expression_tree_mutator_impl (node=0x55bea88457f8,
mutator=0x55bea6ed2317 <fix_join_expr_mutator>, context=0x7ffcabf84820)
at nodeFuncs.c:2771
#8 0x000055bea6ed269b in fix_join_expr_mutator (node=0x55bea88457f8,
context=0x7ffcabf84820) at setrefs.c:3104
#9 0x000055bea6e0f1ea in expression_tree_mutator_impl (node=0x55bea88457c8,
mutator=0x55bea6ed2317 <fix_join_expr_mutator>, context=0x7ffcabf84820)
at nodeFuncs.c:3137
#10 0x000055bea6ed269b in fix_join_expr_mutator (node=0x55bea88457c8,
context=0x7ffcabf84820) at setrefs.c:3104
#11 0x000055bea6e0e229 in expression_tree_mutator_impl (node=0x55bea88456c8,
mutator=0x55bea6ed2317 <fix_join_expr_mutator>, context=0x7ffcabf84820)
at nodeFuncs.c:2811
#12 0x000055bea6ed269b in fix_join_expr_mutator (node=0x55bea88456c8,
context=0x7ffcabf84820) at setrefs.c:3104
#13 0x000055bea6e0f1ea in expression_tree_mutator_impl (node=0x55bea884c330,
=======
SQL
rollback; begin;
create table tn(i name);
create table rc(cname text);
SELECT
FROM (select '' a) AS sample_1
LEFT JOIN ((SELECT
'(429.25491125213796,40.254082597002764)'::point p) AS sample_2
RIGHT JOIN ((SELECT 'int2col'::name colname,
'int2'::text typ) AS sample_3
RIGHT JOIN rc AS sample_4 ON
sample_4.cname IS NOT NULL)
ON NULL
RIGHT JOIN tn AS ref_0 ON sample_2.p IS NULL
OR
sample_3.colname !~~ sample_3.typ)
ON sample_1.a ^@ sample_4.cname;
rollback;
Checking (0736fc1ceb~16) - 3bef56e11650a33f70adeb6dd442bc2b48bb9b72 - Crash
Checking (0736fc1ceb~17) - b448f1c8d83f8b65e2f0080c556ee21a7076da25 - Crash
Checking (0736fc1ceb~18) - 2489d76c4906f4461a364ca8ad7e0751ead8aa0d - Success
Checking (0736fc1ceb~19) - ec7e053a98f39a9e3c7e6d35f0d2e83933882399 - Success
-
Robins Tharakan
Amazon Web Services
Robins Tharakan <tharakan@gmail.com> writes:
> I'll probably switch off testing master for a day (and probably stop
> posting more on this thread), but thought I'd post one last
> (seemingly different) signature around the same commit, before
> I wrap up for the day.
Thanks for all the test cases! I remembered that I have to start
on release-note writing today, which with other commitments means
I may not get to these bugs before the weekend. But I very much
appreciate your reports.
regards, tom lane
Robins Tharakan <tharakan@gmail.com> writes:
> I'll probably switch off testing master for a day (and probably stop
> posting more on this thread), but thought I'd post one last
> (seemingly different) signature around the same commit, before
> I wrap up for the day.
I'd hoped that some of these failures shared a root cause, but nope
they were really four different bugs :-(. I've now pushed fixes
for all four, and I hope you'll turn your fuzzer back on. This
is very valuable testing.
regards, tom lane
On Mon, 6 Feb 2023 at 05:57, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I'd hoped that some of these failures shared a root cause, but nope > they were really four different bugs :-(. I've now pushed fixes > for all four, and I hope you'll turn your fuzzer back on. This > is very valuable testing. Thanks Tom for fixing these promptly but more importantly for confirming that this was helpful... allows for similar effort going forward. - robins