On Tue, Apr 24, 2018 at 4:49 PM, Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> wrote: > (2018/04/17 19:00), Etsuro Fujita wrote: >> >> (2018/04/17 18:43), Ashutosh Bapat wrote: >>> >>> Here's updated patch-set. >> >> >> Will review. > > > I started reviewing this. > > o 0001-Handle-ConvertRowtypeExprs-in-pull_vars_clause.patch: > > + else if (IsA(node, ConvertRowtypeExpr)) > + { > +#ifdef USE_ASSERT_CHECKING > + ConvertRowtypeExpr *cre = castNode(ConvertRowtypeExpr, node); > + Var *var; > + > + /* > + * ConvertRowtypeExprs only result when parent's whole-row reference > is > + * translated for a child using adjust_appendrel_attrs(). That > function > + * does not handle any upper level Var references. > + */ > + while (IsA(cre->arg, ConvertRowtypeExpr)) > + cre = castNode(ConvertRowtypeExpr, cre->arg); > + var = castNode(Var, cre->arg); > + Assert (var->varlevelsup == 0); > +#endif /* USE_ASSERT_CHECKING */ > > Isn't it better to throw ERROR as in other cases in pull_vars_clause()?
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. > 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. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
expr_ref_error_pwj_v3.tar.gz
Description: GNU Zip compressed data