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

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.
Date: 2018-07-11 07:53:27
Message-ID: 5B45B777.7010502@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(2018/07/10 19:58), Ashutosh Bapat wrote:
> On Tue, Jul 10, 2018 at 9:08 AM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> (2018/07/09 20:43), Ashutosh Bapat wrote:
>>> At the cost of having targetlist being type inconsistent. I don't have
>>> any testcase either to show that that's a problem in practice.

>> As I said before, I don't see any issue in having such a targetlist, but I
>> might be missing something, so I'd like to discuss a bit more about that.
>> Could you tell me the logic/place in the PG code where you think the problem
>> might occur. (IIRC, you mentioned something about that before (pathkeys? or
>> index-only scans?), but sorry, I don't understand that.)
>
> IIUC, index-only scan will be used when the all the required columns
> are covered by an index. If there is an index on the whole-row
> reference of the parent, it will be translated into a
> ConvertRowtypeExpr of the child when an index on the child is created.

I think so too.

> If the targetlist doesn't have ConvertRowtypeExpr, as your patch does,
> the planner won't be able to use such an index on the child table. But
> I couldn't create an index with a whole-row reference in it. So, I
> think this isn't possible right now.

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.

> Pathkey points to an equivalence class, which contains equivalence
> members. A parent equivalence class member containing a whole-row
> reference gets translated into a child equivalence member containing a
> ConvertRowtypeExpr.

I think so too.

> 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. So, I think it is safe to
have whole-row Vars instead of ConvertRowtypeExprs in the targetlist.

Thanks for the explanation!

Best regards,
Etsuro Fujita

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Taiki Kondo 2018-07-11 07:58:26 RE: Typo in Japanese translation of psql.
Previous Message Dave Page 2018-07-11 07:49:56 Re: How can we submit code patches that implement our (pending) patents?