You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org> on 2018/11/07 18:44:30 UTC

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

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 <bi...@cloudera.com>

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:46:49 +0000
Gerrit-HasComments: No

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

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
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 (#3).

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, 18 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/11901/3
-- 
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: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

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

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:05:54 +0000
Gerrit-HasComments: Yes

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

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

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

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:40:43 +0000
Gerrit-HasComments: Yes

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

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 20:28:03 +0000
Gerrit-HasComments: Yes

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

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 23:36:03 +0000
Gerrit-HasComments: Yes

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:46:50 +0000
Gerrit-HasComments: No

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

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 23:32:12 +0000
Gerrit-HasComments: Yes

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 05:42:43 +0000
Gerrit-HasComments: No

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

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:43:13 +0000
Gerrit-HasComments: No

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 22:21:18 +0000
Gerrit-HasComments: No

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

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers 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:

As it turns out, I tackled this at the root cause (NDV=0) in #11528, IMPALA-7310: Use NDV=1 for a Column with all nulls (Waiting for final review.)

Perhaps it is good to tackle the issue in both places: on the metadata side and by being safe when doing calculations.


-- 
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 02:44:52 +0000
Gerrit-HasComments: No

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 19:36:10 +0000
Gerrit-HasComments: No

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

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 23:08:54 +0000
Gerrit-HasComments: Yes

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

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 21:46:29 +0000
Gerrit-HasComments: Yes

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <bi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 02:11:33 +0000
Gerrit-HasComments: No