You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2019/06/06 02:34:11 UTC
[kudu-CR] [java] Add newComparisonPredicate Object API
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13527
Change subject: [java] Add newComparisonPredicate Object API
......................................................................
[java] Add newComparisonPredicate Object API
Adds a `newComparisonPredicate` API that takes an
Object in order to support creating comparison
predicates for Java objects generically.
Change-Id: If05b2d9f61990b86d92af4b89e21704d5188192e
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
2 files changed, 56 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/13527/1
--
To view, visit http://gerrit.cloudera.org:8080/13527
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If05b2d9f61990b86d92af4b89e21704d5188192e
Gerrit-Change-Number: 13527
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
[kudu-CR] [java] Add newComparisonPredicate Object API
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13527
to look at the new patch set (#2).
Change subject: [java] Add newComparisonPredicate Object API
......................................................................
[java] Add newComparisonPredicate Object API
Adds a `newComparisonPredicate` API that takes an
Object in order to support creating comparison
predicates for Java objects generically.
Change-Id: If05b2d9f61990b86d92af4b89e21704d5188192e
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
3 files changed, 89 insertions(+), 27 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/13527/2
--
To view, visit http://gerrit.cloudera.org:8080/13527
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If05b2d9f61990b86d92af4b89e21704d5188192e
Gerrit-Change-Number: 13527
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] Add newComparisonPredicate Object API
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13527 )
Change subject: [java] Add newComparisonPredicate Object API
......................................................................
[java] Add newComparisonPredicate Object API
Adds a `newComparisonPredicate` API that takes an
Object in order to support creating comparison
predicates for Java objects generically.
Change-Id: If05b2d9f61990b86d92af4b89e21704d5188192e
Reviewed-on: http://gerrit.cloudera.org:8080/13527
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
3 files changed, 89 insertions(+), 27 deletions(-)
Approvals:
Kudu Jenkins: Verified
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13527
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If05b2d9f61990b86d92af4b89e21704d5188192e
Gerrit-Change-Number: 13527
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [java] Add newComparisonPredicate Object API
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13527 )
Change subject: [java] Add newComparisonPredicate Object API
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13527/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:
http://gerrit.cloudera.org:8080/#/c/13527/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java@490
PS2, Line 490: * Type.BOOL -> java.lang.Boolean
: * Type.INT8 -> java.lang.Byte
: * Type.INT16 -> java.lang.Short
: * Type.INT32 -> java.lang.Integer
: * Type.INT64 -> java.lang.Long
: * Type.UNIXTIME_MICROS -> java.sql.Timestamp or java.lang.Long
: * Type.FLOAT -> java.lang.Float
: * Type.DOUBLE -> java.lang.Double
: * Type.STRING -> java.lang.String
: * Type.BINARY -> byte[]
: * Type.DECIMAL -> java.math.BigDecimal
This is yet another list to update when we add a new type. Does it add enough value to warrant that cost?
http://gerrit.cloudera.org:8080/#/c/13527/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java@506
PS2, Line 506: public static KuduPredicate newComparisonPredicate(ColumnSchema column,
Does this work as a generic method? I can't remember the rules for Java generics.
As-is there's a runtime cost (in terms of casts) that the current approach doesn't have, and that a generic approach wouldn't have either (assuming it worked).
--
To view, visit http://gerrit.cloudera.org:8080/13527
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If05b2d9f61990b86d92af4b89e21704d5188192e
Gerrit-Change-Number: 13527
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 06 Jun 2019 05:37:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Add newComparisonPredicate Object API
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13527 )
Change subject: [java] Add newComparisonPredicate Object API
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13527/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:
http://gerrit.cloudera.org:8080/#/c/13527/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java@490
PS2, Line 490: * Type.BOOL -> java.lang.Boolean
: * Type.INT8 -> java.lang.Byte
: * Type.INT16 -> java.lang.Short
: * Type.INT32 -> java.lang.Integer
: * Type.INT64 -> java.lang.Long
: * Type.UNIXTIME_MICROS -> java.sql.Timestamp or java.lang.Long
: * Type.FLOAT -> java.lang.Float
: * Type.DOUBLE -> java.lang.Double
: * Type.STRING -> java.lang.String
: * Type.BINARY -> byte[]
: * Type.DECIMAL -> java.math.BigDecimal
> This is yet another list to update when we add a new type. Does it add enou
I think so. Given this is public API and these Javadocs are a way for users to understand the expected behavior.
http://gerrit.cloudera.org:8080/#/c/13527/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java@506
PS2, Line 506: public static KuduPredicate newComparisonPredicate(ColumnSchema column,
> Does this work as a generic method? I can't remember the rules for Java generics.
Java matches the most specific method and falls back to less specific. So in this case, this method will be called when the type for value doesn't match any of the other methods.
If you mean generic in terms of specifying the type like `T Value`, that doesn't help here since I would still need to handle all the expected types, including Object.
The main motivation for this patch is integration with Hive which provides values as an Object from its APIs. I suspect this is common from other frameworks as well. Instead of spreading these if/else/switch/cast statements throughout the integrations, it's nice to have the, in one place in the Kudu API.
--
To view, visit http://gerrit.cloudera.org:8080/13527
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If05b2d9f61990b86d92af4b89e21704d5188192e
Gerrit-Change-Number: 13527
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 06 Jun 2019 12:56:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Add newComparisonPredicate Object API
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13527 )
Change subject: [java] Add newComparisonPredicate Object API
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13527
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If05b2d9f61990b86d92af4b89e21704d5188192e
Gerrit-Change-Number: 13527
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 06 Jun 2019 17:28:34 +0000
Gerrit-HasComments: No