public inbox for [email protected]  
help / color / mirror / Atom feed
From: Etsuro Fujita <[email protected]>
To: Ashutosh Bapat <[email protected]>
Cc: Robert Haas <[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, 06 Jul 2018 12:40:08 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAFjFpRdUppB6CNAy3ewvUpuwCSAuSm6JO_RwDPO0WWt_L9g6hg@mail.gmail.com>
References: <CAKcux6ktu-8tefLWtQuuZBYFaZA83vUzuRd7c1YHC-yEWyYFpg@mail.gmail.com>
	<[email protected]>
	<CAFjFpRczEuG9b0_j_qve2JAi-HHqS9_m1_yK5QVQa5=oDr5mJA@mail.gmail.com>
	<[email protected]>
	<CAFjFpRfkafBFJSEEkGwzLf=-y9Q8TUEGfpR=+K7-zCOw80R10w@mail.gmail.com>
	<CAFjFpRc8ZoDm0+zhx+MckwGyEqkOzWcpVqbvjaxwdGarZSNrmA@mail.gmail.com>
	<[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]>
	<[email protected]>
	<[email protected]>
	<CAFjFpRfVwaxfrRi8u5LmBX-zMhyzwYdGcj2VzpoxKYWLm9Y91A@mail.gmail.com>
	<[email protected]>
	<CAFjFpRdUppB6CNAy3ewvUpuwCSAuSm6JO_RwDPO0WWt_L9g6hg@mail.gmail.com>

(2018/07/05 22:04), Ashutosh Bapat wrote:
> On Thu, Jul 5, 2018 at 4:45 PM, Etsuro Fujita
> <[email protected]>  wrote:
>> 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:

> 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.

Actually, I don't think we can create such a Parallel Append, because in 
the case where we have scanjoin_target_parallel_safe=false, child paths 
would not be parallel-safe anymore as those paths are rewritten to have 
parallel-unsafe targets in the recursion of apply_scanjoin_target_to_paths.

> 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.

I agree that we might allow such an Append in future, but I'm not sure 
we should write code for that in PG11, because 1) that would make code 
complicated than necessary and 2) that might cause overlooking of 
unexpected behavior of the planner; if the planner created such an 
Append erroneously, we would run that by that code without noticing 
that.  To reduce the maintenance burden and the overlooking risk, I 
think it's better to extend code for such a plan when we allow that plan 
for a partitionwise join.

Best regards,
Etsuro Fujita




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

* 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