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

Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>

From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Cc: Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2018-07-12T03:32:03Z
Lists: pgsql-hackers
(2018/07/11 20:02), Ashutosh Bapat wrote:
> On Wed, Jul 11, 2018 at 1:23 PM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.jp>  wrote:
>> Actually, even if we could create such an index on the child table and the
>> targetlist had the ConvertRowtypeExpr, the planner would still not be able
>> to use an index-only scan with that index; because check_index_only would
>> not consider that an index-only scan is possible for that index because that
>> index is an expression index and that function currently does not consider
>> that index expressions are able to be returned back in an index-only scan.
>> That behavior of the planner might be improved in future, though.

> Right and when we do so, not having ConvertRowtypeExpr in the
> targetlist will be a problem.

Yeah, but I don't think that that's unsolvable; because in that case the 
CRE as an index expression could be converted back to the whole-row 
Var's rowtype by adding another CRE to the index expression for that 
conversion, I suspect that that special handling could allow us to 
support an index-only scan even when having the whole-row Var instead of 
the CRE in the targetlist.  (Having said that, I'm not 100% sure we need 
to solve that problem when we improve the planner, because there doesn't 
seem to me to be enough use-case to justify making the code complicated 
for that.)  Anyway, I think that that would be a matter of future 
versions of PG.

>>> At places in planner we match equivalence members
>>> to the targetlist entries. This matching will fail unexpectedly when
>>> ConvertRowtypeExpr is removed from a child's targetlist. But again I
>>> couldn't reproduce a problem when such a mismatch arises.
>>
>>
>> IIUC, I don't think the planner assumes that for an equivalence member there
>> is an matching entry for that member in the targetlist; what I think the
>> planner assumes is: an equivalence member is able to be computed from
>> expressions in the targetlist.
>
> This is true. However,
>
>>   So, I think it is safe to have whole-row
>> Vars instead of ConvertRowtypeExprs in the targetlist.
>
> when it's looking for an expression, it finds a whole-row expression
> so it think it needs to add a ConvertRowtypeExpr on that. But when the
> plan is created, there is ConvertRowtypeExpr already, but there is no
> way to know that a new ConvertRowtypeExpr is not needed anymore. So,
> we may have two ConvertRowtypeExprs giving wrong results.

Maybe I'm missing something, but I don't think that we need to worry 
about that, because in the approach I proposed, we only add CREs above 
whole-row Vars in the targetlists for subplans of an Append/MergeAppend 
for a partitioned relation at plan creation time.

Best regards,
Etsuro Fujita


Commits

  1. Adjust EXPLAIN's output for disabled nodes

  2. Disable support for partitionwise joins in problematic cases.

  3. Rewrite the code that applies scan/join targets to paths.