Menu
▾
▴
Re: [Postgres-xc-bugs] Query are shipped to the data nodes more than needed.
|
From: Koichi S. <koi...@gm...> - 2014-02-13 05:31:04
|
Hi Ashutosh;
I'm afraid this patch is still pending for three weeks. Could you
respond if this looks nice and good to go?
Regards;
---
Koichi Suzuki
2014-01-16 17:00 GMT+09:00 ZhangJulian <jul...@ou...>:
> Hi Ashutosh,
>
> How about fixing it as the attached patch?
>
> Thanks
> Julian
> ________________________________
> Date: Wed, 15 Jan 2014 08:49:33 +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...
>
> As you have spotted already changing make_ands_implicit() is not desirable,
> since it has not got modified in many revisions of PostgreSQL and has many
> callers which comply with the current behaviour.
>
> The queries you mentioned are correctly inferring the datanode where to ship
> the query. I think you will have to worry about queries like SELECT c1, c2,
> c3, c4 FROM public.t1 WHERE ((((c1 = 1) AND (c2 = 2)) AND (c3 = 3)) AND (c4
> = 4)), which might not be shipped to only required datanodes.
>
> I do not have the right solutions at hand, but the existing ones are not
> enough. You might want to continue experimenting more.
>
>
> On Tue, Jan 14, 2014 at 12:31 PM, ZhangJulian <jul...@ou...>
> wrote:
>
> 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
>
>
>
>
> --
> 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
>
|
