public inbox for [email protected]  

help / color / mirror / Atom feed

From: 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: Tue, 24 Apr 2018 20:19:45 +0900
Message-ID: <[email protected]> (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]>

(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()? 
  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.

That's all I have for now.  Will continue the review.

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