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/09/23 22:47:19 UTC

[kudu-CR] [java-client] IN-list predicate 

Dan Burkert has uploaded a new change for review.

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

Change subject: [java-client] IN-list predicate
......................................................................

[java-client] IN-list predicate

Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
5 files changed, 672 insertions(+), 144 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>

[kudu-CR] [java-client] IN-list predicate

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

Change subject: [java-client] IN-list predicate
......................................................................


[java-client] IN-list predicate

Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Reviewed-on: http://gerrit.cloudera.org:8080/4530
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
5 files changed, 616 insertions(+), 118 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
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

[kudu-CR] [java-client] IN-list predicate

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: [java-client] IN-list predicate
......................................................................

[java-client] IN-list predicate

Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
5 files changed, 566 insertions(+), 118 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/4530/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4530
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
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

[kudu-CR] [java-client] IN-list predicate

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

Change subject: [java-client] IN-list predicate
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/4530/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java:

Line 91:   /** IN-list values. */
May want to indicate that the outer array (the actual "list") is sorted.


PS1, Line 330:   /**
             :    * Creates a new builder for an IN list predicate.
             :    *
             :    * Only values of the correct type for the column may be added to the builder.
             :    *
             :    * @param column the column schema
             :    * @return the IN list builder
             :    */
             :   public static InListBuilder newInListBuilder(ColumnSchema column) {
             :     return new InListBuilder(column);
             :   }
I don't really see the advantage of this indirection vs. just making the InListBuilder constructor public.


Line 363:   public KuduPredicate(ColumnSchema column, byte[][] inListValues) {
Why is this public? Shouldn't clients go through the builder?


PS1, Line 580:       case IS_NOT_NULL: {
             :         return newIsNotNullPredicate(column);
             :       }
Whoops. What was the effect of missing this?


PS1, Line 596:   /**
             :    * Compares two bounds based on the type of this predicate's column.
             :    * @param a the first serialized value
             :    * @param b the second serialized value
             :    * @return the comparison of the serialized values based on the column type
             :    */
Update the param list.


Line 825:    * Builder for an IN-list predicate.
I don't really like this abstraction, for a couple reasons:
1. Given the various addValue() variants, it suggests that it's possible to add values of different types in the same predicate.
2. Once you strip that away, all that's left is the ability to add a list of values, and there's no need for a builder to get that.

What about a generic KuduPredicate factory method that takes a List<T> of values? That way the language guarantees that all of the values are of the same type, modulo funky casting. I expect that it'll be hard to implement the switch statement in addValue() this way, but maybe there's a clever approach to do it. Or maybe you force callers to provide the data type as an additional argument.


http://gerrit.cloudera.org:8080/#/c/4530/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java:

PS1, Line 448: case IS_NOT_NULL: break;
And here too; what was the effect of missing this?


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

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

[kudu-CR] [java-client] IN-list predicate

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

Change subject: [java-client] IN-list predicate
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4530/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java:

PS2, Line 351:     if (t instanceof Boolean) {
Yessssss (AKA "thanks, type erasure")


Line 392:       checkColumn(column, Type.BINARY);
Don't want to add the bytes? Or maybe this is http://www.snopes.com/music/artists/vanhalen.asp?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
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: Yes

[kudu-CR] [java-client] IN-list predicate

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

Change subject: [java-client] IN-list predicate
......................................................................


Patch Set 1:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/4530/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java:

Line 91:   /** IN-list values. */
> May want to indicate that the outer array (the actual "list") is sorted.
Done


PS1, Line 330:   /**
             :    * Creates a new builder for an IN list predicate.
             :    *
             :    * Only values of the correct type for the column may be added to the builder.
             :    *
             :    * @param column the column schema
             :    * @return the IN list builder
             :    */
             :   public static InListBuilder newInListBuilder(ColumnSchema column) {
             :     return new InListBuilder(column);
             :   }
> I don't really see the advantage of this indirection vs. just making the In
removed.


Line 363:   public KuduPredicate(ColumnSchema column, byte[][] inListValues) {
> Why is this public? Shouldn't clients go through the builder?
Done


PS1, Line 490: .
> nit, remove the trailing period
Done


PS1, Line 504: .
> same
Done


PS1, Line 580:       case IS_NOT_NULL: {
             :         return newIsNotNullPredicate(column);
             :       }
> Whoops. What was the effect of missing this?
Exception when deserializing scan tokens containing an IS NOT NULL predicate.  I've moved this fix to a bug fix patch that should be included in 1.0.1.


PS1, Line 596:   /**
             :    * Compares two bounds based on the type of this predicate's column.
             :    * @param a the first serialized value
             :    * @param b the second serialized value
             :    * @return the comparison of the serialized values based on the column type
             :    */
> Update the param list.
Done


Line 825:    * Builder for an IN-list predicate.
> I don't really like this abstraction, for a couple reasons:
Done


http://gerrit.cloudera.org:8080/#/c/4530/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java:

PS1, Line 448: case IS_NOT_NULL: break;
> And here too; what was the effect of missing this?
KUDU-1652.  Fix moved to separate patch.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
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: Yes

[kudu-CR] [java-client] IN-list predicate

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] IN-list predicate
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4530/3/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java:

Line 393:       for (T value : values) {
So are we missing a test here?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
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: Yes

[kudu-CR] [java-client] IN-list predicate

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] IN-list predicate
......................................................................


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
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] [java-client] IN-list predicate

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: [java-client] IN-list predicate
......................................................................

[java-client] IN-list predicate

Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
5 files changed, 563 insertions(+), 118 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/4530/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4530
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java-client] IN-list predicate

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: [java-client] IN-list predicate
......................................................................

[java-client] IN-list predicate

Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
5 files changed, 616 insertions(+), 118 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/4530/4
-- 
To view, visit http://gerrit.cloudera.org:8080/4530
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
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

[kudu-CR] [java-client] IN-list predicate

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] IN-list predicate
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4530/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java:

PS1, Line 490: .
nit, remove the trailing period


PS1, Line 504: .
same


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

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

[kudu-CR] [java-client] IN-list predicate

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] IN-list predicate
......................................................................


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
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] [java-client] IN-list predicate

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

Change subject: [java-client] IN-list predicate
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4530/3/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java:

Line 393:       for (T value : values) {
> Agreed with JD; I'd like to see test coverage for this case.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
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: Yes

[kudu-CR] [java-client] IN-list predicate

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

Change subject: [java-client] IN-list predicate
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4530/3/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java:

Line 393:       for (T value : values) {
> So are we missing a test here?
Agreed with JD; I'd like to see test coverage for this case.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
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: Yes

[kudu-CR] [java-client] IN-list predicate

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

Change subject: [java-client] IN-list predicate
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4530/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java:

Line 392:       checkColumn(column, Type.BINARY);
> Don't want to add the bytes? Or maybe this is http://www.snopes.com/music/a
Never attribute to malice that which can be adequately explained by stupidity.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia556196963153f9db64d67bc699f96cb920ecac6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
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: Yes