MIME-Version: 1.0
In-Reply-To: <5B28FB19.3090809@lab.ntt.co.jp>
References: 
 <CAKcux6ktu-8tefLWtQuuZBYFaZA83vUzuRd7c1YHC-yEWyYFpg@mail.gmail.com>
 <5AE84FBF.3040905@lab.ntt.co.jp>
 <CAFjFpRdSvXxOkSMc=jEHfTVGnCFgziLA7m+onp=ak=Tfa_V1zA@mail.gmail.com>
 <5AF2E09F.9060208@lab.ntt.co.jp>
 <CAFjFpRcFucgvrzEFNi10g00-va640poSmJDmBrZNfFarB6eP-Q@mail.gmail.com>
 <5AF440C2.3040805@lab.ntt.co.jp>
 <CAFjFpRczEuG9b0_j_qve2JAi-HHqS9_m1_yK5QVQa5=oDr5mJA@mail.gmail.com>
 <5AF59415.10309@lab.ntt.co.jp>
 <CAFjFpRfkafBFJSEEkGwzLf=-y9Q8TUEGfpR=+K7-zCOw80R10w@mail.gmail.com>
 <CAFjFpRc8ZoDm0+zhx+MckwGyEqkOzWcpVqbvjaxwdGarZSNrmA@mail.gmail.com>
 <5AFC0865.8050802@lab.ntt.co.jp>
 <CAFjFpRfGL9w-4C5N-ioaG3m+iheC1ey7_RMZ4vJijGn=wWJyxw@mail.gmail.com>
 <5AFD6580.5090308@lab.ntt.co.jp> <5AFE81CC.8000508@lab.ntt.co.jp>
 <5B17C5EC.1090602@lab.ntt.co.jp>
 <CAFjFpRffA-BfL0RXcUVLLbcOBF1ie7oObTB-RE2q_o45X1HTYg@mail.gmail.com>
 <5B1E5071.1000704@lab.ntt.co.jp> <5B23A964.9020800@lab.ntt.co.jp>
 <5B28FB19.3090809@lab.ntt.co.jp>
From: Robert Haas <robertmhaas@gmail.com>
Date: Fri, 22 Jun 2018 10:58:28 -0400
Message-ID: 
 <CA+TgmoZboTszGQk3OecBOZUFNNjzKE-SOo_Q46xtTJ4Yj95-RQ@mail.gmail.com>
Subject: Re: Expression errors with "FOR UPDATE" and postgres_fdw with
 partition wise join enabled.
To: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
Cc: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>,
	Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Jun 19, 2018 at 8:46 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
> Here is a patch for that.
>
> * As I said upthread, the patch makes code much more simple; I removed all
> the changes to setrefs.c added by the partitionwise-join patch.  I also
> simplified the logic for building a tlist for a child-join rel. The original
> PWJ computes attr_needed data even for child rels, and build the tlist for a
> child-join by passing to build_joinrel_tlist that data for input child rels
> for the child-join.  But I think that's redundant, and it's more
> straightforward to apply adjust_appendrel_attrs to the parent-join's tlist
> to get the child-join's tlist.  So, I changed that way, which made
> unnecessary all the changes to build_joinrel_tlist and placeholder.c added
> by the PWJ patch, so I removed those as well.
>
> * The patch contains all of the regression tests in the original patch
> proposed by Ashutosh.

I think this approach is going to run into trouble if the level at
which we have to apply the ConvertRowTypeExpr happens not to be a
projection-capable node.  And, in general, it seems to me that we want
to produce the right outputs at the lowest possible level of the plan
tree.  For instance, suppose that one of the relations being scanned
is not parallel-safe but the others are.  Then, we could end up with a
plan like this:

Append
-> Seq Scan on temp_rela
-> Gather
  -> Parallel Seq Scan on thing1
-> Gather
  -> Parallel Seq Scan on thing2

If a projection is required to convert the row type expression, we
certainly want that to get pushed below the Gather, not to happen at
the Gather level itself.  We certainly don't want it to happen at the
Append level, which can't even handle it.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company