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: Fri, 27 Apr 2018 11:10:45 +0530
Message-ID: <CAFjFpRetD0aPMgw_mhOtJ11AqWJFCTo9dNJ758Zh4SPqgN1qvw@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]>
On Thu, Apr 26, 2018 at 5:36 PM, Etsuro Fujita
<[email protected]> wrote:
> (2018/04/25 18:51), Ashutosh Bapat wrote:
>>
>> On Tue, Apr 24, 2018 at 4:49 PM, Etsuro Fujita
>> <[email protected]> wrote:
>>>
>>> o 0001-Handle-ConvertRowtypeExprs-in-pull_vars_clause.patch:
>
>
>>> Another thing I noticed about this patch is this:
>>>
>>> postgres=# create table prt1 (a int, b int, c varchar) partition by range
>>> (a);
>>> postgres=# create table prt1_p1 partition of prt1 FOR VALUES FROM (0) TO
>>> (250);
>>> postgres=# create table prt1_p2 partition of prt1 FOR VALUES FROM (250)
>>> TO
>>> (500)
>>> ;
>>> postgres=# insert into prt1 select i, i % 25, to_char(i, 'FM0000') from
>>> generate
>>> _series(0, 499) i where i % 2 = 0;
>>> postgres=# analyze prt1;
>>> postgres=# create table prt2 (a int, b int, c varchar) partition by range
>>> (b);
>>> postgres=# create table prt2_p1 partition of prt2 FOR VALUES FROM (0) TO
>>> (250);
>>> postgres=# create table prt2_p2 partition of prt2 FOR VALUES FROM (250)
>>> TO
>>> (500)
>>> ;
>>> postgres=# insert into prt2 select i % 25, i, to_char(i, 'FM0000') from
>>> generate
>>> _series(0, 499) i where i % 3 = 0;
>>> postgres=# analyze prt2;
>>>
>>> postgres=# update prt1 t1 set c = 'foo' from prt2 t2 where t1::text =
>>> t2::text a
>>> nd t1.a = t2.b;
>>> ERROR: ConvertRowtypeExpr found where not expected
>>>
>>> To fix this, I think we need to pass PVC_RECURSE_CONVERTROWTYPES to
>>> pull_vars_clause() in distribute_qual_to_rels() and
>>> generate_base_implied_equalities_no_const() as well.
>>
>>
>> Thanks for the catch. I have updated patch with your suggested fix.
>
>
> Thanks, but I don't think that's enough. Consider:
>
> postgres=# create table base_tbl (a int, b int);
> postgres=# insert into base_tbl select i % 25, i from generate_series(0,
> 499) i where i % 6 = 0;
>
> postgres=# update prt1 t1 set c = 'foo' from prt2 t2 left join (select a, b,
> 1 from base_tbl) ss(c1, c2, c3) on (t2.b = ss.c2) where t1::text = t2::text
> and t1.a = t2.b;
> ERROR: ConvertRowtypeExpr found where not expected
>
Thanks for the test.
> To fix this, I think we need to pass PVC_RECURSE_CONVERTROWTYPES to
> pull_var_clause() in find_placeholders_in_expr() as well.
>
> The patch doesn't pass that to pull_var_clause() in other places such as
> fix_placeholder_input_needed_levels() or planner.c, but I don't understand
> the reason why that's OK.
I was trying to be conservative not to include
PVC_RECURSE_CONVERTROWTYPES everywhere. But it looks like because of
inheritance_planner() and partition-wise aggregate commit, we can have
ConvertRowtypeExprs almost everywhere. Added
PVC_RECURSE_CONVERTROWTYPEs in almost all the places except the ones
listed in 0001 patch.
>
> Sorry, I've not finished the review yet, so I'll continue that.
Thanks for the tests and review.
Here's updated patch set.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Attachments:
[application/x-gzip] expr_ref_error_pwj_v4.tar.gz (11.2K, ../CAFjFpRetD0aPMgw_mhOtJ11AqWJFCTo9dNJ758Zh4SPqgN1qvw@mail.gmail.com/2-expr_ref_error_pwj_v4.tar.gz)
download
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: <CAFjFpRetD0aPMgw_mhOtJ11AqWJFCTo9dNJ758Zh4SPqgN1qvw@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