Message-ID: <5AE1C0D3.5060009@lab.ntt.co.jp>
Date: Thu, 26 Apr 2018 21:06:43 +0900
From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64;
 rv:11.0) Gecko/20120327 Thunderbird/11.0.1
MIME-Version: 1.0
Subject: Re: Expression errors with "FOR UPDATE" and postgres_fdw with
 partition
 wise join enabled.
References: 
 <CAKcux6ktu-8tefLWtQuuZBYFaZA83vUzuRd7c1YHC-yEWyYFpg@mail.gmail.com>
 <CAFjFpRfD5=LsOqG1eUf-vMpm9KoEZBBGJo68D9wkUBJxop_emQ@mail.gmail.com>
 <CAFjFpReD2rwzw64T5ZHS9+S8avREuazLu4nnCRAXToUN=WC+-Q@mail.gmail.com>
 <5AD5B1E0.8020404@lab.ntt.co.jp>
 <CAFjFpRcHQ9p0JEpx4eMhuNvD7GbyMYm1TykfCeBrNTdK+mSbgA@mail.gmail.com>
 <5AD5C5A1.1070509@lab.ntt.co.jp> <5ADF12D1.1030406@lab.ntt.co.jp>
 <CAFjFpRennScZ47UwYaE+3SB_KcGVToPmR2dOTEb7T7w2=j-jbQ@mail.gmail.com>
In-Reply-To: 
 <CAFjFpRennScZ47UwYaE+3SB_KcGVToPmR2dOTEb7T7w2=j-jbQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Cc: Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>,
        pgsql-hackers <pgsql-hackers@postgresql.org>
Precedence: bulk

(2018/04/25 18:51), Ashutosh Bapat wrote:
> On Tue, Apr 24, 2018 at 4:49 PM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.jp>  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

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.

Sorry, I've not finished the review yet, so I'll continue that.

Thanks for updating the patch!

Best regards,
Etsuro Fujita