[Impala-ASF-CR] IMPALA-4662: Fix NULL literal handling in Kudu IN list predicates

2016-12-15 Thread Internal Jenkins (Code Review) 
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4662: Fix NULL literal handling in Kudu IN list 
predicates
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf2c10a326373ad80aef51a85cec64071daefa7b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4662: Fix NULL literal handling in Kudu IN list predicates

2016-12-15 Thread Internal Jenkins (Code Review) 
Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4662: Fix NULL literal handling in Kudu IN list 
predicates
..


IMPALA-4662: Fix NULL literal handling in Kudu IN list predicates

The KuduScanNode attempts to push IN list predicates to the
Kudu scan, but NULL literals cannot be pushed. The code in
KuduScanNode needed to check if the Literals in the
InPredicate is a NullLiteral, in which case the entire IN
list should not be pushed to Kudu.

The same handling is already in place for binary predicate
pushdown.

Change-Id: Iaf2c10a326373ad80aef51a85cec64071daefa7b
Reviewed-on: http://gerrit.cloudera.org:8080/5505
Reviewed-by: Michael Brown 
Reviewed-by: Matthew Jacobs 
Tested-by: Internal Jenkins
---
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
2 files changed, 20 insertions(+), 3 deletions(-)

Approvals:
  Michael Brown: Looks good to me, but someone else must approve
  Matthew Jacobs: Looks good to me, approved
  Internal Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Iaf2c10a326373ad80aef51a85cec64071daefa7b
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown

[Impala-ASF-CR] IMPALA-4662: Fix NULL literal handling in Kudu IN list predicates

2016-12-15 Thread Matthew Jacobs (Code Review) 
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4662: Fix NULL literal handling in Kudu IN list 
predicates
..


Patch Set 2: Code-Review+2

carrying dimitris' +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf2c10a326373ad80aef51a85cec64071daefa7b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4662: Fix NULL literal handling in Kudu IN list predicates

2016-12-14 Thread Michael Brown (Code Review) 
Michael Brown has posted comments on this change.

Change subject: IMPALA-4662: Fix NULL literal handling in Kudu IN list 
predicates
..


Patch Set 2: Code-Review+1

Thanks for adding the additional test cases. My +1 means I'm satisfied that you 
addressed my comments, but obviously someone should take a look again at your 
FE changes.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf2c10a326373ad80aef51a85cec64071daefa7b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4662: Fix NULL literal handling in Kudu IN list predicates

2016-12-14 Thread Matthew Jacobs (Code Review) 
Hello Dimitris Tsirogiannis,

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

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

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

Change subject: IMPALA-4662: Fix NULL literal handling in Kudu IN list 
predicates
..

IMPALA-4662: Fix NULL literal handling in Kudu IN list predicates

The KuduScanNode attempts to push IN list predicates to the
Kudu scan, but NULL literals cannot be pushed. The code in
KuduScanNode needed to check if the Literals in the
InPredicate is a NullLiteral, in which case the entire IN
list should not be pushed to Kudu.

The same handling is already in place for binary predicate
pushdown.

Change-Id: Iaf2c10a326373ad80aef51a85cec64071daefa7b
---
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
2 files changed, 20 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iaf2c10a326373ad80aef51a85cec64071daefa7b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown

[Impala-ASF-CR] IMPALA-4662: Fix NULL literal handling in Kudu IN list predicates

2016-12-14 Thread Matthew Jacobs (Code Review) 
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4662: Fix NULL literal handling in Kudu IN list 
predicates
..


Patch Set 1:

(2 comments)

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

PS1, Line 384: prediates
> Typo: predicates
Done


http://gerrit.cloudera.org:8080/#/c/5505/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
File testdata/workloads/functional-planner/queries/PlannerTest/kudu.test:

PS1, Line 319: select id from functional_kudu.alltypestiny where id in (1, 
null);
> Is it necessary to add additional tests for other types? (string, bool, etc
Sure


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf2c10a326373ad80aef51a85cec64071daefa7b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4662: Fix NULL literal handling in Kudu IN list predicates

2016-12-14 Thread Matthew Jacobs (Code Review) 
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4662: Fix NULL literal handling in Kudu IN list 
predicates
..


Patch Set 1:

(1 comment)

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

PS1, Line 388: Preconditions.checkNotNull(value == null);
> Shouldn't this be checkNotNull(value)?
It depends, if you want this Precondition to do anything then yes :)
Good catch. Fixing this too.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf2c10a326373ad80aef51a85cec64071daefa7b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4662: Fix NULL literal handling in Kudu IN list predicates

2016-12-14 Thread Michael Brown (Code Review) 
Michael Brown has posted comments on this change.

Change subject: IMPALA-4662: Fix NULL literal handling in Kudu IN list 
predicates
..


Patch Set 1:

(2 comments)

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

PS1, Line 384: prediates
Typo: predicates


http://gerrit.cloudera.org:8080/#/c/5505/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
File testdata/workloads/functional-planner/queries/PlannerTest/kudu.test:

PS1, Line 319: select id from functional_kudu.alltypestiny where id in (1, 
null);
Is it necessary to add additional tests for other types? (string, bool, etc.)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf2c10a326373ad80aef51a85cec64071daefa7b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4662: Fix NULL literal handling in Kudu IN list predicates

2016-12-14 Thread Dimitris Tsirogiannis (Code Review) 
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4662: Fix NULL literal handling in Kudu IN list 
predicates
..


Patch Set 1: Code-Review+2

(1 comment)

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

PS1, Line 388: Preconditions.checkNotNull(value == null);
Shouldn't this be checkNotNull(value)?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf2c10a326373ad80aef51a85cec64071daefa7b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4662: Fix NULL literal handling in Kudu IN list predicates

2016-12-14 Thread Matthew Jacobs (Code Review) 
Matthew Jacobs has uploaded a new change for review.

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

Change subject: IMPALA-4662: Fix NULL literal handling in Kudu IN list 
predicates
..

IMPALA-4662: Fix NULL literal handling in Kudu IN list predicates

The KuduScanNode attempts to push IN list predicates to the
Kudu scan, but NULL literals cannot be pushed. The code in
KuduScanNode needed to check if the Literals in the
InPredicate is a NullLiteral, in which case the entire IN
list should not be pushed to Kudu.

The same handling is already in place for binary predicate
pushdown.

Change-Id: Iaf2c10a326373ad80aef51a85cec64071daefa7b
---
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
2 files changed, 15 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaf2c10a326373ad80aef51a85cec64071daefa7b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs