Re: postgres_fdw: wrong results with self join + enable_nestloop off

From: Önder Kalacı <onderkalaci(at)gmail(dot)com>
To: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Richard Guo <guofenglinux(at)gmail(dot)com>, Nishant Sharma <nishant(dot)sharma(at)enterprisedb(dot)com>, Suraj Kharage <suraj(dot)kharage(at)enterprisedb(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: postgres_fdw: wrong results with self join + enable_nestloop off
Date: 2023-08-21 13:34:24
Message-ID: CACawEhWKi5XgqHk+MvuMefdGjkfeBRyQNqWFiz-+tYR_fvERxw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Thanks for the explanation.

As described in the commit message, we assume that extensions use the
> hook in a similar way to FDWs

I'm not sure if it is fair to assume that extensions use any hook in any
way.

So my question is: does the Citus extension use the hook like this?
> (Sorry, I do not fully understand Onder's explanation.)
>
>
I haven't gone into detail about how Citus uses this hook, but I don't
think we should
need to explain it. In general, Citus uses many hooks, and many other
extensions
use this specific hook. With minor version upgrades, we haven't seen this
kind of
behavior change before.

In general, Citus relies on this hook for collecting information about
joins across
relations/ctes/subqueries. So, its scope is bigger than a single join for
Citus.

The extension assigns a special marker(s) for RTE Relations, and then
checks whether
all relations with these special markers joined transitively across
subqueries, such that
it can decide to pushdown the whole or some parts of the (sub)query.

I must admit, I have not yet looked into whether we can fix the problem
within the extension.
Maybe we can, maybe not.

But the bigger issue is that there has usually been a clear line between
the extensions and
the PG itself when it comes to hooks within the minor version upgrades.
Sadly, this change
breaks that line. We wanted to share our worries here and find out what
others think.

>Except that this was only noticed after it was released in a set of minor
> > versions, I would say that 6f80a8d9c should just straight up be reverted.

I cannot be the one to ask for reverting a commit in PG, but I think doing
it would be a
fair action. We kindly ask those who handle this to think about it.

Thanks,
Onder

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2023-08-21 15:33:45 Re: meson: pgxs Makefile.global differences
Previous Message Robert Haas 2023-08-21 13:32:27 Re: BUG #18059: Unexpected error 25001 in stored procedure