[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-08 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11901/2/fe/src/main/java/org/apache/impala/planner/JoinNode.java
File fe/src/main/java/org/apache/impala/planner/JoinNode.java:

http://gerrit.cloudera.org:8080/#/c/11901/2/fe/src/main/java/org/apache/impala/planner/JoinNode.java@367
PS2, Line 367:   if (slots.rhsNumRows() > rhsCard) rhsAdjNdv *= rhsCard / 
slots.rhsNumRows();
> nit: Add a comment that explains adjusting to 1 part. It is a bit subtle.
Done


http://gerrit.cloudera.org:8080/#/c/11901/2/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
File fe/src/test/java/org/apache/impala/planner/PlannerTest.java:

http://gerrit.cloudera.org:8080/#/c/11901/2/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@708
PS2, Line 708: testCardinalityDivisionByZero
> nit: rename this to testNullColumnJoinCardinality() or something?
Done



--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:40:43 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-08 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/main/java/org/apache/impala/planner/JoinNode.java
File fe/src/main/java/org/apache/impala/planner/JoinNode.java:

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/main/java/org/apache/impala/planner/JoinNode.java@368
PS1, Line 368:   if (lhsAdjNdv <= 0 && rhsAdjNdv <= 0) return 0;
> Tim qq, the actual cardinality (result) would still be 0 right? What is the
We don't know for sure it's zero. I think mostly NDV == 0 occurs if there are 
all nulls (but I'm not totally sure if there are other cases), and definitely 
plain inner joins return zero rows if either side is all nulls.

But other join conditions and modes handled by this function return > 0 rows if 
both sides are null (e.g. outer joins, <=> operator, etc).



--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Nov 2018 23:36:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-08 Thread Bharath Vissapragada (Code Review)
Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/main/java/org/apache/impala/planner/JoinNode.java
File fe/src/main/java/org/apache/impala/planner/JoinNode.java:

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/main/java/org/apache/impala/planner/JoinNode.java@368
PS1, Line 368:   if (lhsAdjNdv <= 0 && rhsAdjNdv <= 0) return 0;
> Tim qq, the actual cardinality (result) would still be 0 right? What is the
Was discussing with Bikram offline and he pointed me to Paul's change 
https://gerrit.cloudera.org/#/c/11528/

Is this one still needed if Paul's change is committed soon? It adjusts ndvs at 
the SlotRef level and we probably won't hit "division by zero" after that IIUC?



--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Nov 2018 23:32:12 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 4: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:46:49 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-08 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 3: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:43:13 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1324/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Nov 2018 02:11:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-08 Thread Bharath Vissapragada (Code Review)
Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 2: Code-Review+1

(3 comments)

Tim can you +2?

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/main/java/org/apache/impala/planner/JoinNode.java
File fe/src/main/java/org/apache/impala/planner/JoinNode.java:

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/main/java/org/apache/impala/planner/JoinNode.java@368
PS1, Line 368:   long joinCard = Math.round((lhsCard / Math.max(1, 
Math.max(lhsAdjNdv, rhsAdjNdv))) *
> We don't know for sure it's zero. I think mostly NDV == 0 occurs if there a
Ah, outer joins. I thought this code path was inner joins only, for some 
reason. makes sense.


http://gerrit.cloudera.org:8080/#/c/11901/2/fe/src/main/java/org/apache/impala/planner/JoinNode.java
File fe/src/main/java/org/apache/impala/planner/JoinNode.java:

http://gerrit.cloudera.org:8080/#/c/11901/2/fe/src/main/java/org/apache/impala/planner/JoinNode.java@367
PS2, Line 367:   if (slots.rhsNumRows() > rhsCard) rhsAdjNdv *= rhsCard / 
slots.rhsNumRows();
nit: Add a comment that explains adjusting to 1 part. It is a bit subtle.


http://gerrit.cloudera.org:8080/#/c/11901/2/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
File fe/src/test/java/org/apache/impala/planner/PlannerTest.java:

http://gerrit.cloudera.org:8080/#/c/11901/2/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@708
PS2, Line 708: testCardinalityDivisionByZero
nit: rename this to testNullColumnJoinCardinality() or something?



--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:05:54 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3443/ 
DRY_RUN=false


--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:46:50 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..

IMPALA-7528: Fix division by zero when computing cardinalities

This patch fixes a case where there can be a division by zero when
computing cardinalities of many to many joins on NULL columns.

Testing:
Added a planner test case

Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Reviewed-on: http://gerrit.cloudera.org:8080/11901
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M fe/src/main/java/org/apache/impala/planner/JoinNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/bin/compute-table-stats.sh
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
4 files changed, 18 insertions(+), 4 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 4: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Nov 2018 05:42:43 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-07 Thread Bharath Vissapragada (Code Review)
Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/main/java/org/apache/impala/planner/JoinNode.java
File fe/src/main/java/org/apache/impala/planner/JoinNode.java:

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/main/java/org/apache/impala/planner/JoinNode.java@368
PS1, Line 368:   if (lhsAdjNdv <= 0 && rhsAdjNdv <= 0) return 0;
> Done. This is inline with ongoing changes for
Tim qq, the actual cardinality (result) would still be 0 right? What is the 
advantage in overestimating if we know for sure it will be 0? Trying to 
understand.



--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Nov 2018 23:08:54 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 2:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1320/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Nov 2018 22:21:18 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-07 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/main/java/org/apache/impala/planner/JoinNode.java
File fe/src/main/java/org/apache/impala/planner/JoinNode.java:

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/main/java/org/apache/impala/planner/JoinNode.java@368
PS1, Line 368:   if (lhsAdjNdv <= 0 && rhsAdjNdv <= 0) return 0;
> Would it make sense to instead treat the NDV as 1 in this case? Since if th
Done. This is inline with ongoing changes for
IMPALA-7310


http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
File fe/src/test/java/org/apache/impala/planner/PlannerTest.java:

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@713
PS1, Line 713: checkCardinality(query, 0, 1);  }
> newline
Done



--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Nov 2018 21:46:29 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-07 Thread Bikramjeet Vig (Code Review)
Hello Bharath Vissapragada, Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/11901

to look at the new patch set (#2).

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..

IMPALA-7528: Fix division by zero when computing cardinalities

This patch fixes a case where there can be a division by zero when
computing cardinalities of many to many joins on NULL columns.

Testing:
Added a planner test case

Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
---
M fe/src/main/java/org/apache/impala/planner/JoinNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/bin/compute-table-stats.sh
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
4 files changed, 14 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/11901/2
--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/main/java/org/apache/impala/planner/JoinNode.java
File fe/src/main/java/org/apache/impala/planner/JoinNode.java:

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/main/java/org/apache/impala/planner/JoinNode.java@368
PS1, Line 368:   if (lhsAdjNdv <= 0 && rhsAdjNdv <= 0) return 0;
Would it make sense to instead treat the NDV as 1 in this case? Since if the 
NDV estimate is lower, we expect the join cardinality to be higher.

I.e. Math.max(lhsAdjNdv, rhsAdjNdv) becomes Math.max(1, Math.max(lhsAdjNdv, 
rhsAdjNdv))


http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
File fe/src/test/java/org/apache/impala/planner/PlannerTest.java:

http://gerrit.cloudera.org:8080/#/c/11901/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@713
PS1, Line 713: checkCardinality(query, 0, 1);  }
newline



--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Nov 2018 20:28:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11901 )

Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1313/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 07 Nov 2018 19:36:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7528: Fix division by zero when computing cardinalities

2018-11-07 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11901


Change subject: IMPALA-7528: Fix division by zero when computing cardinalities
..

IMPALA-7528: Fix division by zero when computing cardinalities

This patch fixes a case where there can be a division by zero when
computing cardinalities of many to many joins on NULL columns.

Testing:
Added a planner test case

Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
---
M fe/src/main/java/org/apache/impala/planner/JoinNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/bin/compute-table-stats.sh
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
4 files changed, 12 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/11901/1
--
To view, visit http://gerrit.cloudera.org:8080/11901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieedd51d3ad6131a4ea2a5883f05104e6a0f2cd14
Gerrit-Change-Number: 11901
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig