You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@carbondata.apache.org by ku...@apache.org on 2018/11/02 10:39:13 UTC

carbondata git commit: [CARBONDATA-3061] Add validation for supported format version and Encoding type to throw proper exception to the user while reading a file

Repository: carbondata
Updated Branches:
  refs/heads/master f51f5cde1 -> fc2a53991


[CARBONDATA-3061] Add validation for supported format version and Encoding type
to throw proper exception to the user while reading a file

1. Validation for columnar format version. Exception will be thrown during reading
if specified format version is not valid to read.
2. Validation for supported encoding type. Exception will be throw during reading
if any of the written encoding is not supported for read in the current version.

This closes #2877


Project: http://git-wip-us.apache.org/repos/asf/carbondata/repo
Commit: http://git-wip-us.apache.org/repos/asf/carbondata/commit/fc2a5399
Tree: http://git-wip-us.apache.org/repos/asf/carbondata/tree/fc2a5399
Diff: http://git-wip-us.apache.org/repos/asf/carbondata/diff/fc2a5399

Branch: refs/heads/master
Commit: fc2a539913c6a4399e80f2fab7ecb93c0c534e13
Parents: f51f5cd
Author: manishgupta88 <to...@gmail.com>
Authored: Mon Oct 29 20:52:02 2018 +0530
Committer: kunal642 <ku...@gmail.com>
Committed: Fri Nov 2 16:06:38 2018 +0530

----------------------------------------------------------------------
 .../chunk/reader/CarbonDataReaderFactory.java   |  8 +--
 ...mpressedDimensionChunkFileBasedReaderV3.java |  1 +
 ...CompressedMeasureChunkFileBasedReaderV3.java |  1 +
 .../indexstore/BlockletDataMapIndexStore.java   |  2 +-
 .../core/metadata/ColumnarFormatVersion.java    |  2 +-
 .../core/metadata/encoder/Encoding.java         | 43 ++++++++++++-
 .../carbondata/core/util/CarbonProperties.java  | 11 ++--
 .../TestQueryWithOldCarbonDataFile.scala        | 66 --------------------
 .../TestNonTransactionalCarbonTable.scala       |  2 +-
 9 files changed, 56 insertions(+), 80 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/carbondata/blob/fc2a5399/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/CarbonDataReaderFactory.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/CarbonDataReaderFactory.java b/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/CarbonDataReaderFactory.java
index 07cd7b4..bcbe882 100644
--- a/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/CarbonDataReaderFactory.java
+++ b/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/CarbonDataReaderFactory.java
@@ -74,7 +74,6 @@ public class CarbonDataReaderFactory {
         return new CompressedDimensionChunkFileBasedReaderV2(blockletInfo, eachColumnValueSize,
             filePath);
       case V3:
-      default:
         if (readPagebyPage) {
           return new CompressedDimChunkFileBasedPageLevelReaderV3(blockletInfo, eachColumnValueSize,
               filePath);
@@ -82,6 +81,8 @@ public class CarbonDataReaderFactory {
           return new CompressedDimensionChunkFileBasedReaderV3(blockletInfo, eachColumnValueSize,
               filePath);
         }
+      default:
+        throw new UnsupportedOperationException("Unsupported columnar format version: " + version);
     }
   }
 
@@ -101,14 +102,13 @@ public class CarbonDataReaderFactory {
       case V2:
         return new CompressedMeasureChunkFileBasedReaderV2(blockletInfo, filePath);
       case V3:
-      default:
         if (readPagebyPage) {
           return new CompressedMsrChunkFileBasedPageLevelReaderV3(blockletInfo, filePath);
         } else {
           return new CompressedMeasureChunkFileBasedReaderV3(blockletInfo, filePath);
         }
-
+      default:
+        throw new UnsupportedOperationException("Unsupported columnar format version: " + version);
     }
-
   }
 }

http://git-wip-us.apache.org/repos/asf/carbondata/blob/fc2a5399/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/dimension/v3/CompressedDimensionChunkFileBasedReaderV3.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/dimension/v3/CompressedDimensionChunkFileBasedReaderV3.java b/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/dimension/v3/CompressedDimensionChunkFileBasedReaderV3.java
index 602e694..9df5bc1 100644
--- a/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/dimension/v3/CompressedDimensionChunkFileBasedReaderV3.java
+++ b/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/dimension/v3/CompressedDimensionChunkFileBasedReaderV3.java
@@ -264,6 +264,7 @@ public class CompressedDimensionChunkFileBasedReaderV3 extends AbstractChunkRead
       ByteBuffer pageData, DataChunk2 pageMetadata, int offset, ColumnVectorInfo vectorInfo)
       throws IOException, MemoryException {
     List<Encoding> encodings = pageMetadata.getEncoders();
+    org.apache.carbondata.core.metadata.encoder.Encoding.validateEncodingTypes(encodings);
     if (CarbonUtil.isEncodedWithMeta(encodings)) {
       int[] invertedIndexes = new int[0];
       int[] invertedIndexesReverse = new int[0];

http://git-wip-us.apache.org/repos/asf/carbondata/blob/fc2a5399/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/measure/v3/CompressedMeasureChunkFileBasedReaderV3.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/measure/v3/CompressedMeasureChunkFileBasedReaderV3.java b/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/measure/v3/CompressedMeasureChunkFileBasedReaderV3.java
index 8394029..a754cf2 100644
--- a/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/measure/v3/CompressedMeasureChunkFileBasedReaderV3.java
+++ b/core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/measure/v3/CompressedMeasureChunkFileBasedReaderV3.java
@@ -233,6 +233,7 @@ public class CompressedMeasureChunkFileBasedReaderV3 extends AbstractMeasureChun
   protected ColumnPage decodeMeasure(DataChunk2 pageMetadata, ByteBuffer pageData, int offset,
       ColumnVectorInfo vectorInfo, BitSet nullBitSet) throws MemoryException, IOException {
     List<Encoding> encodings = pageMetadata.getEncoders();
+    org.apache.carbondata.core.metadata.encoder.Encoding.validateEncodingTypes(encodings);
     List<ByteBuffer> encoderMetas = pageMetadata.getEncoder_meta();
     String compressorName =
         CarbonMetadataUtil.getCompressorNameFromChunkMeta(pageMetadata.getChunk_meta());

http://git-wip-us.apache.org/repos/asf/carbondata/blob/fc2a5399/core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java b/core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java
index 5f1bca4..c534f35 100644
--- a/core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java
+++ b/core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java
@@ -192,7 +192,7 @@ public class BlockletDataMapIndexStore
           dataMap.clear();
         }
       }
-      throw new IOException("Problem in loading segment blocks.", e);
+      throw new IOException("Problem in loading segment blocks: " + e.getMessage(), e);
     }
 
     return blockletDataMapIndexWrappers;

http://git-wip-us.apache.org/repos/asf/carbondata/blob/fc2a5399/core/src/main/java/org/apache/carbondata/core/metadata/ColumnarFormatVersion.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/metadata/ColumnarFormatVersion.java b/core/src/main/java/org/apache/carbondata/core/metadata/ColumnarFormatVersion.java
index 240f891..9ddefd6 100644
--- a/core/src/main/java/org/apache/carbondata/core/metadata/ColumnarFormatVersion.java
+++ b/core/src/main/java/org/apache/carbondata/core/metadata/ColumnarFormatVersion.java
@@ -49,7 +49,7 @@ public enum ColumnarFormatVersion {
       case 3:
         return V3;
       default:
-        return V3;
+        throw new UnsupportedOperationException("Unsupported columnar format version: " + version);
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/carbondata/blob/fc2a5399/core/src/main/java/org/apache/carbondata/core/metadata/encoder/Encoding.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/metadata/encoder/Encoding.java b/core/src/main/java/org/apache/carbondata/core/metadata/encoder/Encoding.java
index f3c21b1..995eaee 100644
--- a/core/src/main/java/org/apache/carbondata/core/metadata/encoder/Encoding.java
+++ b/core/src/main/java/org/apache/carbondata/core/metadata/encoder/Encoding.java
@@ -16,6 +16,8 @@
  */
 package org.apache.carbondata.core.metadata.encoder;
 
+import java.util.List;
+
 /**
  * Encoding type supported in carbon
  */
@@ -27,11 +29,14 @@ public enum Encoding {
   BIT_PACKED,
   DIRECT_DICTIONARY,
   IMPLICIT,
-
   DIRECT_COMPRESS,
   ADAPTIVE_INTEGRAL,
   ADAPTIVE_DELTA_INTEGRAL,
   RLE_INTEGRAL,
+  DIRECT_STRING,
+  ADAPTIVE_FLOATING,
+  BOOL_BYTE,
+  ADAPTIVE_DELTA_FLOATING,
   DIRECT_COMPRESS_VARCHAR;
 
   public static Encoding valueOf(int ordinal) {
@@ -57,10 +62,46 @@ public enum Encoding {
       return ADAPTIVE_DELTA_INTEGRAL;
     } else if (ordinal == RLE_INTEGRAL.ordinal()) {
       return RLE_INTEGRAL;
+    } else if (ordinal == DIRECT_STRING.ordinal()) {
+      return DIRECT_STRING;
+    } else if (ordinal == ADAPTIVE_FLOATING.ordinal()) {
+      return ADAPTIVE_FLOATING;
+    } else if (ordinal == BOOL_BYTE.ordinal()) {
+      return BOOL_BYTE;
+    } else if (ordinal == ADAPTIVE_DELTA_FLOATING.ordinal()) {
+      return ADAPTIVE_DELTA_FLOATING;
     } else if (ordinal == DIRECT_COMPRESS_VARCHAR.ordinal()) {
       return DIRECT_COMPRESS_VARCHAR;
     } else {
       throw new RuntimeException("create Encoding with invalid ordinal: " + ordinal);
     }
   }
+
+  /**
+   * Method to validate for supported encoding types that can be read using the current version
+   *
+   * @param encodings
+   */
+  public static void validateEncodingTypes(List<org.apache.carbondata.format.Encoding> encodings) {
+    if (null != encodings && !encodings.isEmpty()) {
+      for (org.apache.carbondata.format.Encoding encoder : encodings) {
+        // if case is handle unsupported encoding type. An encoding not supported for read will
+        // be added as null by thrift during deserialization
+        // if given encoding name is not supported exception will be thrown
+        if (null == encoder) {
+          throw new UnsupportedOperationException(
+              "There is mismatch between the encodings in data file and the encodings supported"
+                  + " for read in the current version");
+        } else {
+          try {
+            Encoding.valueOf(encoder.name());
+          } catch (IllegalArgumentException ex) {
+            throw new UnsupportedOperationException(
+                "There is mismatch between the encodings in data file and the encodings supported"
+                    + " for read in the current version. Encoding: " + encoder.name());
+          }
+        }
+      }
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/carbondata/blob/fc2a5399/core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java b/core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java
index d117b4d..191a33e 100644
--- a/core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java
+++ b/core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java
@@ -686,7 +686,8 @@ public final class CarbonProperties {
           CarbonCommonConstants.CARBON_DATA_FILE_DEFAULT_VERSION);
     } else {
       try {
-        ColumnarFormatVersion.valueOf(carbondataFileVersionString);
+        carbonProperties.setProperty(CARBON_DATA_FILE_VERSION,
+            ColumnarFormatVersion.valueOf(carbondataFileVersionString).name());
       } catch (IllegalArgumentException e) {
         // use default property if user specifies an invalid version property
         LOGGER.warn("Specified file version property is invalid: " + carbondataFileVersionString
@@ -696,9 +697,8 @@ public final class CarbonProperties {
             CarbonCommonConstants.CARBON_DATA_FILE_DEFAULT_VERSION);
       }
     }
-    LOGGER.info("Carbon Current data file version: " + carbonProperties
-        .setProperty(CARBON_DATA_FILE_VERSION,
-            CarbonCommonConstants.CARBON_DATA_FILE_DEFAULT_VERSION));
+    LOGGER.info(
+        "Considered file format is: " + carbonProperties.getProperty(CARBON_DATA_FILE_VERSION));
   }
 
   /**
@@ -847,8 +847,7 @@ public final class CarbonProperties {
       return getDefaultFormatVersion();
     } else {
       try {
-        short version = Short.parseShort(versionStr);
-        return ColumnarFormatVersion.valueOf(version);
+        return ColumnarFormatVersion.valueOf(versionStr);
       } catch (IllegalArgumentException e) {
         return getDefaultFormatVersion();
       }

http://git-wip-us.apache.org/repos/asf/carbondata/blob/fc2a5399/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/allqueries/TestQueryWithOldCarbonDataFile.scala
----------------------------------------------------------------------
diff --git a/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/allqueries/TestQueryWithOldCarbonDataFile.scala b/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/allqueries/TestQueryWithOldCarbonDataFile.scala
deleted file mode 100644
index 54b58fc..0000000
--- a/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/allqueries/TestQueryWithOldCarbonDataFile.scala
+++ /dev/null
@@ -1,66 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.carbondata.spark.testsuite.allqueries
-
-import org.scalatest.BeforeAndAfterAll
-import org.apache.carbondata.core.constants.CarbonCommonConstants
-import org.apache.carbondata.core.util.CarbonProperties
-import org.apache.spark.sql.test.util.QueryTest
-
-/*
- * Test Class for query without data load
- *
- */
-class TestQueryWithOldCarbonDataFile extends QueryTest with BeforeAndAfterAll {
-  override def beforeAll {
-	  CarbonProperties.getInstance.addProperty(CarbonCommonConstants.CARBON_DATA_FILE_VERSION, "V1");
-    sql("drop table if exists OldFormatTable")
-    sql("drop table if exists OldFormatTableHIVE")
-     sql("""
-           CREATE TABLE IF NOT EXISTS OldFormatTable
-           (country String,
-           name String, phonetype String, serialname String, salary Int)
-           STORED BY 'carbondata'
-           """)
-      sql("""
-           CREATE TABLE IF NOT EXISTS OldFormatTableHIVE
-           (country String,
-           name String, phonetype String, serialname String, salary Int)
-          row format delimited fields terminated by ','
-           """)      
-    sql(s"LOAD DATA local inpath '$resourcesPath/OLDFORMATTABLE.csv' INTO table OldFormatTable")
-   sql(s"""
-           LOAD DATA LOCAL INPATH '$resourcesPath/OLDFORMATTABLEHIVE.csv' into table OldFormatTableHIVE
-           """)
-
-  }
-
-  CarbonProperties.getInstance.addProperty(CarbonCommonConstants.CARBON_DATA_FILE_VERSION, "V2")
-  test("Test select * query") {
-    checkAnswer(
-      sql("select * from OldFormatTable"), sql("select * from OldFormatTableHIVE")
-    )
-  }
-
-  override def afterAll {
-     CarbonProperties.getInstance.addProperty(CarbonCommonConstants.CARBON_DATA_FILE_VERSION, "V1")
-    sql("drop table if exists OldFormatTable")
-    sql("drop table if exists OldFormatTableHIVE")
-  }
-
-}

http://git-wip-us.apache.org/repos/asf/carbondata/blob/fc2a5399/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestNonTransactionalCarbonTable.scala
----------------------------------------------------------------------
diff --git a/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestNonTransactionalCarbonTable.scala b/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestNonTransactionalCarbonTable.scala
index e254792..a3a3fc3 100644
--- a/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestNonTransactionalCarbonTable.scala
+++ b/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestNonTransactionalCarbonTable.scala
@@ -1076,7 +1076,7 @@ class TestNonTransactionalCarbonTable extends QueryTest with BeforeAndAfterAll {
         sql("select * from sdkOutputTable").show(false)
       }
     assert(exception.getMessage()
-      .contains("Problem in loading segment blocks."))
+      .contains("Problem in loading segment blocks"))
 
 
     sql("DROP TABLE sdkOutputTable")