You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org> on 2019/10/21 06:55:40 UTC
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Volodymyr Verovkin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14517
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
---
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
M src/kudu/common/common.proto
20 files changed, 367 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/14517/1
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 1
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 9: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 9
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Dec 2019 17:20:10 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 12:
> Build Failed
>
> http://jenkins.kudu.apache.org/job/kudu-gerrit/20204/ : FAILURE
Does the failure seem relevant?
There was 1 failure:
1) testRandomBackupAndRestore(org.apache.kudu.backup.TestKuduBackup)
java.lang.AssertionError: expected:<0> but was:<1>
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.failNotEquals(Assert.java:834)
at org.junit.Assert.assertEquals(Assert.java:645)
at org.junit.Assert.assertEquals(Assert.java:631)
at org.apache.kudu.backup.TestKuduBackup.testRandomBackupAndRestore(TestKuduBackup.scala:273)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
at org.apache.kudu.test.junit.RetryRule$RetryStatement.doOneAttempt(RetryRule.java:217)
at org.apache.kudu.test.junit.RetryRule$RetryStatement.evaluate(RetryRule.java:234)
at org.junit.rules.RunRules.evaluate(RunRules.java:20)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runners.Suite.runChild(Suite.java:128)
at org.junit.runners.Suite.runChild(Suite.java:27)
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 12
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Jan 2020 08:26:28 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14517
to look at the new patch set (#5).
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
from
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
---
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
A java/kudu-client/src/test/java/org/apache/kudu/util/TestDateUtil.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
18 files changed, 449 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/14517/5
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 5
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14517/2/src/kudu/common/common.proto
File src/kudu/common/common.proto:
PS2:
This ought to be in part 1, no?
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 2
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 25 Oct 2019 03:09:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 9: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 9
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Jan 2020 19:25:23 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14517
to look at the new patch set (#13).
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
from
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
---
M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
A java/kudu-client/src/test/java/org/apache/kudu/util/TestDateUtil.java
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/DistributedDataGenerator.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
25 files changed, 507 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/14517/13
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 13
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 14: Code-Review+2
This change looks good to me. Maybe, you want to get some feedback from Scala and Java experts as well.
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 14
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 03 Feb 2020 17:55:06 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14517
to look at the new patch set (#3).
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
from
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
---
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
A java/kudu-client/src/test/java/org/apache/kudu/util/TestDateUtil.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
20 files changed, 449 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/14517/3
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 3
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 12:
> > Build Failed
> >
> > http://jenkins.kudu.apache.org/job/kudu-gerrit/20204/ : FAILURE
>
> Does the failure seem relevant?
>
> There was 1 failure:
> 1) testRandomBackupAndRestore(org.apache.kudu.backup.TestKuduBackup)
> java.lang.AssertionError: expected:<0> but was:<1>
> at org.junit.Assert.fail(Assert.java:88)
> at org.junit.Assert.failNotEquals(Assert.java:834)
> at org.junit.Assert.assertEquals(Assert.java:645)
> at org.junit.Assert.assertEquals(Assert.java:631)
> at org.apache.kudu.backup.TestKuduBackup.testRandomBackupAndRestore(TestKuduBackup.scala:273)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:498)
> at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
> at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
> at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
> at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
> at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
> at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
> at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
> at org.apache.kudu.test.junit.RetryRule$RetryStatement.doOneAttempt(RetryRule.java:217)
> at org.apache.kudu.test.junit.RetryRule$RetryStatement.evaluate(RetryRule.java:234)
> at org.junit.rules.RunRules.evaluate(RunRules.java:20)
> at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
> at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
> at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
> at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
> at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
> at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
> at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
> at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
> at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
> at org.junit.runners.Suite.runChild(Suite.java:128)
> at org.junit.runners.Suite.runChild(Suite.java:27)
If looking further into the logs which you can download from http://dist-test.cloudera.org/job?job_id=jenkins-slave.1579592343.9849 , I can see the following error (just a part of the stack trace):
07:40:14.278 [ERROR - ForkJoinPool-23-worker-3] (KuduBackup.scala:148) Failed to back up table random-1579592413571
java.lang.ClassCastException: java.lang.Integer cannot be cast to java.sql.Date
at org.apache.kudu.backup.TableMetadata$.org$apache$kudu$backup$TableMetadata$$valueToString(TableMetadata.scala:300) ~[kudu-backup-common-1.12.0-SNAPSHOT.jar:?]
at org.apache.kudu.backup.TableMetadata$$anonfun$1.apply(TableMetadata.scala:76) ~[kudu-backup-common-1.12.0-SNAPSHOT.jar:?]
at org.apache.kudu.backup.TableMetadata$$anonfun$1.apply(TableMetadata.scala:60) ~[kudu-backup-common-1.12.0-SNAPSHOT.jar:?]
at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234) ~[scala-library-2.11.12.jar:?]
at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234) ~[scala-library-2.11.12.jar:?]
at scala.collection.Iterator$class.foreach(Iterator.scala:891) ~[scala-library-2.11.12.jar:?]
at scala.collection.AbstractIterator.foreach(Iterator.scala:1334) ~[scala-library-2.11.12.jar:?]
at scala.collection.IterableLike$class.foreach(IterableLike.scala:72) ~[scala-library-2.11.12.jar:?]
at scala.collection.AbstractIterable.foreach(Iterable.scala:54) ~[scala-library-2.11.12.jar:?]
at scala.collection.TraversableLike$class.map(TraversableLike.scala:234) ~[scala-library-2.11.12.jar:?]
It seems the issue comes from the fact that the default value for a DATE column was interpreted as Integer.
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 12
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Jan 2020 21:04:31 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 1:
(17 comments)
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
File java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java:
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java@110
PS1, Line 110: buf.append(value.getInt(i));
I think users would expect this to output the String representation of a date.
http://gerrit.cloudera.org:8080/#/c/14517/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:
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java@27
PS1, Line 27: import java.time.LocalDate;
Is LocalDate used anywhere?
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java:
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@316
PS1, Line 316: checkColumn(schema.getColumnByIndex(columnIndex), Type.INT32, Type.DATE);
I don't think we should allow date here given the storage as an integer is an internal detail.
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@622
PS1, Line 622: public void addDate(int columnIndex, int val) {
Do you think we need the integer version of this? We don't have the integer version for decimal for example. I think the smaller the API surface area the better unless there is strong evidence this will be useful. Especially because the storage as an integer is an internal detail.
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@642
PS1, Line 642: * Add a sql.Date for the specified column.
nit. remove `sql.`
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@698
PS1, Line 698:
nit: whitespace
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@1040
PS1, Line 1040: * The accepted Object type is based on the column's {@link Type}:
Update the javadoc to include Type.DATE
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@1080
PS1, Line 1080: case DATE:
I would only handle Date here to keep the expectations simpler. The usage of integer is an internal concern.
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@1114
PS1, Line 1114: * The Object type is based on the column's {@link Type}:
Update the javadoc to include Type.DATE
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@268
PS1, Line 268: case DATE:
I think this needs to handle the sql.Date object.
I also think `UNIXTIME_MICROS` needs to additionally handle the sql.Timestamp object and that is a bug.
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@301
PS1, Line 301: case DATE:
I think this should return the sql.Date object.
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java:
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java@137
PS1, Line 137: checkType(columnIndex, Type.INT32, Type.DATE);
I don't think this should accept the date type to avoid user error given storage as an integer is an internal detail.
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java@604
PS1, Line 604: * Type.DECIMAL -> java.math.BigDecimal
Update the javadoc for Type.DATE
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
File java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java:
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@10
PS1, Line 10: public static final int MIN_DATE_VALUE =
nit: there are some trailing spaces in this file.
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java:
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java@151
PS1, Line 151:
nit: whitespace
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
File java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java:
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java@232
PS1, Line 232: case DATE:
I think you can remove the changes for kudu-flume given we are currently in the process of moving this integration to the flume project.
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java
File java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java:
http://gerrit.cloudera.org:8080/#/c/14517/1/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java@329
PS1, Line 329: case DATE:
nit: trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 1
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 21 Oct 2019 14:47:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Volodymyr Verovkin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 10:
Added new tests to https://gerrit.cloudera.org/#/c/14517/10/java/kudu-client/src/test/java/org/apache/kudu/util/TestDateUtil.java
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 10
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 Jan 2020 08:41:03 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14517
to look at the new patch set (#10).
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
from
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
---
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
A java/kudu-client/src/test/java/org/apache/kudu/util/TestDateUtil.java
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/DistributedDataGenerator.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
21 files changed, 485 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/14517/10
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 10
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14517
to look at the new patch set (#9).
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
from
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
---
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
A java/kudu-client/src/test/java/org/apache/kudu/util/TestDateUtil.java
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/DistributedDataGenerator.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
21 files changed, 462 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/14517/9
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 9
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 16:
Backup test scenarios also fail, and it seems we have seen this before:
09:34:37.564 [ERROR - ForkJoinPool-23-worker-3] (KuduBackup.scala:148) Failed to back up table random-1581068076770
java.lang.ClassCastException: java.sql.Date cannot be cast to java.lang.Integer
at scala.runtime.BoxesRunTime.unboxToInt(BoxesRunTime.java:101) ~[scala-library-2.11.12.jar:?]
at org.apache.kudu.backup.TableMetadata$.org$apache$kudu$backup$TableMetadata$$valueToString(TableMetadata.scala:304) ~[kudu-backup-common-1.12.0-SNAPSHOT.jar:?]
at org.apache.kudu.backup.TableMetadata$$anonfun$1.apply(TableMetadata.scala:77) ~[kudu-backup-common-1.12.0-SNAPSHOT.jar:?]
at org.apache.kudu.backup.TableMetadata$$anonfun$1.apply(TableMetadata.scala:61) ~[kudu-backup-common-1.12.0-SNAPSHOT.jar:?]
at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234) ~[scala-library-2.11.12.jar:?]
at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234) ~[scala-library-2.11.12.jar:?]
at scala.collection.Iterator$class.foreach(Iterator.scala:891) ~[scala-library-2.11.12.jar:?]
at scala.collection.AbstractIterator.foreach(Iterator.scala:1334) ~[scala-library-2.11.12.jar:?]
at scala.collection.IterableLike$class.foreach(IterableLike.scala:72) ~[scala-library-2.11.12.jar:?]
at scala.collection.AbstractIterable.foreach(Iterable.scala:54) ~[scala-library-2.11.12.jar:?]
at scala.collection.TraversableLike$class.map(TraversableLike.scala:234) ~[scala-library-2.11.12.jar:?]
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 16
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Feb 2020 17:59:06 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 10: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 10
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 Jan 2020 12:08:06 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Volodymyr Verovkin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 15:
Changes in Java code do not induce Scala code recompilation. Seems like there is no dependency between Java and Scala code.
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 15
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Feb 2020 09:21:41 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
from
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Reviewed-on: http://gerrit.cloudera.org:8080/14517
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Alexey Serbin <as...@cloudera.com>
---
M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
A java/kudu-client/src/test/java/org/apache/kudu/util/TestDateUtil.java
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/DistributedDataGenerator.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
25 files changed, 490 insertions(+), 18 deletions(-)
Approvals:
Kudu Jenkins: Verified
Grant Henke: Looks good to me, approved
Alexey Serbin: Verified
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 20
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14517
to look at the new patch set (#14).
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
from
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
---
M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
A java/kudu-client/src/test/java/org/apache/kudu/util/TestDateUtil.java
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/DistributedDataGenerator.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
25 files changed, 505 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/14517/14
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 14
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 15:
It seems PS15 doesn't even compile:
15:05:03 > Task :kudu-spark-tools:compileScala FAILED
15:05:03 Pruning sources from previous analysis, due to incompatible CompileSetup.
15:05:03 /home/jenkins-slave/workspace/kudu-master/0/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/DistributedDataGenerator.scala:217: overloaded method value addDate with alternatives:
15:05:03 (x$1: String,x$2: java.sql.Date)Unit <and>
15:05:03 (x$1: Int,x$2: java.sql.Date)Unit
15:05:03 cannot be applied to (Int, Int)
15:05:03 row.addDate(i, value.toInt)
15:05:03 ^
15:05:03 one error found
15:05:03
15:05:03 > Task :kudu-spark-tools:processResources NO-SOURCE
15:05:03 > Task :kudu-spark-tools:processTestResources
15:05:04 > Task :kudu-test-utils:compileTestJava
15:05:04 > Task :kudu-test-utils:processTestResources
15:05:04 > Task :kudu-test-utils:testClasses
15:05:04
15:05:04 FAILURE: Build completed with 2 failures.
15:05:04
15:05:04 1: Task failed with an exception.
15:05:04 -----------
15:05:04 * What went wrong:
15:05:04 Execution failed for task ':kudu-spark:compileTestScala'.
15:05:04 > Compilation failed
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 15
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Feb 2020 05:02:25 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 7:
> Build Failed
>
> http://jenkins.kudu.apache.org/job/kudu-gerrit/19818/ : FAILURE
These test failures look relevant.
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 7
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Dec 2019 06:39:45 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14517/4/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
File java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java:
PS4:
can you add test cases to cover all dates from the C++ client to make sure they're converted to dates the same way?
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 9
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jan 2020 14:09:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Volodymyr Verovkin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 15:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java:
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@316
PS14, Line 316: checkColumn(schema.getColumnByIndex(columnIndex), Type.INT32);
> I don't think we need to let users call getInt on a Date column. They can a
Done
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@622
PS14, Line 622: */
> Can we simplify the interface and only accept Date instead of int. Is there
Done
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@1079
PS14, Line 1079: }
> Can you add Date to this Java doc?
Done
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@1175
PS14, Line 1175: * Type.INT8 -> java.lang.Byte
> Here too.
Done
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@274
PS14, Line 274: case INT16:
> I think this also needs to handle the Date object. This is called for defau
Done
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@308
PS14, Line 308: return buf.get();
> I think this should return a Date object.
Done
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java:
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java@624
PS14, Line 624: * Type.BOOL -> java.lang.Boolean
> Can you update this Javadoc?
Done
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
File java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java:
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java@142
PS14, Line 142: */
> This should return a Date object. That will help eliminated the need for a
Done
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
File java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java:
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java@470
PS14, Line 470:
> Do we test default values anywhere? I think
We do not test default values anywhere.
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 15
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 06 Feb 2020 23:02:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 8:
It looks like there are failing Java tests likely do to missing DATE type support: http://dist-test.cloudera.org/job?job_id=jenkins-slave.1576548118.23043
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 8
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Dec 2019 20:17:24 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Volodymyr Verovkin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 3:
(19 comments)
http://gerrit.cloudera.org:8080/#/c/14517/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14517/2//COMMIT_MSG@13
PS2, Line 13: from
> from
Done
http://gerrit.cloudera.org:8080/#/c/14517/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/14517/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java@172
PS2, Line 172:
> trailing space
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java:
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@646
PS2, Line 646: IllegalArgumentException
> Is it possible with given the Date range constraints? Overall, I think it
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@674
PS2, Line 674: a Date
> nit: Date value stored in the column?
I just copied comments from Timestamp method. And it similar to other methods
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@692
PS2, Line 692: checkColumnExists(schema.getColumnByIndex(columnIndex));
: checkColumn(schema.getColumnByIndex(columnIndex), Type.D
> Should these two be swapped?
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@698
PS2, Line 698:
> nit: trailing spaces
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
File java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java:
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java@128
PS2, Line 128: inte
> nit: int value representable as DATE ?
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
File java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java:
PS2:
> Add Apache license header.
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@9
PS2, Line 9: // http://www.apache.org/licenses/LICENSE-2.0
> It would be great to add unit tests for the methods of this class. Points
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@10
PS2, Line 10:
> Trailing spaces here and in few lines in this file.
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@11
PS2, Line 11: // Unless required by applicable law or agreed to in writing,
: // software distributed under the License is distributed on an
: // "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> Could you add information on how these numbers were produced? I understand
Those numbers are produced by printing result Java expressions
System.out.println(LocalDate.parse("0001-01-01").toEpochDay()) -> -719162
System.out.println(LocalDate.parse("9999-12-31").toEpochDay()) -> 2932896
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@22
PS2, Line 22:
> Does it make sense to provide the value in the error message?
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@27
PS2, Line 27:
> number of days
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@57
PS2, Line 57: /**
> Does it make sense to call checkDateWithinRange() here as it's done in epoc
It works correctly for all dates. It's useful to print out of range dates in readable form.
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@653
PS2, Line 653: date colu
> date ?
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java:
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java@440
PS2, Line 440:
> Does it make sense to add negative scenarios for PartialRow.addDate() as we
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java:
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java@156
PS2, Line 156:
> nit: trailing spaces
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java
File java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java:
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java@329
PS2, Line 329:
> trailing spaces
Done
http://gerrit.cloudera.org:8080/#/c/14517/2/src/kudu/common/common.proto
File src/kudu/common/common.proto:
PS2:
> This ought to be in part 1, no?
Done
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 3
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Nov 2019 09:12:23 +0000
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14517
to look at the new patch set (#2).
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
---
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
M src/kudu/common/common.proto
20 files changed, 368 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/14517/2
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 2
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14517
to look at the new patch set (#17).
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
from
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
---
M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
A java/kudu-client/src/test/java/org/apache/kudu/util/TestDateUtil.java
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/DistributedDataGenerator.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
25 files changed, 490 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/14517/17
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 17
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 14:
(9 comments)
Sorry for the late review. It looks close, I just noticed a small issue around default values and had a few other nit-picky comments.
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java:
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@316
PS14, Line 316: checkColumn(schema.getColumnByIndex(columnIndex), Type.INT32, Type.DATE);
I don't think we need to let users call getInt on a Date column. They can always convert the Date object if needed.
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@622
PS14, Line 622: public void addDate(int columnIndex, int val) {
Can we simplify the interface and only accept Date instead of int. Is there a reason we want to accept integers?
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@1079
PS14, Line 1079: * Type.BOOL -> java.lang.Boolean
Can you add Date to this Java doc?
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@1175
PS14, Line 1175: * Type.BOOL -> java.lang.Boolean
Here too.
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@274
PS14, Line 274: case DATE:
I think this also needs to handle the Date object. This is called for default values which can accept Date. Adding a test which has a default Date value to a Date column should show this issue.
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@308
PS14, Line 308: case DATE:
I think this should return a Date object.
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java:
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java@624
PS14, Line 624: * Type.BOOL -> java.lang.Boolean
Can you update this Javadoc?
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
File java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java:
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java@142
PS14, Line 142: public static int randomDate(Random random) {
This should return a Date object. That will help eliminated the need for a integer based addDate method (used in SchemaGenerator.java)
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
File java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java:
http://gerrit.cloudera.org:8080/#/c/14517/14/java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java@470
PS14, Line 470: public static Schema createSchemaWithDateColumns() {
Do we test default values anywhere? I think
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 14
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 03 Feb 2020 19:45:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 13:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14517/13/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala
File java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala:
http://gerrit.cloudera.org:8080/#/c/14517/13/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala@302
PS13, Line 302: case d: Date => d.toString()
: case n: Integer => DateUtil.epochDaysToDateString(n)
I'm not a Scala expert, but this looks a bit strange.
Why value.asInstanceOf[Date] would return sometimes a Date, but sometimes an Integer instance? Is it possible to unify the type of the returned value so it's always Date?
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 13
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Jan 2020 01:01:53 +0000
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14517
to look at the new patch set (#15).
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
from
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
---
M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
A java/kudu-client/src/test/java/org/apache/kudu/util/TestDateUtil.java
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/DistributedDataGenerator.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
25 files changed, 487 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/14517/15
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 15
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 19: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 19
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Feb 2020 19:07:18 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14517
to look at the new patch set (#11).
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
from
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
---
M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
A java/kudu-client/src/test/java/org/apache/kudu/util/TestDateUtil.java
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/DistributedDataGenerator.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
25 files changed, 501 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/14517/11
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 11
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14517
to look at the new patch set (#16).
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
from
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
---
M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
A java/kudu-client/src/test/java/org/apache/kudu/util/TestDateUtil.java
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/DistributedDataGenerator.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
25 files changed, 489 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/14517/16
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 16
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 7:
> > Build Failed
> >
> > http://jenkins.kudu.apache.org/job/kudu-gerrit/19818/ : FAILURE
>
> These test failures look relevant.
Also, check for LINT build failures:
http://jenkins.kudu.apache.org/job/kudu-gerrit/19816/BUILD_TYPE=LINT/artifact/build/latest/test-logs/lint.log
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 7
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Dec 2019 06:41:12 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14517
to look at the new patch set (#4).
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
[KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
This adds a new DATE type, represented by an INT32 and that should
store the number of days from the Unix epoch, January 1, 1970.
Range:
from
LocalDate.parse("0001-01-01").toEpochDay()
to
LocalDate.parse("9999-12-31").toEpochDay()
Range validation is done in DateUtil::checkDateWithinRange() function.
Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
---
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.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/TestOperation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
A java/kudu-client/src/test/java/org/apache/kudu/util/TestDateUtil.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
18 files changed, 443 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/14517/4
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 4
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 19: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 19
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Feb 2020 19:03:36 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 10:
A TestKuduBackup test is failing. It looks like the date type is missing in TableMetadata$$valueToString:
java.lang.IllegalArgumentException: Unsupported column type: Type: date
at org.apache.kudu.backup.TableMetadata$.org$apache$kudu$backup$TableMetadata$$valueToString(TableMetadata.scala:297) ~[kudu-backup-common-1.12.0-SNAPSHOT.jar:?]
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 10
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 Jan 2020 17:11:17 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 2:
(19 comments)
http://gerrit.cloudera.org:8080/#/c/14517/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14517/2//COMMIT_MSG@13
PS2, Line 13: LocalDate.parse("0001-01-01").toEpochDay()
from
http://gerrit.cloudera.org:8080/#/c/14517/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/14517/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java@172
PS2, Line 172:
trailing space
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java:
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@646
PS2, Line 646: IllegalArgumentException
Is it possible with given the Date range constraints? Overall, I think it would be nice to add a unit test for this method to verify the declared behavior.
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@674
PS2, Line 674: a Date
nit: Date value stored in the column?
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@692
PS2, Line 692: checkColumn(schema.getColumnByIndex(columnIndex), Type.DATE);
: checkColumnExists(schema.getColumnByIndex(columnIndex));
Should these two be swapped?
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@698
PS2, Line 698:
nit: trailing spaces
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
File java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java:
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java@128
PS2, Line 128: date
nit: int value representable as DATE ?
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java
File java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java:
PS2:
Add Apache license header.
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@9
PS2, Line 9: public class DateUtil {
It would be great to add unit tests for the methods of this class. Points of interest for unit tests are boundary conditions (i.e. min/max values and under/over those, etc.) at least.
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@10
PS2, Line 10:
Trailing spaces here and in few lines in this file.
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@11
PS2, Line 11: (int)LocalDate.parse("0001-01-01").toEpochDay(); // -719162
: public static final int MAX_DATE_VALUE =
: (int)LocalDate.parse("9999-12-31").toEpochDay(); // 2932896
Could you add information on how these numbers were produced? I understand the principle, but counting number of leap years in a span of years might be a bit non-trivial.
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@16
PS2, Line 16: Check
nit: Checks
The idea is to use the same form of verbs in doc comments throughout the methods.
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@22
PS2, Line 22: Date value
Does it make sense to provide the value in the error message?
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@27
PS2, Line 27: number days
number of days
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/main/java/org/apache/kudu/util/DateUtil.java@57
PS2, Line 57: return LocalDate.ofEpochDay(days).toString();
Does it make sense to call checkDateWithinRange() here as it's done in epochDaysToSqlDate() method?
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@653
PS2, Line 653: timestamp
date ?
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java:
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java@440
PS2, Line 440: addDate
Does it make sense to add negative scenarios for PartialRow.addDate() as well (i.e. adding a value not representable as DATE)?
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java:
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java@156
PS2, Line 156:
nit: trailing spaces
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java
File java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java:
http://gerrit.cloudera.org:8080/#/c/14517/2/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java@329
PS2, Line 329:
trailing spaces
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 2
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 13 Nov 2019 18:50:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Volodymyr Verovkin (Code Review)" <ge...@cloudera.org>.
Volodymyr Verovkin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 14:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14517/13/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala
File java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala:
http://gerrit.cloudera.org:8080/#/c/14517/13/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala@302
PS13, Line 302: .toString // TODO: Explicitly control print format
: case Type.DATE =>
> I'm not a Scala expert, but this looks a bit strange.
Actually, behavior of this function is same like https://github.com/apache/kudu/blob/0a555961c6dfa672bfc2d90d4fb26d0bff21c299/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java#L1149
And there we read comment "Returns the string value of serialized value according to the type of column."
Thus, the type must Int.
Fixed.
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 14
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 20:42:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14517 )
Change subject: [KUDU-2632] Add a DATE type backed by INT32 (Part 2, Java client)
......................................................................
Patch Set 16:
Tests fail:
java.lang.IllegalArgumentException: date isn't [Type: int32], it's date
at org.apache.kudu.client.PartialRow.checkColumn(PartialRow.java:1228) ~[main/:?]
at org.apache.kudu.client.PartialRow.getInt(PartialRow.java:316) ~[main/:?]
at org.apache.kudu.client.PartialRow.getInt(PartialRow.java:304) ~[main/:?]
at org.apache.kudu.client.TestPartialRow.testSetMin(TestPartialRow.java:474) ~[test/:?]
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_141]
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_141]
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_141]
--
To view, visit http://gerrit.cloudera.org:8080/14517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a09f4f9c804c1b2965e78da78528225a9c769e2
Gerrit-Change-Number: 14517
Gerrit-PatchSet: 16
Gerrit-Owner: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Feb 2020 17:49:02 +0000
Gerrit-HasComments: No