MIME-Version: 1.0
In-Reply-To: <5B3DFDDE.2020803@lab.ntt.co.jp>
References: 
 <CAKcux6ktu-8tefLWtQuuZBYFaZA83vUzuRd7c1YHC-yEWyYFpg@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> <5B39F47B.10707@lab.ntt.co.jp>
 <5B3C9C33.6030203@lab.ntt.co.jp>
 <CAFjFpRfVwaxfrRi8u5LmBX-zMhyzwYdGcj2VzpoxKYWLm9Y91A@mail.gmail.com>
 <5B3DFDDE.2020803@lab.ntt.co.jp>
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Thu, 5 Jul 2018 18:34:46 +0530
Message-ID: 
 <CAFjFpRdUppB6CNAy3ewvUpuwCSAuSm6JO_RwDPO0WWt_L9g6hg@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: Robert Haas <robertmhaas@gmail.com>,
	Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jul 5, 2018 at 4:45 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
>
>
> In the case where scanjoin_target_parallel_safe=false, we actually will
> consider such parallel paths using the existing partial paths for the parent
> appendrel in the code path shown in a previous email (note: we would already
> have done generate_partitionwise_join_paths or set_append_rel_pathlist for
> the parent appendrel in query_planner.)  For such a parallel path, we might
> need to do a projection in the Gather node for generating the SRF-free
> scan/join target, but I think that would be still efficient.  So, I'm not
> sure that we really need to create child Gathers and generate an Append with
> those Gathers, as you mentioned.

I don't think it will be Append of Gathers, but Append where one of
the subplans is Gather. I don't think that possibility is completely
eliminated.

>
> Another thing I noticed is: actually, we don't produce an Append with child
> Gathers in apply_scanjoin_target_to_paths, which I thought we would do that
> in the case of scanjoin_target_parallel_safe=false, but I noticed I was
> wrong.  Sorry for that.  The reason is because in that case, even if we
> create new partial Append paths with child Gathers, we don't run
> generate_gatehr_paths for the newly created partial paths at the end of that
> function shown below, since the parent's consider_parallel flag is set to
> false in that case:
>
>     /*
>      * Consider generating Gather or Gather Merge paths.  We must only do
> this
>      * if the relation is parallel safe, and we don't do it for child rels
> to
>      * avoid creating multiple Gather nodes within the same plan. We must do
>      * this after all paths have been generated and before set_cheapest,
> since
>      * one of the generated paths may turn out to be the cheapest one.
>      */
>     if (rel->consider_parallel && !IS_OTHER_REL(rel))
>         generate_gather_paths(root, rel, false);

Hmm. I don't think that's a great idea since such a plan might turn
out cheaper esp. when there are very few children which could use
parallel query and parallel append is possible at the top parent. But
anyway, that's what it is today. But I think, we shouldn't write code
assuming that an Append will never see a Gather below it. We might see
some plans like that in future and need to change your code.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company