[CARBONDATA-3017] Map DDL Support #2980

manishnalla1994 · 2018-12-07T14:49:04Z

Support Create DDL for Map type.
This PR is dependant on PR#2979 for the change of delimiters.

Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:

Any interfaces changed?
Any backward compatibility impacted?
Document update required?
Testing done
Please provide details on
- Whether new unit test cases have been added or why no new tests are required?
- How it is tested? Please attach test report.
- Is it a performance related change? Please attach the performance test report.
- Any additional information to help reviewers in testing this change.
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

CarbonDataQA · 2018-12-07T15:00:12Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1657/

CarbonDataQA · 2018-12-07T15:45:23Z

Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9917/

CarbonDataQA · 2018-12-07T15:59:09Z

Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1869/

qiuchenjian · 2018-12-08T03:07:55Z

...essing/src/main/java/org/apache/carbondata/processing/loading/parser/impl/RowParserImpl.java

        (String[]) configuration.getDataLoadProperty(DataLoadProcessorConstants.COMPLEX_DELIMITERS);
+    Queue<String> complexDelimiters = new LinkedList<>();
+    for (int i = 0; i < 4; i++) {


“i < 4” the 4 is not clear， it's better to exchage is by constant

manishgupta88 · 2018-12-10T09:00:57Z

hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonTableOutputFormat.java

@@ -338,12 +338,15 @@ public static CarbonLoadModel getLoadModel(Configuration conf) throws IOExceptio
            SKIP_EMPTY_LINE,
            carbonProperty.getProperty(CarbonLoadOptionConstants.CARBON_OPTIONS_SKIP_EMPTY_LINE)));

-    String complexDelim = conf.get(COMPLEX_DELIMITERS, "$" + "," + ":");
+    String complexDelim = conf.get(COMPLEX_DELIMITERS, "$" + "," + ":" + "," + "\\\\003");


Move all the default delimters to constants and use it everywhere

manishgupta88 · 2018-12-10T09:01:52Z

examples/spark2/src/main/resources/maptest2.csv

@@ -0,0 +1,2 @@
+1\002Nalla\0012\002Singh\0011\002Gupta\0014\002Kumar
+10\002Nallaa\00120\002Sissngh\001100\002Gusspta\00140\002Kumar


Move the above newly added files to spark-common-test

manishgupta88 · 2018-12-10T09:57:32Z

hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonTableOutputFormat.java

    String[] split = complexDelim.split(",");
    model.setComplexDelimiterLevel1(split[0]);
    if (split.length > 1) {
      model.setComplexDelimiterLevel2(split[1]);
    }
+    if (split.length > 2) {
+      model.setComplexDelimiterLevel3(split[2]);
+    }


Modify the above code as below
if (split.length > 2) { model.setComplexDelimiterLevel2(split[1]); model.setComplexDelimiterLevel3(split[2]); } else if (split.length > 1) { model.setComplexDelimiterLevel2(split[1]); }

manishgupta88 · 2018-12-10T10:02:02Z

...scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateDDLForComplexMapType.scala

+      if (dimensionRawColumnChunk.getDataChunkV3
+        .isSetLocal_dictionary) {
+        isLocalDictionaryGenerated = true
+      }


You can directly use scala filter operation to check for local dictionary

manishgupta88 · 2018-12-10T10:12:01Z

...src/main/scala/org/apache/spark/sql/execution/command/management/CarbonLoadDataCommand.scala

@@ -188,11 +188,13 @@ case class CarbonLoadDataCommand(
    val carbonLoadModel = new CarbonLoadModel()
    val tableProperties = table.getTableInfo.getFactTable.getTableProperties
    val optionsFinal = LoadOption.fillOptionWithDefaultValue(options.asJava)
+    // These two delimiters are non configurable and hardcoded for map type
+    optionsFinal.put("complex_delimiter_level_3", "\003")
+    optionsFinal.put("complex_delimiter_level_4", "\004")


Remove hardcoded values. Create one ComplexDelimeterEnum and use the values from there

manishgupta88 · 2018-12-10T10:13:46Z

processing/src/main/java/org/apache/carbondata/processing/loading/DataLoadProcessBuilder.java

-        new String[] { loadModel.getComplexDelimiterLevel1(),
-            loadModel.getComplexDelimiterLevel2() });
+        new String[] { loadModel.getComplexDelimiterLevel1(), loadModel.getComplexDelimiterLevel2(),
+            loadModel.getComplexDelimiterLevel3(), loadModel.getComplexDelimiterLevel4() });


Instead of adding these many delimeters methods in loadmodel, create a list or array of complex delimeters in loadmodel and then use it here

manishgupta88 · 2018-12-10T10:14:20Z

processing/src/main/java/org/apache/carbondata/processing/loading/model/CarbonLoadModel.java

@@ -631,7 +651,7 @@ public void setFactTimeStamp(long factTimeStamp) {
  }

  public String[] getDelimiters() {
-    return new String[] { complexDelimiterLevel1, complexDelimiterLevel2 };
+    return new String[] { complexDelimiterLevel1, complexDelimiterLevel2, complexDelimiterLevel3 };


Modify this class and related changes in other classes as per the above comment

manishgupta88 · 2018-12-10T10:15:25Z

processing/src/main/java/org/apache/carbondata/processing/loading/model/LoadOption.java

+    optionsFinal.put(
+        "complex_delimiter_level_3",
+        Maps.getOrDefault(options, "complex_delimiter_level_3", "\\\\003"));
+


Remove hardcoding

manishgupta88 · 2018-12-10T10:16:48Z

...ssing/src/main/java/org/apache/carbondata/processing/loading/parser/CarbonParserFactory.java

      String nullFormat, int depth) {
+    if (depth > 2) {
+      return null;


I think we should throw exception with proper error message if depth is more than 2

manishgupta88 · 2018-12-10T10:19:28Z

...essing/src/main/java/org/apache/carbondata/processing/loading/parser/impl/RowParserImpl.java

        (String[]) configuration.getDataLoadProperty(DataLoadProcessorConstants.COMPLEX_DELIMITERS);
+    Queue<String> complexDelimiters = new LinkedList<>();
+    for (int i = 0; i < 4; i++) {


Avoid using hardcoded values like 4 in the loops while coding....instead iterate over tempComplexDelimiters length

CarbonDataQA · 2018-12-11T12:36:52Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1705/

CarbonDataQA · 2018-12-11T12:38:19Z

Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9965/

CarbonDataQA · 2018-12-11T12:39:27Z

Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1915/

CarbonDataQA · 2018-12-11T13:19:27Z

Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1916/

CarbonDataQA · 2018-12-11T13:21:53Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1706/

CarbonDataQA · 2018-12-11T13:23:24Z

Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9966/

CarbonDataQA · 2018-12-11T13:24:21Z

Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1917/

CarbonDataQA · 2018-12-12T04:39:21Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1712/

CarbonDataQA · 2018-12-12T05:16:53Z

Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9972/

CarbonDataQA · 2018-12-12T05:18:29Z

Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1923/

manishgupta88 · 2018-12-12T05:32:29Z

processing/src/main/java/org/apache/carbondata/processing/loading/model/CarbonLoadModel.java

@@ -65,8 +65,7 @@
  private String csvHeader;
  private String[] csvHeaderColumns;
  private String csvDelimiter;
-  private String complexDelimiterLevel1;
-  private String complexDelimiterLevel2;
+  private ArrayList<String> complexDelimiters = new ArrayList<>();


We can do lazy initialization in the setter method. This will avoid extra memory being consumed for non complex type schemas

manishgupta88 · 2018-12-12T05:33:48Z

processing/src/main/java/org/apache/carbondata/processing/loading/model/CarbonLoadModel.java

-    return new String[] { complexDelimiterLevel1, complexDelimiterLevel2 };
+    String[] delimiters = new String[complexDelimiters.size()];
+    delimiters = complexDelimiters.toArray(delimiters);
+    return delimiters;


This method is not required. Conversion can be done whereever required

CarbonDataQA · 2018-12-12T06:59:28Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1716/

CarbonDataQA · 2018-12-12T07:33:51Z

Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9976/

CarbonDataQA · 2018-12-12T07:38:53Z

Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1928/

CarbonDataQA · 2018-12-12T09:54:20Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1719/

manishgupta88 · 2018-12-12T10:28:46Z

LGTM...check for test case failures if any and fix

CarbonDataQA · 2018-12-12T10:58:13Z

Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9979/

CarbonDataQA · 2018-12-12T11:20:01Z

Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1931/

CarbonDataQA · 2018-12-12T11:25:18Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1722/

CarbonDataQA · 2018-12-12T11:39:29Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1723/

CarbonDataQA · 2018-12-12T12:37:42Z

Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9983/

CarbonDataQA · 2018-12-12T13:00:18Z

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

CarbonDataQA · 2018-12-13T05:39:02Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1727/

CarbonDataQA · 2018-12-13T06:35:10Z

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

CarbonDataQA · 2018-12-13T06:37:53Z

Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9987/

CarbonDataQA · 2018-12-14T08:31:05Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1757/

CarbonDataQA · 2018-12-14T09:19:08Z

Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1968/

CarbonDataQA · 2018-12-14T09:38:10Z

Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10016/

CarbonDataQA · 2018-12-14T09:53:59Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1758/

CarbonDataQA · 2018-12-14T10:53:35Z

Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10017/

CarbonDataQA · 2018-12-14T10:53:54Z

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

manishgupta88 · 2018-12-14T11:55:17Z

@manishnalla1994 ...Please rebase and remove the commit for PR 2979 as it is already merged

CarbonDataQA · 2018-12-14T13:43:07Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1764/

CarbonDataQA · 2018-12-14T13:51:38Z

Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10023/

CarbonDataQA · 2018-12-14T13:57:45Z

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

manishgupta88 · 2018-12-14T16:59:30Z

LGTM

Support Create DDL for Map type. This closes #2980

Support Create DDL for Map type. This closes apache#2980

qiuchenjian reviewed Dec 8, 2018

View reviewed changes

manishgupta88 suggested changes Dec 10, 2018

View reviewed changes

manishnalla1994 force-pushed the MapDDL5Dec branch from 4cc8ba1 to 8c186cc Compare December 11, 2018 12:33

manishnalla1994 force-pushed the MapDDL5Dec branch 2 times, most recently from 473f695 to 0ace143 Compare December 11, 2018 13:17

manishnalla1994 force-pushed the MapDDL5Dec branch from 0ace143 to d57e326 Compare December 12, 2018 04:28

manishgupta88 reviewed Dec 12, 2018

View reviewed changes

manishnalla1994 force-pushed the MapDDL5Dec branch 2 times, most recently from 1d18435 to b0fa65c Compare December 12, 2018 06:49

manishnalla1994 force-pushed the MapDDL5Dec branch from b0fa65c to 7843195 Compare December 12, 2018 09:44

manishnalla1994 force-pushed the MapDDL5Dec branch from 7843195 to 72ec8c3 Compare December 12, 2018 11:10

manishnalla1994 force-pushed the MapDDL5Dec branch from 72ec8c3 to c8ffa07 Compare December 12, 2018 11:25

manishnalla1994 force-pushed the MapDDL5Dec branch from c8ffa07 to 0a84d52 Compare December 13, 2018 05:26

manishnalla1994 force-pushed the MapDDL5Dec branch from 0a84d52 to db02b72 Compare December 14, 2018 08:07

manishnalla1994 force-pushed the MapDDL5Dec branch from db02b72 to fe4fd98 Compare December 14, 2018 09:42

MapDDLSupport

d200a65

manishnalla1994 force-pushed the MapDDL5Dec branch from fe4fd98 to d200a65 Compare December 14, 2018 12:01

asfgit closed this in 90f63a0 Dec 14, 2018

asfgit pushed a commit that referenced this pull request Dec 17, 2018

[CARBONDATA-3017] Map DDL Support

c9b136c

Support Create DDL for Map type. This closes #2980

qiuchenjian pushed a commit to qiuchenjian/carbondata that referenced this pull request Jun 14, 2019

[CARBONDATA-3017] Map DDL Support

43dd0ff

Support Create DDL for Map type. This closes apache#2980

		@@ -0,0 +1,2 @@
		1\002Nalla\0012\002Singh\0011\002Gupta\0014\002Kumar
		10\002Nallaa\00120\002Sissngh\001100\002Gusspta\00140\002Kumar

[CARBONDATA-3017] Map DDL Support #2980

[CARBONDATA-3017] Map DDL Support #2980

Conversation

manishnalla1994 commented Dec 7, 2018

CarbonDataQA commented Dec 7, 2018

CarbonDataQA commented Dec 7, 2018

CarbonDataQA commented Dec 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CarbonDataQA commented Dec 11, 2018

CarbonDataQA commented Dec 11, 2018

CarbonDataQA commented Dec 11, 2018

CarbonDataQA commented Dec 11, 2018

CarbonDataQA commented Dec 11, 2018

CarbonDataQA commented Dec 11, 2018

CarbonDataQA commented Dec 11, 2018

CarbonDataQA commented Dec 12, 2018

CarbonDataQA commented Dec 12, 2018

CarbonDataQA commented Dec 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CarbonDataQA commented Dec 12, 2018

CarbonDataQA commented Dec 12, 2018

CarbonDataQA commented Dec 12, 2018

CarbonDataQA commented Dec 12, 2018

manishgupta88 commented Dec 12, 2018

CarbonDataQA commented Dec 12, 2018

CarbonDataQA commented Dec 12, 2018

CarbonDataQA commented Dec 12, 2018

CarbonDataQA commented Dec 12, 2018

CarbonDataQA commented Dec 12, 2018

CarbonDataQA commented Dec 12, 2018

CarbonDataQA commented Dec 13, 2018

CarbonDataQA commented Dec 13, 2018

CarbonDataQA commented Dec 13, 2018

CarbonDataQA commented Dec 14, 2018

CarbonDataQA commented Dec 14, 2018

CarbonDataQA commented Dec 14, 2018

CarbonDataQA commented Dec 14, 2018

CarbonDataQA commented Dec 14, 2018

CarbonDataQA commented Dec 14, 2018

manishgupta88 commented Dec 14, 2018

CarbonDataQA commented Dec 14, 2018

CarbonDataQA commented Dec 14, 2018

CarbonDataQA commented Dec 14, 2018

manishgupta88 commented Dec 14, 2018