| From: | Alena Rybakina <lena(dot)ribackina(at)yandex(dot)ru> |
|---|---|
| To: | Peter Petrov <pspetrov91(at)gmail(dot)com>, Ilia Evdokimov <ilya(dot)evdokimov(at)tantorlabs(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, solaimurugan vellaipandiyan <drsolaimurugan(dot)v(at)gmail(dot)com> |
| Subject: | Re: pull-up subquery if JOIN-ON contains refs to upper-query |
| Date: | 2026-09-24 16:05:53 |
| Message-ID: | e522e944-bb6c-4f3a-a54f-e540c31e7458@yandex.ru |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Peter,
Thank you for the review, and sorry for the long delay.
You were right. While checking your comments I found that v7 gives
wrong results with outer joins, for example:
create table o(x int, y int); insert into o values (1,1),(2,2),(3,3);
create table a(x int, y int); insert into a values (1,10),(5,50);
create table b(x int, y int); insert into b values (7,70);
select * from o where exists
(select 1 from a left join b on a.x = o.x);
It should return all three rows, but v7 returns only one.
So I rewrote the patch the way you suggested. v8 is attached.
On Fri, May 8, 2026 at 8:45 PM Peter Petrov <pspetrov91(at)gmail(dot)com> wrote:
> We need to separate two things: the jointree and the WHERE clause, so
> it's possible to do something like this
> ...
> if (contain_vars_of_level((Node *) subselect, 1))
> return NULL;
Done, almost as you wrote. The jointree is taken out of the sub-select,
and the rest of it must not refer to the parent query. Then the jointree
is put back and checked by the new walker.
> 2) We don't need to use get_relids_in_jointree() and nullable_above
> ...
> I fear that you don't check FULL JOINS here.
Agreed, I was wrong about this in my last mail. The new code walks the
jointree from the top down with an is_nullable_side flag, and handles
FULL JOIN. A correlated qual is moved up only if it is in the ON clause
of an inner join that is not on the nullable side of an outer join.
Otherwise the sublink is not pulled up.
> 3) ... I am not sure that the mutator is a good name here.
Agreed. The mutator and HoistJoinQualsContext are gone. Now it is a
simple walker that collects the joins whose quals should be moved.
> 4) After checking the WHERE clause and the jointree we can traverse our
> list, make a new whereClause by appending quals with outer references
Done, with make_and_qual().
> 5) I have also noticed that you are using canonicalize_qual()
Removed.
> 6) I have noticed the new output from one regression test.
That was a bug: the ON clauses of the left joins were replaced with
"true". v8 doesn't change join.out.
> 7) There is a SubLink which won't be pulled up
Right, the subqueries in FROM are not flattened yet when
pull_up_sublinks() runs, so the reference to A looks like a lateral
reference. I agree with you that this is a separate change, so I'd
like to leave it for later.
Other changes:
- Rebased on current master.
- Removed the tests that didn't check anything new and added tests
for outer joins, including the case above.
- Rewrote the commit message.
--
Regards,
Alena Rybakina
Yandex
| Attachment | Content-Type | Size |
|---|---|---|
| v8-0001-Allow-pulling-up-EXISTS-sublinks-with-correlated-.patch | text/plain | 24.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-09-24 16:09:45 | Re: merge-delete isolation test fails since 85f55534e80 |
| Previous Message | Alexander Korotkov | 2026-09-24 15:49:34 | Re: JSON_TABLE: table => column ON ERROR propagation |