public inbox for [email protected]
help / color / mirror / Atom feedFrom: 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: Mon, 11 Jun 2018 19:35:29 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAFjFpRffA-BfL0RXcUVLLbcOBF1ie7oObTB-RE2q_o45X1HTYg@mail.gmail.com>
References: <CAKcux6ktu-8tefLWtQuuZBYFaZA83vUzuRd7c1YHC-yEWyYFpg@mail.gmail.com>
<CAFjFpRennScZ47UwYaE+3SB_KcGVToPmR2dOTEb7T7w2=j-jbQ@mail.gmail.com>
<[email protected]>
<CAFjFpRetD0aPMgw_mhOtJ11AqWJFCTo9dNJ758Zh4SPqgN1qvw@mail.gmail.com>
<[email protected]>
<CAFjFpRdSvXxOkSMc=jEHfTVGnCFgziLA7m+onp=ak=Tfa_V1zA@mail.gmail.com>
<[email protected]>
<CAFjFpRcFucgvrzEFNi10g00-va640poSmJDmBrZNfFarB6eP-Q@mail.gmail.com>
<[email protected]>
<CAFjFpRczEuG9b0_j_qve2JAi-HHqS9_m1_yK5QVQa5=oDr5mJA@mail.gmail.com>
<[email protected]>
<CAFjFpRfkafBFJSEEkGwzLf=-y9Q8TUEGfpR=+K7-zCOw80R10w@mail.gmail.com>
<CAFjFpRc8ZoDm0+zhx+MckwGyEqkOzWcpVqbvjaxwdGarZSNrmA@mail.gmail.com>
<[email protected]>
<CAFjFpRfGL9w-4C5N-ioaG3m+iheC1ey7_RMZ4vJijGn=wWJyxw@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected] l.com>
(2018/06/07 19:42), Ashutosh Bapat wrote:
> On Wed, Jun 6, 2018 at 5:00 PM, Etsuro Fujita
> <[email protected]> wrote:
>> Since I'm not 100% sure that that is the right way to go, I've been
>> rethinking how to fix this issue. Yet another idea I came up with to fix
>> this is to redesign the handling of the tlists for children in the
>> partitioning case. Currently, we build the reltarget for a child by
>> applying adjust_appendrel_attrs to the reltarget for its parent in
>> set_append_rel_size, which maps a wholerow Var referring to the parent rel
>> to a ConvertRowtypeExpr that translates a wholerow of the child rel into a
>> wholerow of the parent rel's rowtype. This works well for the
>> non-partitioned inheritance case, but makes complicated the code for
>> handling the partitioning case especially when planning partitionwise-joins.
>> And I think this would be the root cause of this issue.
>
> Although true, this is not only about targetlist. Even the whole-row
> expressions in the conditions, equivalence classes and other
> planner/optimizer data structures are translated to
> ConvertRowtypeExpr.
Yeah, but I mean we modify set_append_rel_size so that we only map a
parent wholerow Var in the parent tlist to a child wholerow Var in the
child's tlist; parent wholerow Vars in the parent's other expressions
such as conditions are transformed into CREs as-is.
>> I don't think the
>> tlists for the children need to have their wholerows transformed into the
>> corresponding ConvertRowtypeExprs *at this point*, so what I'd like to
>> propose is to 1) map a parent wholerow Var simply to a child wholerow Var,
>> instead (otherwise, the same as adjust_appendrel_attrs), when building the
>> reltarget for a child in set_append_rel_size, 2) build the tlists for child
>> joins using those children's wholerow Vars at path creation time, and 3)
>> adjust those wholerow Vars in the tlists for subpaths in the chosen
>> AppendPath so that those are transformed into the corresponding
>> ConvertRowtypeExprs, at plan creation time (ie, in
>> create_append_plan/create_merge_append_plan). IIUC, this would not require
>> any changes to pull_var_clause as proposed by the patch. This would not
>> require any changes to postgres_fdw as proposed by the patch, either. In
>> addition, this would make unnecessary the code added to setrefs.c by the
>> partitionwise-join patch. Maybe I'm missing something though.
>
> Not translating whole-row expressions to ConvertRowtypeExpr before
> creating paths can lead to a number of anomalies. For example,
>
> 1. if there's an index on the whole-row expression of child,
> translating parent's whole-row expression to child's whole-row
> expression as is will lead to using that index, when in reality the it
> can not be used since the condition/ORDER BY clause (which originally
> referred the parent's whole-row expression) require the child's
> whole-row reassembled as parent's whole-row.
> 2. Constraints on child'd whole-row expression, will be used to prune
> a child when they can not be used since the original condition was on
> parent' whole-row expression and not that of the child.
> 3. Equivalence classes will think that a child whole-row expression
> (without ConvertRowtypeExpr) is equivalent to an expression which is
> part of the same equivalence class as the parent' whole-row
> expression.
Is that still possible when we only map a parent wholerow Var in the
parent's tlist to a child wholerow Var in the child's tlist?
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