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: Thu, 19 Jul 2018 13:25:03 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAFjFpRdk-OqBtS7P3M89WFE8fUWWNyCNYOW2tzOBM4-xwu9-WQ@mail.gmail.com>
References: <CAKcux6ktu-8tefLWtQuuZBYFaZA83vUzuRd7c1YHC-yEWyYFpg@mail.gmail.com>
	<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]>
	<CAFjFpRfxsg3dXEDQPdUD+HYqE6v=jEYFiEtEgXuF+q+MVpA8eg@mail.gmail.com>
	<[email protected]>
	<CAFjFpRf-n75_mN=EFAvY-YY1xd_AXEDUdedW7sDX5poxpqs7fA@mail.gmail.com>
	<[email protected]>
	<CAFjFpReZqRx4FE1EZBBm4FGBem8u-8ijoPT=xcXsyfz5eTqpiw@mail.gmail.com>
	<[email protected]>
	<CAFjFpRdk-OqBtS7P3M89WFE8fUWWNyCNYOW2tzOBM4-xwu9-WQ@mail.gmail.com>

(2018/07/13 23:05), Ashutosh Bapat wrote:
> On Thu, Jul 12, 2018 at 4:32 PM, Etsuro Fujita
> <[email protected]>  wrote:
>> In this example, the value of the whole-row reference to the child table
>> ptp1 for that record is ('foo',1), and that of the index expression for that
>> record is (1,'foo').  Those have different column orders, but the latter
>> could be mapped to the former by a technique like do_convert_tuple.

> The expression in this case would look like ptp1::pt::ptp1 which won't
> match targetlist expression ptp1. I am also doubtful that the planner
> will be able to deduce that it need to apply an inverse function of
> ::pt and what exactly such an inverse function is. So index only scan
> won't be picked.

>> we could support index-only scans
>> with such an index in the case where we have the whole-row reference in the
>> targetlist, not the index expression itself.
>
> Can you please show an index only scan path being created in this case?

We currently don't consider index-only scan with index expressions, so I 
haven't thought in detail yet about how the planner would work.  But 
once we have that index-only scan, I think we could extend that to the 
case mentioned above, by adding this to the planner: if the index 
expression is of the form var::parenttype, consider that (not only the 
expression itself but) var can be returned from the index.  I think the 
expression like ptp1::pt::ptp1 would be useful to get the value of ptp1 
from the index at execution time.

>>> There's a patch in an adjacent thread started by David Rowley to rip
>>> out Append/MergeAppend when there is only one subplan. So, your
>>> solution won't work there.
>>
>>
>> Thanks for sharing that information!  I skimmed the thread.  I haven't yet
>> caught up with all the discussions there, so I think I'm missing something,
>> but it looks like that we haven't yet reached any consensus on the way to
>> go.  In my opinion, I like the approach mentioned in [1].  And if we go that
>> way, my patch seems to fit into that, because in that approach the
>> Append/MergeAppend could be removed after adjusting the targetlists for its
>> subplans in create_append_plan/create_merge_append_plan.  Anyway, I'd like
>> to join in that work for PG12.
>
> Whatever may be the outcome of that work, I think what we fix here
> shouldn't require to reverted in a few months from now, just so that
> that patch works.

I think we could add that optimization without reverting this change 
because the essential part of this change is to make create_plan adjust 
the tlists of the subplans based on the instruction stored into the 
subplans' RelOptInfos (ie, need_adjust_tlist in the second version of 
the patch).  I think this technique could be extended even to the case 
where we have that optimization.

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