Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.
Date: 2018-07-20 12:38:09
Message-ID: CA+TgmobR9XWEhmF3s_WsoLNnnjaFyS_xtbrVG3dCGiUgof20Nw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jul 18, 2018 at 8:00 AM, Etsuro Fujita
<fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Victor Wagner 2018-07-20 12:38:20 Non-portable shell code in pg_upgrade tap tests
Previous Message Robert Haas 2018-07-20 12:27:34 Re: Faster str to int conversion (was Table with large number of int columns, very slow COPY FROM)