You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by GitBox <gi...@apache.org> on 2020/01/07 11:00:46 UTC
[GitHub] [carbondata] niuge01 opened a new pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
niuge01 opened a new pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564
### Why is this PR needed?
Currently, only support set string with delimiter as struct field value, sometime it doesn't work very well on struct<binary> field.
### What changes were proposed in this PR?
Support set base64 string as struct<binary> field value
### Does this PR introduce any user interface change?
- No
### Is any new testcase added?
- Yes
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r364030859
##########
File path: integration/flink/src/main/java/org/apache/carbon/flink/CarbonLocalWriter.java
##########
@@ -51,19 +52,29 @@
) {
super(factory, identifier, table);
final Properties writerProperties = factory.getConfiguration().getWriterProperties();
+ final Properties carbonProperties = factory.getConfiguration().getCarbonProperties();
final String commitThreshold =
writerProperties.getProperty(CarbonLocalProperty.COMMIT_THRESHOLD);
this.writerFactory = new WriterFactory(table, writePath) {
@Override
protected org.apache.carbondata.sdk.file.CarbonWriter newWriter(
final Object[] row) {
try {
- return org.apache.carbondata.sdk.file.CarbonWriter.builder()
+ final CarbonWriterBuilder writerBuilder =
+ org.apache.carbondata.sdk.file.CarbonWriter.builder()
.outputPath(super.getWritePath(row))
.writtenBy("flink")
.withSchemaFile(CarbonTablePath.getSchemaFilePath(table.getTablePath()))
- .withCsvInput()
- .build();
+ .withCsvInput();
+ for (String propertyName : carbonProperties.stringPropertyNames()) {
+ try {
+ writerBuilder.withLoadOption(propertyName,
+ carbonProperties.getProperty(propertyName));
+ } catch (IllegalArgumentException ignore) {
+ // Ignore.
Review comment:
suggest to print a log to warm the user
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r363713592
##########
File path: streaming/src/main/scala/org/apache/carbondata/streaming/parser/FieldConverter.scala
##########
@@ -63,16 +66,22 @@ object FieldConverter {
case b: java.lang.Boolean => b.toString
case s: java.lang.Short => s.toString
case f: java.lang.Float => f.toString
- case bs: Array[Byte] => new String(bs,
- Charset.forName(CarbonCommonConstants.DEFAULT_CHARSET))
+ case bs: Array[Byte] =>
+ if (isInsertFlow) {
+ Base64.getEncoder.encodeToString(bs)
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] QiangCai commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
QiangCai commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r364079770
##########
File path: integration/flink/src/main/java/org/apache/carbon/flink/CarbonS3Writer.java
##########
@@ -54,20 +55,31 @@
) {
super(factory, identifier, table);
final Properties writerProperties = factory.getConfiguration().getWriterProperties();
+ final Properties carbonProperties = factory.getConfiguration().getCarbonProperties();
final String commitThreshold =
writerProperties.getProperty(CarbonS3Property.COMMIT_THRESHOLD);
this.writerFactory = new WriterFactory(table, writePath) {
@Override
protected org.apache.carbondata.sdk.file.CarbonWriter newWriter(
final Object[] row) {
try {
- return org.apache.carbondata.sdk.file.CarbonWriter.builder()
+ final CarbonWriterBuilder writerBuilder =
+ org.apache.carbondata.sdk.file.CarbonWriter.builder()
.outputPath(super.getWritePath(row))
.writtenBy("flink")
.withSchemaFile(CarbonTablePath.getSchemaFilePath(table.getTablePath()))
.withCsvInput()
- .withHadoopConf(configuration)
- .build();
+ .withHadoopConf(configuration);
+ for (String propertyName : carbonProperties.stringPropertyNames()) {
Review comment:
use withLoadOptions with a map parameter
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] QiangCai commented on issue #3564: [CARBONDATA-3655]
Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
QiangCai commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571952639
LGTM
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] Zhangshunyu commented on a change in pull request
#3564: [CARBONDATA-3655] Support set base64 string as struct field
value
Posted by GitBox <gi...@apache.org>.
Zhangshunyu commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r363709880
##########
File path: integration/spark-common/src/main/scala/org/apache/carbondata/spark/util/CarbonScalaUtil.scala
##########
@@ -70,7 +70,7 @@ object CarbonScalaUtil {
level: Int = 0): String = {
FieldConverter.objectToString(value, serializationNullFormat, complexDelimiters,
timeStampFormat, dateFormat, isVarcharType = isVarcharType, isComplexType = isComplexType,
- level)
+ level, true)
Review comment:
this method is an static method in Util, why the last input paramer 'insertflow' always use 'true' ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] Zhangshunyu commented on a change in pull request
#3564: [CARBONDATA-3655] Support set base64 string as struct field
value
Posted by GitBox <gi...@apache.org>.
Zhangshunyu commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r363709880
##########
File path: integration/spark-common/src/main/scala/org/apache/carbondata/spark/util/CarbonScalaUtil.scala
##########
@@ -70,7 +70,7 @@ object CarbonScalaUtil {
level: Int = 0): String = {
FieldConverter.objectToString(value, serializationNullFormat, complexDelimiters,
timeStampFormat, dateFormat, isVarcharType = isVarcharType, isComplexType = isComplexType,
- level)
+ level, true)
Review comment:
this method is a static method in Util, why the last input paramer 'insertflow' always use 'true' ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571890723
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1521/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571924642
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1528/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r363716048
##########
File path: streaming/src/main/scala/org/apache/carbondata/streaming/parser/FieldConverter.scala
##########
@@ -63,16 +66,22 @@ object FieldConverter {
case b: java.lang.Boolean => b.toString
case s: java.lang.Short => s.toString
case f: java.lang.Float => f.toString
- case bs: Array[Byte] => new String(bs,
- Charset.forName(CarbonCommonConstants.DEFAULT_CHARSET))
+ case bs: Array[Byte] =>
+ if (isInsertFlow) {
+ Base64.getEncoder.encodeToString(bs)
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on issue #3564: [CARBONDATA-3655]
Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571539841
please test this
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on issue #3564: [CARBONDATA-3655]
Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571876003
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on issue #3564: [CARBONDATA-3655]
Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571763295
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on issue #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571856857
@niuge01 : why not support this for Array and MAP also ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r364051255
##########
File path: integration/flink/src/main/java/org/apache/carbon/flink/CarbonLocalWriter.java
##########
@@ -51,19 +52,29 @@
) {
super(factory, identifier, table);
final Properties writerProperties = factory.getConfiguration().getWriterProperties();
+ final Properties carbonProperties = factory.getConfiguration().getCarbonProperties();
final String commitThreshold =
writerProperties.getProperty(CarbonLocalProperty.COMMIT_THRESHOLD);
this.writerFactory = new WriterFactory(table, writePath) {
@Override
protected org.apache.carbondata.sdk.file.CarbonWriter newWriter(
final Object[] row) {
try {
- return org.apache.carbondata.sdk.file.CarbonWriter.builder()
+ final CarbonWriterBuilder writerBuilder =
+ org.apache.carbondata.sdk.file.CarbonWriter.builder()
.outputPath(super.getWritePath(row))
.writtenBy("flink")
.withSchemaFile(CarbonTablePath.getSchemaFilePath(table.getTablePath()))
- .withCsvInput()
- .build();
+ .withCsvInput();
+ for (String propertyName : carbonProperties.stringPropertyNames()) {
+ try {
+ writerBuilder.withLoadOption(propertyName,
+ carbonProperties.getProperty(propertyName));
+ } catch (IllegalArgumentException ignore) {
+ // Ignore.
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] QiangCai commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
QiangCai commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r364079697
##########
File path: integration/flink/src/main/java/org/apache/carbon/flink/CarbonLocalWriter.java
##########
@@ -51,19 +52,30 @@
) {
super(factory, identifier, table);
final Properties writerProperties = factory.getConfiguration().getWriterProperties();
+ final Properties carbonProperties = factory.getConfiguration().getCarbonProperties();
final String commitThreshold =
writerProperties.getProperty(CarbonLocalProperty.COMMIT_THRESHOLD);
this.writerFactory = new WriterFactory(table, writePath) {
@Override
protected org.apache.carbondata.sdk.file.CarbonWriter newWriter(
final Object[] row) {
try {
- return org.apache.carbondata.sdk.file.CarbonWriter.builder()
+ final CarbonWriterBuilder writerBuilder =
+ org.apache.carbondata.sdk.file.CarbonWriter.builder()
.outputPath(super.getWritePath(row))
.writtenBy("flink")
.withSchemaFile(CarbonTablePath.getSchemaFilePath(table.getTablePath()))
- .withCsvInput()
- .build();
+ .withCsvInput();
+ for (String propertyName : carbonProperties.stringPropertyNames()) {
Review comment:
use withLoadOptions with a map parameter
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571750466
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1515/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571874636
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1519/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571796245
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1516/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on issue #3564: [CARBONDATA-3655]
Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571857428
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] QiangCai commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
QiangCai commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r364082467
##########
File path: streaming/src/main/scala/org/apache/carbondata/streaming/parser/FieldConverter.scala
##########
@@ -63,16 +66,22 @@ object FieldConverter {
case b: java.lang.Boolean => b.toString
case s: java.lang.Short => s.toString
case f: java.lang.Float => f.toString
- case bs: Array[Byte] => new String(bs,
- Charset.forName(CarbonCommonConstants.DEFAULT_CHARSET))
+ case bs: Array[Byte] =>
+ if ("base64".equalsIgnoreCase(binaryCodec)) {
Review comment:
how about to use BinaryDecoder
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r364030859
##########
File path: integration/flink/src/main/java/org/apache/carbon/flink/CarbonLocalWriter.java
##########
@@ -51,19 +52,29 @@
) {
super(factory, identifier, table);
final Properties writerProperties = factory.getConfiguration().getWriterProperties();
+ final Properties carbonProperties = factory.getConfiguration().getCarbonProperties();
final String commitThreshold =
writerProperties.getProperty(CarbonLocalProperty.COMMIT_THRESHOLD);
this.writerFactory = new WriterFactory(table, writePath) {
@Override
protected org.apache.carbondata.sdk.file.CarbonWriter newWriter(
final Object[] row) {
try {
- return org.apache.carbondata.sdk.file.CarbonWriter.builder()
+ final CarbonWriterBuilder writerBuilder =
+ org.apache.carbondata.sdk.file.CarbonWriter.builder()
.outputPath(super.getWritePath(row))
.writtenBy("flink")
.withSchemaFile(CarbonTablePath.getSchemaFilePath(table.getTablePath()))
- .withCsvInput()
- .build();
+ .withCsvInput();
+ for (String propertyName : carbonProperties.stringPropertyNames()) {
+ try {
+ writerBuilder.withLoadOption(propertyName,
+ carbonProperties.getProperty(propertyName));
+ } catch (IllegalArgumentException ignore) {
+ // Ignore.
Review comment:
suggest to print a long to warm the user
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] Zhangshunyu commented on a change in pull request
#3564: [CARBONDATA-3655] Support set base64 string as struct field
value
Posted by GitBox <gi...@apache.org>.
Zhangshunyu commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r363706267
##########
File path: streaming/src/main/scala/org/apache/carbondata/streaming/parser/FieldConverter.scala
##########
@@ -63,16 +66,22 @@ object FieldConverter {
case b: java.lang.Boolean => b.toString
case s: java.lang.Short => s.toString
case f: java.lang.Float => f.toString
- case bs: Array[Byte] => new String(bs,
- Charset.forName(CarbonCommonConstants.DEFAULT_CHARSET))
+ case bs: Array[Byte] =>
+ if (isInsertFlow) {
+ Base64.getEncoder.encodeToString(bs)
Review comment:
could you pls add some comment here about the reason in case 'isInsertFlow' use base64
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571583737
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1505/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r364092466
##########
File path: integration/flink/src/main/java/org/apache/carbon/flink/CarbonLocalWriter.java
##########
@@ -51,19 +52,30 @@
) {
super(factory, identifier, table);
final Properties writerProperties = factory.getConfiguration().getWriterProperties();
+ final Properties carbonProperties = factory.getConfiguration().getCarbonProperties();
final String commitThreshold =
writerProperties.getProperty(CarbonLocalProperty.COMMIT_THRESHOLD);
this.writerFactory = new WriterFactory(table, writePath) {
@Override
protected org.apache.carbondata.sdk.file.CarbonWriter newWriter(
final Object[] row) {
try {
- return org.apache.carbondata.sdk.file.CarbonWriter.builder()
+ final CarbonWriterBuilder writerBuilder =
+ org.apache.carbondata.sdk.file.CarbonWriter.builder()
.outputPath(super.getWritePath(row))
.writtenBy("flink")
.withSchemaFile(CarbonTablePath.getSchemaFilePath(table.getTablePath()))
- .withCsvInput()
- .build();
+ .withCsvInput();
+ for (String propertyName : carbonProperties.stringPropertyNames()) {
Review comment:
The withLoadOptions method requires all property are load option, but carbonProperties may have some unsupported non-load option.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on issue #3564: [CARBONDATA-3655]
Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571557965
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571700026
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1512/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] asfgit closed pull request #3564: [CARBONDATA-3655]
Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on issue #3564: [CARBONDATA-3655]
Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571842368
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571954986
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1529/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r364090191
##########
File path: streaming/src/main/scala/org/apache/carbondata/streaming/parser/FieldConverter.scala
##########
@@ -63,16 +66,22 @@ object FieldConverter {
case b: java.lang.Boolean => b.toString
case s: java.lang.Short => s.toString
case f: java.lang.Float => f.toString
- case bs: Array[Byte] => new String(bs,
- Charset.forName(CarbonCommonConstants.DEFAULT_CHARSET))
+ case bs: Array[Byte] =>
+ if ("base64".equalsIgnoreCase(binaryCodec)) {
Review comment:
will use BinaryDecoder later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r364051276
##########
File path: integration/flink/src/main/java/org/apache/carbon/flink/CarbonS3Writer.java
##########
@@ -54,20 +55,30 @@
) {
super(factory, identifier, table);
final Properties writerProperties = factory.getConfiguration().getWriterProperties();
+ final Properties carbonProperties = factory.getConfiguration().getCarbonProperties();
final String commitThreshold =
writerProperties.getProperty(CarbonS3Property.COMMIT_THRESHOLD);
this.writerFactory = new WriterFactory(table, writePath) {
@Override
protected org.apache.carbondata.sdk.file.CarbonWriter newWriter(
final Object[] row) {
try {
- return org.apache.carbondata.sdk.file.CarbonWriter.builder()
+ final CarbonWriterBuilder writerBuilder =
+ org.apache.carbondata.sdk.file.CarbonWriter.builder()
.outputPath(super.getWritePath(row))
.writtenBy("flink")
.withSchemaFile(CarbonTablePath.getSchemaFilePath(table.getTablePath()))
.withCsvInput()
- .withHadoopConf(configuration)
- .build();
+ .withHadoopConf(configuration);
+ for (String propertyName : carbonProperties.stringPropertyNames()) {
+ try {
+ writerBuilder.withLoadOption(propertyName,
+ carbonProperties.getProperty(propertyName));
+ } catch (IllegalArgumentException ignore) {
+ // Ignore.
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on issue #3564: [CARBONDATA-3655]
Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571718103
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r363713449
##########
File path: integration/spark-common/src/main/scala/org/apache/carbondata/spark/util/CarbonScalaUtil.scala
##########
@@ -70,7 +70,7 @@ object CarbonScalaUtil {
level: Int = 0): String = {
FieldConverter.objectToString(value, serializationNullFormat, complexDelimiters,
timeStampFormat, dateFormat, isVarcharType = isVarcharType, isComplexType = isComplexType,
- level)
+ level, true)
Review comment:
Currently, the method is only invoked by insert flow, so the insertflow parameter is fixed with 'true'.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r364092501
##########
File path: integration/flink/src/main/java/org/apache/carbon/flink/CarbonS3Writer.java
##########
@@ -54,20 +55,31 @@
) {
super(factory, identifier, table);
final Properties writerProperties = factory.getConfiguration().getWriterProperties();
+ final Properties carbonProperties = factory.getConfiguration().getCarbonProperties();
final String commitThreshold =
writerProperties.getProperty(CarbonS3Property.COMMIT_THRESHOLD);
this.writerFactory = new WriterFactory(table, writePath) {
@Override
protected org.apache.carbondata.sdk.file.CarbonWriter newWriter(
final Object[] row) {
try {
- return org.apache.carbondata.sdk.file.CarbonWriter.builder()
+ final CarbonWriterBuilder writerBuilder =
+ org.apache.carbondata.sdk.file.CarbonWriter.builder()
.outputPath(super.getWritePath(row))
.writtenBy("flink")
.withSchemaFile(CarbonTablePath.getSchemaFilePath(table.getTablePath()))
.withCsvInput()
- .withHadoopConf(configuration)
- .build();
+ .withHadoopConf(configuration);
+ for (String propertyName : carbonProperties.stringPropertyNames()) {
Review comment:
The withLoadOptions method requires all property are load option, but carbonProperties may have some unsupported non-load option.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on issue #3564: [CARBONDATA-3655]
Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571879889
> @niuge01 : why not support this for Array and MAP also ?
It is also support for Array and Map.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3564:
[CARBONDATA-3655] Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#discussion_r364030946
##########
File path: integration/flink/src/main/java/org/apache/carbon/flink/CarbonS3Writer.java
##########
@@ -54,20 +55,30 @@
) {
super(factory, identifier, table);
final Properties writerProperties = factory.getConfiguration().getWriterProperties();
+ final Properties carbonProperties = factory.getConfiguration().getCarbonProperties();
final String commitThreshold =
writerProperties.getProperty(CarbonS3Property.COMMIT_THRESHOLD);
this.writerFactory = new WriterFactory(table, writePath) {
@Override
protected org.apache.carbondata.sdk.file.CarbonWriter newWriter(
final Object[] row) {
try {
- return org.apache.carbondata.sdk.file.CarbonWriter.builder()
+ final CarbonWriterBuilder writerBuilder =
+ org.apache.carbondata.sdk.file.CarbonWriter.builder()
.outputPath(super.getWritePath(row))
.writtenBy("flink")
.withSchemaFile(CarbonTablePath.getSchemaFilePath(table.getTablePath()))
.withCsvInput()
- .withHadoopConf(configuration)
- .build();
+ .withHadoopConf(configuration);
+ for (String propertyName : carbonProperties.stringPropertyNames()) {
+ try {
+ writerBuilder.withLoadOption(propertyName,
+ carbonProperties.getProperty(propertyName));
+ } catch (IllegalArgumentException ignore) {
+ // Ignore.
Review comment:
suggest to print a log to warm the user
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on issue #3564: [CARBONDATA-3655]
Support set base64 string as struct field value
Posted by GitBox <gi...@apache.org>.
niuge01 commented on issue #3564: [CARBONDATA-3655] Support set base64 string as struct<binary> field value
URL: https://github.com/apache/carbondata/pull/3564#issuecomment-571667948
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services