Re: ERROR: no relation entry for relid 6 - Mailing list pgsql-hackers
| From | Richard Guo |
|---|---|
| Subject | Re: ERROR: no relation entry for relid 6 |
| Date | |
| Msg-id | CAMbWs4_8L-o1FCnHt7Dzg3ZGaYzJKoiNHP_jrg-AOZx=RDaYtg@mail.gmail.com Whole thread Raw |
| In response to | Re: ERROR: no relation entry for relid 6 (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: ERROR: no relation entry for relid 6
|
| List | pgsql-hackers |
On Fri, May 26, 2023 at 6:06 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
1. The test case you give upthread only needs to ignore commute_below_l,
that is it still passes without the lines
+ join_plus_commute = bms_add_members(join_plus_commute,
+ removable_sjinfo->commute_above_r);
Based on what deconstruct_distribute_oj_quals is doing, it seems
likely to me that there are cases that require ignoring
commute_above_r, but I've not tried to devise one. It'd be good to
have one to include in the commit, if we can find one.
It seems that queries of the second form of identity 3 require ignoring
commute_above_r.
select 1 from t t1 left join (t t2 left join t t3 on t2.a = t3.a) on
t1.a = t2.a;
When removing t2/t3 join, the clone of 't2.a = t3.a' with t1 join in the
nulling bitmap would be put back if we do not ignore commute_above_r.
There is no observable problem though because it would be rejected later
in subbuild_joinrel_restrictlist, but still I think it should not be put
back in the first place.
commute_above_r.
select 1 from t t1 left join (t t2 left join t t3 on t2.a = t3.a) on
t1.a = t2.a;
When removing t2/t3 join, the clone of 't2.a = t3.a' with t1 join in the
nulling bitmap would be put back if we do not ignore commute_above_r.
There is no observable problem though because it would be rejected later
in subbuild_joinrel_restrictlist, but still I think it should not be put
back in the first place.
2. We could go a little further in refactoring, specifically the
computation of joinrelids could be moved into remove_rel_from_query,
since remove_useless_joins itself doesn't need it. Not sure if
this'd be an improvement or not. Certainly it'd be a loser if
remove_useless_joins grew a reason to need the value; but I can't
foresee a reason for that to happen --- remove_rel_from_query is
where basically all the work is going on.
+1 to move the computation of joinrelids into remove_rel_from_query().
We also do that in join_is_removable().
BTW, I doubt that the check of 'sjinfo->ojrelid != 0' in
remove_useless_joins() is needed. Since we've known that the join is
removable, I think we can just Assert that sjinfo->ojrelid cannot be 0.
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -91,8 +91,8 @@ restart:
/* Compute the relid set for the join we are considering */
joinrelids = bms_union(sjinfo->min_lefthand, sjinfo->min_righthand);
- if (sjinfo->ojrelid != 0)
- joinrelids = bms_add_member(joinrelids, sjinfo->ojrelid);
+ Assert(sjinfo->ojrelid != 0);
+ joinrelids = bms_add_member(joinrelids, sjinfo->ojrelid);
remove_rel_from_query(root, innerrelid, sjinfo, joinrelids);
We also do that in join_is_removable().
BTW, I doubt that the check of 'sjinfo->ojrelid != 0' in
remove_useless_joins() is needed. Since we've known that the join is
removable, I think we can just Assert that sjinfo->ojrelid cannot be 0.
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -91,8 +91,8 @@ restart:
/* Compute the relid set for the join we are considering */
joinrelids = bms_union(sjinfo->min_lefthand, sjinfo->min_righthand);
- if (sjinfo->ojrelid != 0)
- joinrelids = bms_add_member(joinrelids, sjinfo->ojrelid);
+ Assert(sjinfo->ojrelid != 0);
+ joinrelids = bms_add_member(joinrelids, sjinfo->ojrelid);
remove_rel_from_query(root, innerrelid, sjinfo, joinrelids);
3. I called the parameter removable_sjinfo because sjinfo is already
used within another loop, leading to a shadowed-parameter warning.
In a green field we'd probably have called the parameter sjinfo
and used another name for the loop's local variable, but that would
make the patch bulkier without adding anything. Haven't decided
whether to rename before commit or leave it as-is.
Personally I prefer to rename before commit but I'm OK with both ways.
Thanks
Richard
Thanks
Richard
pgsql-hackers by date: