public inbox for [email protected]  

help / color / mirror / Atom feed

From: 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]>
Subject: Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.
Date: Fri, 20 Jul 2018 08:38:09 -0400
Message-ID: <CA+TgmobR9XWEhmF3s_WsoLNnnjaFyS_xtbrVG3dCGiUgof20Nw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAFjFpRfGL9w-4C5N-ioaG3m+iheC1ey7_RMZ4vJijGn=wWJyxw@mail.gmail.com>
	<[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]>

On Wed, Jul 18, 2018 at 8:00 AM, Etsuro Fujita
<[email protected]> wrote:
> [ new patch ]

+ /*
+ * If the child plan is an Append or MergeAppend, the targetlists of its
+ * subplans would have already been adjusted before we get here, so need
+ * to work here.
+ */
+ if (IsA(subplan, Append) || IsA(subplan, MergeAppend))
+ return;
+
+ /* The child plan should be a scan or join */
+ Assert(is_projection_capable_plan(subplan));

This looks like a terrible design to me.  If somebody in future
invents a new plan type that is not projection-capable, then this
could fail an assertion here and there won't be any simple fix.  And
if you reach here and the child targetlists somehow haven't been
adjusted, then you won't even fail an assertion, you'll just crash (or
maybe return wrong answers).

I'm going to study this some more now, but I really think this is
going in the wrong direction.

-- 
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]
  Subject: Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.
  In-Reply-To: <CA+TgmobR9XWEhmF3s_WsoLNnnjaFyS_xtbrVG3dCGiUgof20Nw@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