Hi,
Thanks for the detailed explanation!
On Mon, Aug 21, 2023 at 10:34 PM Önder Kalacı <onderkalaci@gmail.com> wrote:
>> 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.
I am not sure either, but as for the hook, I think it is an undeniable
fact that the core system assumes that extensions will use it in that
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.
IIUC, I think that that is going beyond what the hook supports.
> 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.
My understanding is: at least for hooks with intended usages, if an
extension uses them as intended, it is guaranteed that the extension
as-is will work correctly with minor version upgrades; otherwise it is
not necessarily. I think it is unfortunate that my commit broke the
Citus extension, though.
>> >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.
Reverting the commit would resolve your issue, but re-introduce the
issue mentioned upthread to extensions that use the hook properly, so
I do not think that reverting the commit would be a fair action.
Sorry for the delay.
Best regards,
Etsuro Fujita