Re: [Postgres-xc-bugs] Query are shipped to the data nodes more than needed.

SourceForge Headquarters 1320 Columbia Street Suite 310 San Diego, CA 92101 +1 (858) 422-6466

Ok, good observation.

BTW, with your fix, we are still in problem if there are four conditions
ANDED e.g. A and B and C and D.

What should happen, and which actually happens before starting the planning
phase, is all the ANDs are flattened into a list so that the above tree
looks like
AND
 | -> args A, B, C, D

So, a better solution seems to be that we should pass the quals to
pgxc_find_dist_qual(), by converting them into and ANDed list of args.

On Mon, Jan 13, 2014 at 2:43 PM, ZhangJulian <jul...@ou...>wrote:

> Hi Ashutosh，
>
> The parser will parse "where c1=1 and c2=1 and c3=1" to:
> BoolExpr
>     |--BoolExpr
>     |        |-OpExpr (c1=1)
>     |        |-OpExpr (c2=1)
>     |
>     |--OpExpr (c3=1)
>
> make_ands_implicit() will translate it to a list containing two elements:
> 1. BoolExpr
>           |-OpExpr (c1=1)
>           |-OpExpr (c2=1)
>
>  2. OpExpr (c3=1)
> But it will not further split the first element to two OpExpr and add them
> to the parent list.
>
> Thanks
> Julian
>
> ------------------------------
> Date: Mon, 13 Jan 2014 14:19:48 +0530
>
> Subject: Re: [Postgres-xc-bugs] Query are shipped to the data nodes more
> than needed.
> From: ash...@en...
> To: jul...@ou...
> CC: pos...@li...
>
> Hi Julian,
> Can you please send a patch after taking care of my comments in my
> previous mail. I am pasting them here for your quick reference,
> --
> In function pgxc_find_distcol_expr() there is code
>
> 828     /* Convert the qualification into List if it's not already so */
> 829     if (!IsA(quals, List))
> 830         lquals = make_ands_implicit((Expr *)quals);
> 831     else
> 832         lquals = (List *)quals;
>
> which takes care of the quals which are not in the List form. This part of
> the code should have taken care of the case you mentioned. It seems that
> for some reason, this part is not converting the ANDed quals into List. Can
> you please check why? May be you want to step through make_ands_implicit()
> (using gdb or some debugger) to see what's happening there.
>
>
>
>
> On Mon, Jan 13, 2014 at 2:16 PM, ZhangJulian <jul...@ou...>wrote:
>
> Hi Ashutosh,
>
> I am so sorry that I had sent a patch with wrong format, it is because I
> edited it after Git Diff.
> I regenerated a patch as the attached file and verified that it can be
> applied.
>
> Thanks
> Julian
>
> ------------------------------
> From: jul...@ou...
> To: ash...@en...
> Date: Sat, 11 Jan 2014 07:52:34 +0800
> CC: pos...@li...
>
> Subject: Re: [Postgres-xc-bugs] Query are shipped to the data nodes more
> than needed.
>
> Hi Ashutosh,
>
> I am at home and can not connect to the office to debug it today. I will
> double check the problem on Monday.
>
> I remember when I debugged it, the proplem is,
> Parser will parse "where c1=1 and c2=1 and c3=1" to  "WHERE (((c1 = 1)
> AND (c2 = 1)) AND (c3 = 1))", which is a list containing two elements:
> element 1:  ((c1 = 1) AND (c2 = 1)) which is a BoolExpr;
> element 2:  (c3 = 1)
> For the element 1, it would not split it into two simple predidates, so
> the c1 = 1 is missed.
>
> If I just move the c1 = 1 to the last position, the query can generate the
> correct plan.
>
> Thanks
> Julian
>
> ------------------------------
> Date: Fri, 10 Jan 2014 16:48:47 +0530
> Subject: Re: [Postgres-xc-bugs] Query are shipped to the data nodes more
> than needed.
> From: ash...@en...
> To: jul...@ou...
> CC: pos...@li...
>
> Hi Julian,
> I looked at the patch and tried to apply the patch, but there is an error
> [ashutosh@ubuntu coderoot]git apply
> /mnt/hgfs/tmp/20140110_more_datanode_involved.patch
> /mnt/hgfs/tmp/20140110_more_datanode_involved.patch:10: trailing
> whitespace.
>         if (and_clause((Node *) qual_expr))
> /mnt/hgfs/tmp/20140110_more_datanode_involved.patch:11: trailing
> whitespace.
>         {
> /mnt/hgfs/tmp/20140110_more_datanode_involved.patch:12: trailing
> whitespace.
>             List *sub_lquals = ((BoolExpr *) qual_expr)->args;
> /mnt/hgfs/tmp/20140110_more_datanode_involved.patch:13: trailing
> whitespace.
>             distcol_expr = pgxc_find_distcol_expr(varno, attrNum,
> sub_lquals);
> /mnt/hgfs/tmp/20140110_more_datanode_involved.patch:14: trailing
> whitespace.
>             if (distcol_expr != NULL)
> fatal: corrupt patch at line 20
>
> Did you use git diff for creating the patch?
>
> BTW, I looked at the patch. The patch seems to be applied to function
> pgxc_find_distcol_expr() (functions name usually appears along with the
> diff, if you are using git diff).
>
> In this function there is code
>
> 828     /* Convert the qualification into List if it's not already so */
> 829     if (!IsA(quals, List))
> 830         lquals = make_ands_implicit((Expr *)quals);
> 831     else
> 832         lquals = (List *)quals;
>
> which takes care of the quals which are not in the List form. This part of
> the code should have taken care of the case you mentioned. It seems that
> for some reason, this part is not converting the ANDed quals into List. Can
> you please check why? May be you want to step through make_ands_implicit()
> (using gdb or some debugger) to see what's happening there.
>
>
>
>
> On Fri, Jan 10, 2014 at 3:38 PM, ZhangJulian <jul...@ou...>wrote:
>
>
> Your name                  : Julian ZL Zhang
> Your email address        : jul...@ou...
>
>
> System Configuration:
> ---------------------
>   Architecture (example: Intel Pentium)                : Intel Pentium
>
>   Operating System (example: Linux 2.4.18)        : 2.6.32-358.el6.x86_64
>
>   Postgres-XC version (example: Postgres-XC 1.1devel): Github master
>   Compiler used (example: gcc 3.3.5)                : gcc (GCC) 4.4.7
> 20120313 (Red Hat 4.4.7-3)
>
>
> Please enter a FULL description of your problem:
> ------------------------------------------------
> I had opened the bug at http://sourceforge.net/p/postgres-xc/bugs/467/ just
> descripte it more detailedly here.
> Different predicate order results in different plan, the query should be
> shipped only to one datanode, but in the first example, two data nodes are
> involved.
>
> Please describe a way to repeat the problem. Please try to provide a
> concise reproducible example, if at all possible:
> ----------------------------------------------------------------------
>  postgres=# create table t1 (c1 int, c2 int, c3 int) distribute by
> hash(c1);
> CREATE TABLE
> postgres=# explain verbose select * from t1 where c1=1 and c2=1 and c3=1;
> QUERY PLAN
>
> --------------------------------------------------------------------------------
> Data Node Scan on "REMOTE_FQS_QUERY" (cost=0.00..0.00 rows=0 width=0)
> Output: t1.c1, t1.c2, t1.c3
> Node/s: datanode1, datanode2
> Remote query: SELECT c1, c2, c3 FROM benchmarksql.t1 WHERE (((c1 = 1) AND
> (c2 = 1)) AND (c3 = 1))
> (4 rows)
> postgres=# explain verbose select * from t1 where c2=1 and c3=1 and c1=1;
> QUERY PLAN
>
> --------------------------------------------------------------------------------
> Data Node Scan on "REMOTE_FQS_QUERY" (cost=0.00..0.00 rows=0 width=0)
> Output: t1.c1, t1.c2, t1.c3
> Node/s: datanode1
> Remote query: SELECT c1, c2, c3 FROM benchmarksql.t1 WHERE (((c2 = 1) AND
> (c3 = 1)) AND (c1 = 1))
> (4 rows)
>
>
> If you know how this problem might be fixed, list the solution below:
> ---------------------------------------------------------------------
> I made a fix as the append file, which can resolve this issue, but I am
> not sure if it is correct.
> When I run MAKE CHECK, 3 test cases failed. The results are appended too.
> But when I run MAKE CHECK without the fix, there are also some test cases
> failed.
> I do not know if the failed test cases are related to the fix.
>
> Thanks
> Julian
>
>
> ------------------------------------------------------------------------------
> CenturyLink Cloud: The Leader in Enterprise Cloud Services.
> Learn Why More Businesses Are Choosing CenturyLink Cloud For
> Critical Workloads, Development Environments & Everything In Between.
> Get a Quote or Start a Free Trial Today.
>
> http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
> _______________________________________________
> Postgres-xc-bugs mailing list
> Pos...@li...
> https://lists.sourceforge.net/lists/listinfo/postgres-xc-bugs
>
>
>
>
> --
> Best Wishes,
> Ashutosh Bapat
> EnterpriseDB Corporation
> The Postgres Database Company
>
> ------------------------------------------------------------------------------
> CenturyLink Cloud: The Leader in Enterprise Cloud Services. Learn Why More
> Businesses Are Choosing CenturyLink Cloud For Critical Workloads,
> Development Environments & Everything In Between. Get a Quote or Start a
> Free Trial Today.
> http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
> _______________________________________________ Postgres-xc-bugs mailing
> list Pos...@li...
> https://lists.sourceforge.net/lists/listinfo/postgres-xc-bugs
>
>
>
>
> --
> Best Wishes,
> Ashutosh Bapat
> EnterpriseDB Corporation
> The Postgres Database Company
>

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company