Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CARBONDATA-2869] Add support for Avro Map data type support for SDK #2649

Closed
wants to merge 1 commit into from
Closed

[CARBONDATA-2869] Add support for Avro Map data type support for SDK #2649

wants to merge 1 commit into from

Conversation

manishgupta88
Copy link
Contributor

@manishgupta88 manishgupta88 commented Aug 23, 2018 •
edited

This PR supports Avro Map data type support. It supports reading and writing complex map data type data using SDK.

For reading and writing Map data type using SDK refer the below test classes for reference

  1. TestNonTransactionalCarbonTableForMapType.scala
  2. CarbonReaderTest.java
  • Any interfaces changed?
    No
  • Any backward compatibility impacted?
    No
  • Document update required?
    No
  • Testing done
    Add test cases for verification
  • For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
    NA

@ravipesala
Copy link
Contributor

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6330/

@CarbonDataQA
Copy link

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7982/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6705/

@jackylk
Copy link
Contributor

jackylk commented Aug 23, 2018 •
edited

Is this only for SDK or can be support in SQL CREATE TABLE also?

@jackylk
Copy link
Contributor

jackylk commented Aug 23, 2018

Is map implemented as an array of struct<key, value>?

@manishgupta88
Copy link
Contributor Author

@jackylk

  1. This PR is only for SDK support
  2. Yes map is implemented as an array of struct<key, value>. For more details you can check the design document
    https://docs.google.com/document/d/1HHe2fdkIh3Jyz1y3494_2kGRSc4muTWuilAmwg5lpVw/edit?usp=sharing

@ravipesala
Copy link
Contributor

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6355/

@CarbonDataQA
Copy link

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8004/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6727/

@manishgupta88 manishgupta88 changed the title [WIP] Add support for Avro Map data type support for SDK [CARBONDATA-2869] Add support for Avro Map data type support for SDK Aug 24, 2018
@ravipesala
Copy link
Contributor

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6386/

@CarbonDataQA
Copy link

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8040/

@CarbonDataQA
Copy link

Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6762/

@ravipesala
Copy link
Contributor

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6395/

@CarbonDataQA
Copy link

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8049/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6771/

@ravipesala
Copy link
Contributor

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6417/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8080/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/21/

@manishgupta88 manishgupta88 mentioned this pull request Aug 27, 2018
Closed
5 tasks
ravipesala
ravipesala reviewed Aug 29, 2018
View reviewed changes
core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilder.java Outdated
@@ -232,6 +235,10 @@ private ColumnSchema addColumn(StructField field, String parentName, AtomicInteg
for (int i = 0; i < fields.size(); i++) {
addColumn(fields.get(i), parentFieldName, valIndex, false, true);
}
} else if (field.getDataType().getName().equalsIgnoreCase("MAP")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use DataTypes.isMapType(field.getDataType())

ravipesala
ravipesala reviewed Aug 29, 2018
View reviewed changes
core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableSchemaBuilder.java Outdated
@@ -195,7 +196,8 @@ private ColumnSchema addColumn(StructField field, String parentName, AtomicInteg
newColumn.setColumnReferenceId(newColumn.getColumnUniqueId());
newColumn.setEncodingList(createEncoding(field.getDataType(), isSortColumn, isComplexChild));
if (field.getDataType().isComplexType()) {
if (field.getDataType().getName().equalsIgnoreCase("ARRAY")) {
if (field.getDataType().getName().equalsIgnoreCase("ARRAY") || field.getDataType().getName()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use DataTypes.isArrayType(field.getDataType())

ravipesala
ravipesala reviewed Aug 29, 2018
View reviewed changes
core/src/main/java/org/apache/carbondata/core/scan/complextypes/MapQueryType.java Outdated
super(name, parentname, blockIndex);
}

@Override public Object getDataBasedOnDataType(ByteBuffer dataBuffer) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add comment to tell how we convert array to map

ravipesala
ravipesala reviewed Aug 29, 2018
View reviewed changes
store/sdk/src/main/java/org/apache/carbondata/sdk/file/AvroCarbonWriter.java Outdated
Map mapEntries = (HashMap) fieldValue;
Object[] arrayMapChildObjects = new Object[mapEntries.size()];
if (!mapEntries.isEmpty()) {
Iterator iterator = ((HashMap) fieldValue).entrySet().iterator();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use mapEntries directly instead of typecast again

@manishgupta88
Copy link
Contributor Author

@ravipesala ...fixed review comment. Kindly review and merge

@ravipesala
Copy link
Contributor

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6451/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/66/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8137/

@ravipesala
Copy link
Contributor

LGTM

@asfgit asfgit closed this in fb6dffe Aug 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ravipesala ravipesala ravipesala left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@manishgupta88 @ravipesala @CarbonDataQA @jackylk