You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2018/02/20 16:49:33 UTC

[kudu-CR] KUDU-721: [Flume] Add DECIMAL type support

Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9365


Change subject: KUDU-721: [Flume] Add DECIMAL type support
......................................................................

KUDU-721: [Flume] Add DECIMAL type support

Adds decimal column support to the Flume KuduSink
including the Regex and Avro operations producers.

Change-Id: Ibc02d683dd1bce2cc0de255e4d072436b6c0163a
---
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-flume-sink/src/test/avro/testAvroKuduOperationsProducer.avsc
M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java
5 files changed, 61 insertions(+), 23 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibc02d683dd1bce2cc0de255e4d072436b6c0163a
Gerrit-Change-Number: 9365
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@gmail.com>

[kudu-CR] KUDU-721: [Flume] Add DECIMAL type support

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9365 )

Change subject: KUDU-721: [Flume] Add DECIMAL type support
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9365/2/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
File java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java:

http://gerrit.cloudera.org:8080/#/c/9365/2/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java@154
PS2, Line 154: 9, 1
> It would coerce that value if possible and fail if it can't coerce without 
Great, thanks!



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc02d683dd1bce2cc0de255e4d072436b6c0163a
Gerrit-Change-Number: 9365
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Wed, 21 Feb 2018 21:14:38 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-721: [Flume] Add DECIMAL type support

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9365 )

Change subject: KUDU-721: [Flume] Add DECIMAL type support
......................................................................

KUDU-721: [Flume] Add DECIMAL type support

Adds decimal column support to the Flume KuduSink
including the Regex and Avro operations producers.

Note: Sets enableDecimalLogicalType to true in the
Maven Avro plugin for code generation because the
default is false.

Change-Id: Ibc02d683dd1bce2cc0de255e4d072436b6c0163a
Reviewed-on: http://gerrit.cloudera.org:8080/9365
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
M java/kudu-flume-sink/pom.xml
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-flume-sink/src/test/avro/testAvroKuduOperationsProducer.avsc
M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java
6 files changed, 64 insertions(+), 23 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Mike Percy: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibc02d683dd1bce2cc0de255e4d072436b6c0163a
Gerrit-Change-Number: 9365
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-721: [Flume] Add DECIMAL type support

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, 

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

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

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

Change subject: KUDU-721: [Flume] Add DECIMAL type support
......................................................................

KUDU-721: [Flume] Add DECIMAL type support

Adds decimal column support to the Flume KuduSink
including the Regex and Avro operations producers.

Note: Sets enableDecimalLogicalType to true in the
Maven Avro plugin for code generation because the
default is false.

Change-Id: Ibc02d683dd1bce2cc0de255e4d072436b6c0163a
---
M java/kudu-flume-sink/pom.xml
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-flume-sink/src/test/avro/testAvroKuduOperationsProducer.avsc
M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java
6 files changed, 64 insertions(+), 23 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibc02d683dd1bce2cc0de255e4d072436b6c0163a
Gerrit-Change-Number: 9365
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-721: [Flume] Add DECIMAL type support

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9365 )

Change subject: KUDU-721: [Flume] Add DECIMAL type support
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9365/2/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
File java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java:

http://gerrit.cloudera.org:8080/#/c/9365/2/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java@154
PS2, Line 154: 9, 1
> what would happen if the decimal scale and precision didn't match between a
It would coerce that value if possible and fail if it can't coerce without rounding. In PartialRow.addDecimal, DecimalUtil.coerce is called which handles this. 

In TestPartialRow testAddInvalidPrecisionDecimal, testAddInvalidScaleDecimal, testAddInvalidCoercedScaleDecimal, and testAddCoercedScaleAndPrecisionDecimal verify that behavior.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc02d683dd1bce2cc0de255e4d072436b6c0163a
Gerrit-Change-Number: 9365
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Wed, 21 Feb 2018 21:04:42 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-721: [Flume] Add DECIMAL type support

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9365 )

Change subject: KUDU-721: [Flume] Add DECIMAL type support
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9365/2/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
File java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java:

http://gerrit.cloudera.org:8080/#/c/9365/2/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java@154
PS2, Line 154: 9, 1
what would happen if the decimal scale and precision didn't match between avro and kudu?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc02d683dd1bce2cc0de255e4d072436b6c0163a
Gerrit-Change-Number: 9365
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Wed, 21 Feb 2018 20:53:03 +0000
Gerrit-HasComments: Yes