Re: BUG #18429: Inconsistent results on similar queries with join lateral
От | Richard Guo |
---|---|
Тема | Re: BUG #18429: Inconsistent results on similar queries with join lateral |
Дата | |
Msg-id | CAMbWs49KeppzynvCV-0-DFp5w=ih8ZcDQkCvYJHSMe6rATjnwQ@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: BUG #18429: Inconsistent results on similar queries with join lateral (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: BUG #18429: Inconsistent results on similar queries with join lateral
|
Список | pgsql-bugs |
On Mon, Apr 15, 2024 at 12:44 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Richard Guo <guofenglinux@gmail.com> writes:
> Now that we've learned that join_clause_is_movable_into's heuristic
> about physically referencing the target rel can fail for EC-derived
> clauses, I'm kind of concerned that we may end up with duplicate clauses
> in the final plan, since we do not check EC-derived clauses against
> join_clause_is_movable_into in get_baserel_parampathinfo while we do in
> create_nestloop_path. What if we have an EC-derived clause that in
> get_baserel_parampathinfo it is put into ppi_clauses while in
> create_nestloop_path it does not pass the movability checking? Is it
> possible to occur, or is it just my illusion?
I'm not sure either, but it certainly seems like a hazard. Also,
get_joinrel_parampathinfo is really depending on getting consistent
results from join_clause_is_movable_into to assign clauses to the
correct join level. So after sleeping on it I think that "the
results of generate_join_implied_equalities should pass
join_clause_is_movable_into" is an invariant we don't really want to
let go of, meaning that it would be a good idea to fix equivclass.c
to ensure that's true in these child-rel corner cases. That's not
very hard, requiring just a small hack in create_join_clause, as
attached. It's not that much of a hack either because there are
other places in equivclass.c that force the relid sets for child
expressions to be more than what pull_varnos would conclude (search
for comments mentioning pull_varnos).
Having done that, we can add code in HEAD/v16 to assert that
join_clause_is_movable_into is true here, while in the older branches
we can use it to filter rather than klugily checking nullable_relids
directly. So I end with the attached two patches.
+1 to both patches.
I didn't include the new test case in the HEAD/v16 patch; since it
doesn't represent a live bug for those branches I felt like maybe
it's not worth the test cycles going forward. But there's certainly
room for the opposite opinion. What do you think?
I agree that the new test case for v15 does not seem to be worth
including in v16+. It seems to me that it would be better if we can
have another new test case to verify that we've included child rel's
em_relids even for appendrel child relations with pseudoconstant
translated variables, i.e. to verify that the change in equivclass.c
takes effect. Maybe with a query like below:
explain (costs off)
select * from tenk1 t1
left join lateral
(select t1.unique1 as t1u, 0 as c
union all
select t1.unique1 as t1u, 1 as c) s on true
where t1.unique1 = s.c;
Without the change in equivclass.c, this query would trigger the new
added assert in get_baserel_parampathinfo for v16, and give a wrong plan
for v15. What do you think?
Thanks
Richard
including in v16+. It seems to me that it would be better if we can
have another new test case to verify that we've included child rel's
em_relids even for appendrel child relations with pseudoconstant
translated variables, i.e. to verify that the change in equivclass.c
takes effect. Maybe with a query like below:
explain (costs off)
select * from tenk1 t1
left join lateral
(select t1.unique1 as t1u, 0 as c
union all
select t1.unique1 as t1u, 1 as c) s on true
where t1.unique1 = s.c;
Without the change in equivclass.c, this query would trigger the new
added assert in get_baserel_parampathinfo for v16, and give a wrong plan
for v15. What do you think?
Thanks
Richard
В списке pgsql-bugs по дате отправления: