[CARBONDATA-1286] Change Query related RDD to use TableInfo #1146

jackylk · 2017-07-08T09:40:46Z

Currently query related RDD is using CarbonTable which is read from file system, which introduces unnecessary file read. We can pass this schema information from driver by serializing TableInfo.

CarbonDataQA · 2017-07-08T09:48:39Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2957/

CarbonDataQA · 2017-07-08T09:54:26Z

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/369/

CarbonDataQA · 2017-07-09T17:20:54Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2976/

CarbonDataQA · 2017-07-09T17:24:56Z

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/387/

CarbonDataQA · 2017-07-09T17:27:30Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2977/

CarbonDataQA · 2017-07-09T17:33:00Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2978/

CarbonDataQA · 2017-07-09T17:35:27Z

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/388/

CarbonDataQA · 2017-07-09T17:36:16Z

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/389/

CarbonDataQA · 2017-07-09T18:03:50Z

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/390/

CarbonDataQA · 2017-07-09T18:05:21Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2979/

CarbonDataQA · 2017-07-10T04:07:29Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2982/

CarbonDataQA · 2017-07-10T04:08:53Z

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/393/

CarbonDataQA · 2017-07-10T13:32:51Z

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/407/

CarbonDataQA · 2017-07-10T13:37:39Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2996/

ravipesala · 2017-07-11T03:25:06Z

core/src/main/java/org/apache/carbondata/core/datastore/block/AbstractIndex.java

@@ -69,7 +69,7 @@ public SegmentProperties getSegmentProperties() {
    return segmentProperties;
  }

-  /**
+  /**`


please remove this change

ravipesala · 2017-07-11T03:44:07Z

core/src/main/java/org/apache/carbondata/core/metadata/schema/table/CarbonTable.java

+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;


use import java.util.*;

ravipesala · 2017-07-11T03:57:10Z

core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableInfo.java

+
+  @Override
+  public void write(DataOutput out) throws IOException {
+    WritableUtil.writeString(out, databaseName);


just use out.writeUTF for all string writings, no seperate util is required

ravipesala · 2017-07-11T03:58:32Z

core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableInfo.java

+
+  @Override
+  public void readFields(DataInput in) throws IOException {
+    this.databaseName = WritableUtil.readString(in);


just use in.readUTF for reading string

ravipesala · 2017-07-11T04:00:14Z

core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableInfo.java

+    return identifier;
+  }
+
+  public byte[] serielize() throws IOException {


Typo, change to serialize

ravipesala · 2017-07-11T04:00:56Z

core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableInfo.java

+    return bao.toByteArray();
+  }
+
+  public static TableInfo deserielize(byte[] bytes) throws IOException {


Typo, change to deserialize

ravipesala · 2017-07-11T04:02:04Z

core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableSchema.java

+
+  @Override
+  public void write(DataOutput out) throws IOException {
+    WritableUtil.writeString(out, tableId);


please use out.writeUTF

ravipesala · 2017-07-11T04:04:45Z

core/src/main/java/org/apache/carbondata/core/metadata/schema/table/column/ColumnSchema.java

+    out.writeInt(scale);
+    out.writeInt(precision);
+    out.writeInt(schemaOrdinal);
+    out.writeInt(numberOfChild);


please use short instead of using int

ravipesala · 2017-07-11T04:05:28Z

core/src/main/java/org/apache/carbondata/core/metadata/schema/table/WritableUtil.java

+
+public class WritableUtil {
+
+  public static void writeString(DataOutput out, String string) throws IOException {


no need of this method, use out.writeUTF

ravipesala · 2017-07-11T04:06:06Z

hadoop/src/main/java/org/apache/carbondata/hadoop/CarbonInputFormat.java

+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;


use import java.util.*

ravipesala · 2017-07-11T04:08:03Z

hadoop/src/main/java/org/apache/carbondata/hadoop/CarbonInputFormat.java

+   * Get TableInfo object from `configuration`
+   */
+  private TableInfo getTableInfo(Configuration configuration) throws IOException {
+    String tableInfoStr = configuration.get(TABLE_INFO);


is null check required for tableInfoStr ?

It is checked inside convertStringToObject

ravipesala · 2017-07-11T04:09:32Z

hadoop/src/main/java/org/apache/carbondata/hadoop/CarbonInputFormat.java

-  /**
-   * Set list of files to access
-   */
-  public static void setFilesToAccess(Configuration configuration, List<String> validFiles) {


it seems this method is added by some out side contributor recently to set the files to read, so better keep it

CarbonDataQA · 2017-07-11T06:20:34Z

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/425/

CarbonDataQA · 2017-07-11T06:27:05Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/3013/

CarbonDataQA · 2017-07-11T08:18:30Z

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/428/

CarbonDataQA · 2017-07-11T08:19:35Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/3016/

ravipesala · 2017-07-11T08:20:15Z

LGTM

This closes #1146

This closes apache#1146

change rdd tableinfo

6597f8e

jackylk changed the base branch from master to metadata July 8, 2017 18:40

fix testcase

83e7462

jackylk force-pushed the scanrdd branch from da85942 to 83e7462 Compare July 10, 2017 13:07

jackylk changed the title ~~[WIP] Change Query related RDD to use TableInfo~~ [CARBONDATA-1286] Change Query related RDD to use TableInfo Jul 10, 2017

ravipesala reviewed Jul 11, 2017

View reviewed changes

fix comment

29e0c19

fix compile

1e5f45f

asfgit pushed a commit that referenced this pull request Jul 11, 2017

[CARBONDATA-1286] Change Query related RDD to use TableInfo

4b69c9d

This closes #1146

jackylk closed this Jul 11, 2017

sraghunandan pushed a commit to sraghunandan/carbondata-1 that referenced this pull request Jul 25, 2017

[CARBONDATA-1286] Change Query related RDD to use TableInfo

ac5aee1

This closes apache#1146


		public class WritableUtil {

		public static void writeString(DataOutput out, String string) throws IOException {

[CARBONDATA-1286] Change Query related RDD to use TableInfo #1146

[CARBONDATA-1286] Change Query related RDD to use TableInfo #1146

Conversation

jackylk commented Jul 8, 2017

CarbonDataQA commented Jul 8, 2017

CarbonDataQA commented Jul 8, 2017

CarbonDataQA commented Jul 9, 2017

CarbonDataQA commented Jul 9, 2017

CarbonDataQA commented Jul 9, 2017

CarbonDataQA commented Jul 9, 2017

CarbonDataQA commented Jul 9, 2017

CarbonDataQA commented Jul 9, 2017

CarbonDataQA commented Jul 9, 2017

CarbonDataQA commented Jul 9, 2017

CarbonDataQA commented Jul 10, 2017

CarbonDataQA commented Jul 10, 2017

CarbonDataQA commented Jul 10, 2017

CarbonDataQA commented Jul 10, 2017

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CarbonDataQA commented Jul 11, 2017

CarbonDataQA commented Jul 11, 2017

CarbonDataQA commented Jul 11, 2017

CarbonDataQA commented Jul 11, 2017

ravipesala commented Jul 11, 2017