public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ashutosh Bapat <[email protected]>
To: Etsuro Fujita <[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, 10 May 2018 09:34:33 +0530
Message-ID: <CAFjFpRcFucgvrzEFNi10g00-va640poSmJDmBrZNfFarB6eP-Q@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAKcux6ktu-8tefLWtQuuZBYFaZA83vUzuRd7c1YHC-yEWyYFpg@mail.gmail.com>
<CAFjFpRfD5=LsOqG1eUf-vMpm9KoEZBBGJo68D9wkUBJxop_emQ@mail.gmail.com>
<CAFjFpReD2rwzw64T5ZHS9+S8avREuazLu4nnCRAXToUN=WC+-Q@mail.gmail.com>
<[email protected]>
<CAFjFpRcHQ9p0JEpx4eMhuNvD7GbyMYm1TykfCeBrNTdK+mSbgA@mail.gmail.com>
<[email protected]>
<[email protected]>
<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]>
On Wed, May 9, 2018 at 5:20 PM, Etsuro Fujita
<[email protected]> wrote:
>
> (2018/04/25 18:51), Ashutosh Bapat wrote:
>> Actually I noticed that ConvertRowtypeExpr are used to cast a child's
>> whole row reference expression (not just a Var node) into that of its
>> parent and not. For example a cast like NULL::child::parent produces a
>> ConvertRowtypeExpr encapsulating a NULL constant node and not a Var
>> node. We are interested only in ConvertRowtypeExprs embedding Var
>> nodes with Var::varattno = 0. I have changed this code to use function
>> is_converted_whole_row_reference() instead of the above code with
>> Assert. In order to use that function, I had to take it out of
>> setrefs.c and put it in nodeFuncs.c which seemed to be a better fit.
>
> This change seems a bit confusing to me because the flag bits
> "PVC_INCLUDE_CONVERTROWTYPES" and "PVC_RECURSE_CONVERTROWTYPES" passed to
> pull_var_clause look as if it handles any ConvertRowtypeExpr nodes from a
> given clause, but really, it only handles ConvertRowtypeExprs you mentioned
> above. To make that function easy to understand and use, I think it'd be
> better to use the IsA(node, ConvertRowtypeExpr) test as in the first version
> of the patch, instead of is_converted_whole_row_reference, which would be
> more consistent with other cases such as PlaceHolderVar.
I agree that using is_converted_whole_row_reference() is not
consistent with the other nodes that are handled by pull_var_clause().
I also agree that PVC_*_CONVERTROWTYPES doesn't reflect exactly what's
being done with those options. But using
is_converted_whole_row_reference() is the correct thing to do since we
are interested only in the whole-row references embedded in
ConvertRowtypeExpr. There can be anything encapsulated in
ConvertRowtypeExpr(), a non-shippable function for example. We don't
want to try to push that down in postgres_fdw's case. Neither in other
cases. So, it boils down to changing names of PVC_*_CONVERTROWTYPES to
something more meaningful. How about PVC_*_CONVERTWHOLEROWS?
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
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: <CAFjFpRcFucgvrzEFNi10g00-va640poSmJDmBrZNfFarB6eP-Q@mail.gmail.com>
* 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