public inbox for [email protected]
help / color / mirror / Atom feedFrom: Robert Haas <[email protected]>
To: Etsuro Fujita <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Ashutosh Bapat <[email protected]>
Cc: Rajkumar Raghuwanshi <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.
Date: Thu, 2 Aug 2018 15:19:49 -0400
Message-ID: <CA+TgmoZjYHtkw-o_4=-GXb6obF1epyivr+WnZ1SQCnU6fgtf+w@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CAFjFpRffA-BfL0RXcUVLLbcOBF1ie7oObTB-RE2q_o45X1HTYg@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CA+TgmoZboTszGQk3OecBOZUFNNjzKE-SOo_Q46xtTJ4Yj95-RQ@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CA+TgmobR9XWEhmF3s_WsoLNnnjaFyS_xtbrVG3dCGiUgof20Nw@mail.gmail.com>
<CA+TgmoZSaKq-fYALn5jf6c_X3==Rb2s8eqLDwGpV=LNNhTXYwg@mail.gmail.com>
<[email protected]>
<CA+TgmoYYTCQL=TUYbUz2b=CsQeB4o1L2+_=zHynxGVmyKUtH1A@mail.gmail.com>
<[email protected]>
<CA+TgmoazFzdp5ZVwUjk_b2cuB90WCeeWtzt1ShSbXGyg67c+Zw@mail.gmail.com>
<[email protected]>
<[email protected]>
<CA+TgmoZMdNiKUGV0=-QLV1f1-Xr0CDT+8y9wLktuEiQGkt64UQ@mail.gmail.com>
<[email protected]>
On Thu, Aug 2, 2018 at 7:20 AM, Etsuro Fujita
<[email protected]> wrote:
> The new approach seems to me more localized than what Ashutosh had proposed.
> One obvious advantage of the new approach is that we no longer need changes
> to postgres_fdw for it to work for partitionwise joins, which would apply
> for any other FDWs, making the FDW authors' life easy.
Well, I don't know what to say here expect that I don't agree. I
think Ashutosh's approach is a pretty natural extension of the system
that we have now. It involves needing to handle converted whole row
vars in some new places, most of which were handled in the original
patch, but postgres_fdw was missed. Your approach involves changing
the meaning of the target list, but only in narrow corner cases. I
completely disagree that we can say it's less invasive. It may indeed
be less work for extension authors, though, though perhaps at the
expense of moving the conversion from the remote server to the local
one.
>> But in general, with your approach, any code that
>> looks at the tlist for a child-join has to know that the tlist is to
>> be used as-is *except that* ConvertRowtypeExpr may need to be
>> inserted. I think that special case could be easy to overlook, and it
>> seems pretty arbitrary.
>
> I think we can check to see whether the conversion is needed, from the flags
> added to RelOptInfo.
Sure, I don't dispute that it can be made to work. I just think it's
an ugly kludge.
> I have to admit that it's weird to adjust the child's targetlist in that
> case when putting the child under the parent's Append/MergeAppend, but IMHO
> I think that would be much localized.
Yeah, I just don't agree at all.
Does anyone else want to weigh in on this? It seems to me that there
are several people who are quite willing to complain about the fact
that this hasn't been fixed, but not willing to express an opinion
about the shape of the fix. Either the RMT needs to take executive
action, or we need some more votes.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
view thread (115+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.
In-Reply-To: <CA+TgmoZjYHtkw-o_4=-GXb6obF1epyivr+WnZ1SQCnU6fgtf+w@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox