You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2016/10/04 19:46:42 UTC

[kudu-CR](branch-1.0.x) KUDU-1652 (part 2): Filter IS NOT NULL predicates from scan on client

Hello Jean-Daniel Cryans, Adar Dembo,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: KUDU-1652 (part 2): Filter IS NOT NULL predicates from scan on client
......................................................................

KUDU-1652 (part 2): Filter IS NOT NULL predicates from scan on client

Part 1 fixed the C++ client/server and Java client so that specifying a
an IS NOT NULL predicate on a non-nullable column would not result in a
CHECK failure or exception. This commit filters all such predicates from
new scan requests on the client, in order to avoid crashing server
versions without the part 1 fix (1.0.0 and below).

This is a difficult thing to test, since the underlying issue has
already been fixed, and this just prevents older server version from
crashing. I've manually verified the change by testing this patch with
the additional tests in part 1, but without the fix from part 1.

Change-Id: I542a735ca337ec9ed4eb3c989f8e6fa4ee0cc1da
Reviewed-on: http://gerrit.cloudera.org:8080/4612
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Dan Burkert <da...@cloudera.com>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/scan_spec.cc
3 files changed, 26 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/4619/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4619
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I542a735ca337ec9ed4eb3c989f8e6fa4ee0cc1da
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.0.x
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>

[kudu-CR](branch-1.0.x) KUDU-1652 (part 2): Filter IS NOT NULL predicates from scan on client

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: KUDU-1652 (part 2): Filter IS NOT NULL predicates from scan on client
......................................................................


Patch Set 1: Code-Review+2 Verified+1

Going to manually verify so that it doesn't mess up Jenkins workspaces.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I542a735ca337ec9ed4eb3c989f8e6fa4ee0cc1da
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.0.x
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR](branch-1.0.x) KUDU-1652 (part 2): Filter IS NOT NULL predicates from scan on client

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.

Change subject: KUDU-1652 (part 2): Filter IS NOT NULL predicates from scan on client
......................................................................


KUDU-1652 (part 2): Filter IS NOT NULL predicates from scan on client

Part 1 fixed the C++ client/server and Java client so that specifying a
an IS NOT NULL predicate on a non-nullable column would not result in a
CHECK failure or exception. This commit filters all such predicates from
new scan requests on the client, in order to avoid crashing server
versions without the part 1 fix (1.0.0 and below).

This is a difficult thing to test, since the underlying issue has
already been fixed, and this just prevents older server version from
crashing. I've manually verified the change by testing this patch with
the additional tests in part 1, but without the fix from part 1.

Change-Id: I542a735ca337ec9ed4eb3c989f8e6fa4ee0cc1da
Reviewed-on: http://gerrit.cloudera.org:8080/4612
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Dan Burkert <da...@cloudera.com>
Reviewed-on: http://gerrit.cloudera.org:8080/4619
Reviewed-by: Dan Burkert <da...@cloudera.com>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/scan_spec.cc
3 files changed, 26 insertions(+), 8 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved; Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I542a735ca337ec9ed4eb3c989f8e6fa4ee0cc1da
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.0.x
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>