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

[ZhangJulian <jul...@ou...>]

Hi Ashutosh,

I tested two more scenerios with the fix:
postgres=# create table t1 (c1 int, c2 int, c3 int, c4 int);
CREATE TABLE
postgres=# explain verbose select * from t1 where c1=1 and c2=2 and c3=3 and c4=4;
                                                    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, t1.c4
   Node/s: datanode1
   Remote query: SELECT c1, c2, c3, c4 FROM public.t1 WHERE ((((c1 = 1) AND (c2 = 2)) AND (c3 = 3)) AND (c4 = 4))
(4 rows)
postgres=# explain verbose select * from t1 where c1=1 and c2=2 and (c3=3 or c4=4);
                                                   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, t1.c4
   Node/s: datanode1
   Remote query: SELECT c1, c2, c3, c4 FROM public.t1 WHERE (((c1 = 1) AND (c2 = 2)) AND ((c3 = 3) OR (c4 = 4)))
(4 rows)

For a complex predicate as "where c1=1 and c2=2 and c3=3 and c4=4", it will be parsed to "WHERE ((((c1 = 1) AND (c2 = 2)) AND (c3 = 3)) AND (c4 = 4))", that is:
 BoolExpr
        BoolExpr
               OpExpr (c1=1)
               OpExpr (c2=2)
        OpExpr (c3=3)
 OpExpr (c4=4)

Do you mean I should fix it in another way by updating make_ands_implicit() to return the list with all AND legs? It should be a better way, but there are several invocations to make_ands_implicit(), I can not understand all of the callers and have the confidence to fix it as it. :)

The callers to make_ands_implicit():
make_ands_implicit(Expr *) : List *
    ATAddCheckConstraint(List * *, AlteredTableInfo *, Relation, Constraint *, bool, bool, LOCKMODE) : void
    convert_EXISTS_to_ANY(PlannerInfo *, Query *, Node * *, List * *) : Query *
    cost_subplan(PlannerInfo *, SubPlan *, Plan *) : void
    DefineIndex(RangeVar *, char *, Oid, Oid, char *, char *, List *, Expr *, List *, List *, bool, bool, bool, bool, bool, bool, bool, bool, bool, bool) : Oid
    ExecRelCheck(ResultRelInfo *, TupleTableSlot *, EState *) : const char *
    get_relation_constraints(PlannerInfo *, Oid, RelOptInfo *, bool) : List *
    pgxc_find_dist_equijoin_qual(List *, List *, Node *) : Expr *
    pgxc_find_distcol_expr(Index, AttrNumber, Node *) : Expr *
    preprocess_expression(PlannerInfo *, Node *, int) : Node *
    RelationGetIndexPredicate(Relation) : List *
    set_append_rel_size(PlannerInfo *, RelOptInfo *, Index, RangeTblEntry *) : void
    TriggerEnabled(EState *, ResultRelInfo *, Trigger *, TriggerEvent, Bitmapset *, HeapTuple, HeapTuple) : bool
    validateCheckConstraint(Relation, HeapTuple) : void


Thanks
Julian

Date: Mon, 13 Jan 2014 15:11:00 +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...

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