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

(2018/07/05 22:04), Ashutosh Bapat wrote:
> On Thu, Jul 5, 2018 at 4:45 PM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> 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