public inbox for [email protected]  

help / color / mirror / Atom feed

From: Etsuro Fujita <[email protected]>
To: Ashutosh Bapat <[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: Wed, 11 Jul 2018 16:53:27 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAFjFpRfxsg3dXEDQPdUD+HYqE6v=jEYFiEtEgXuF+q+MVpA8eg@mail.gmail.com>
References: <CAKcux6ktu-8tefLWtQuuZBYFaZA83vUzuRd7c1YHC-yEWyYFpg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAFjFpRffA-BfL0RXcUVLLbcOBF1ie7oObTB-RE2q_o45X1HTYg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAFjFpRedQBySj5Vm-eq+gATP6sqJJfXzPrV+Ld+K6DnaOGkFAQ@mail.gmail.com>
	<[email protected]>
	<CAFjFpRf55MT-9NnaRwp0wkDUTuwb++UA_un9Twai=qNVePh-TQ@mail.gmail.com>
	<[email protected]>
	<CAFjFpRevODMmZ8MF3oLsXr9mSH9-mWC_fxmPWTYCSqHVr_Zwqg@mail.gmail.com>
	<[email protected]>
	<CAFjFpRf2vd0C6B8g1TmpJN8RkPECgZy8iGe9UYZTy9EWu1eL2Q@mail.gmail.com>
	<[email protected]>
	<CAFjFpRcwrESJBDkmm1kvtV234nkw=dr9nJ=RWJ4pmsVQqF4avg@mail.gmail.com>
	<[email protected]>
	<CAFjFpRf-t_dM=qirB5+95wT1hSCLs8mQqbfRSKP=xUuhnOhbOQ@mail.gmail.com>
	<[email protected]>
	<[email protected] .com>

(2018/07/10 19:58), Ashutosh Bapat wrote:
> On Tue, Jul 10, 2018 at 9:08 AM, Etsuro Fujita
> <[email protected]>  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

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