[GitHub] [carbondata] QiangCai commented on a change in pull request #3502: [CARBONATA-3605] Remove global dictionary feature

[GitBox]

QiangCai commented on a change in pull request #3502: [CARBONATA-3605] Remove global dictionary feature
URL: https://github.com/apache/carbondata/pull/3502#discussion_r361788336
 
 

 ##########
 File path: integration/spark-common/src/main/scala/org/apache/spark/sql/catalyst/CarbonDDLSqlParser.scala
 ##########
 @@ -767,78 +766,19 @@ abstract class CarbonDDLSqlParser extends AbstractCarbonSparkSQLParser {
       }
     }
 
-    // All excluded cols should be there in create table cols
     if (tableProperties.get(CarbonCommonConstants.DICTIONARY_EXCLUDE).isDefined) {
-      LOGGER.warn("dictionary_exclude option was deprecated, " +
-                  "by default string column does not use global dictionary.")
-      dictExcludeCols =
-        tableProperties.get(CarbonCommonConstants.DICTIONARY_EXCLUDE).get.split(',').map(_.trim)
-      dictExcludeCols
-        .foreach { dictExcludeCol =>
-          if (!fields.exists(x => x.column.equalsIgnoreCase(dictExcludeCol))) {
-            val errorMsg = "DICTIONARY_EXCLUDE column: " + dictExcludeCol +
-                           " does not exist in table or unsupported for complex child column. " +
-                           "Please check the create table statement."
-            throw new MalformedCarbonCommandException(errorMsg)
-          } else {
-            val dataType = fields.find(x =>
-              x.column.equalsIgnoreCase(dictExcludeCol)).get.dataType.get
-            if (!isDataTypeSupportedForDictionary_Exclude(dataType)) {
-              val errorMsg = "DICTIONARY_EXCLUDE is unsupported for " + dataType.toLowerCase() +
-                             " data type column: " + dictExcludeCol
-              throw new MalformedCarbonCommandException(errorMsg)
-            } else if (varcharCols.exists(x => x.equalsIgnoreCase(dictExcludeCol))) {
-              throw new MalformedCarbonCommandException(
-                "DICTIONARY_EXCLUDE is unsupported for long string datatype column: " +
-                dictExcludeCol)
-            }
-          }
-        }
+      // dictionary_exclude is not supported since 2.0
+      throw new DeprecatedFeatureException("dictionary_exclude")
     }
-    // All included cols should be there in create table cols
     if (tableProperties.get(CarbonCommonConstants.DICTIONARY_INCLUDE).isDefined) {
-      dictIncludeCols =
-        tableProperties(CarbonCommonConstants.DICTIONARY_INCLUDE).split(",").map(_.trim)
-      dictIncludeCols.foreach { distIncludeCol =>
-        if (!fields.exists(x => x.column.equalsIgnoreCase(distIncludeCol.trim))) {
-          val errorMsg = "DICTIONARY_INCLUDE column: " + distIncludeCol.trim +
-                         " does not exist in table or unsupported for complex child column. " +
-                         "Please check the create table statement."
-          throw new MalformedCarbonCommandException(errorMsg)
-        }
-        val rangeField = fields.find(_.column.equalsIgnoreCase(distIncludeCol.trim))
-        if ("binary".equalsIgnoreCase(rangeField.get.dataType.get)) {
-          throw new MalformedCarbonCommandException(
-            "DICTIONARY_INCLUDE is unsupported for binary data type column: " +
-                    distIncludeCol.trim)
-        }
-        if (varcharCols.exists(x => x.equalsIgnoreCase(distIncludeCol.trim))) {
-          throw new MalformedCarbonCommandException(
-            "DICTIONARY_INCLUDE is unsupported for long string datatype column: " +
-            distIncludeCol.trim)
-        }
-      }
-    }
-
-    // include cols should not contain exclude cols
-    dictExcludeCols.foreach { dicExcludeCol =>
-      if (dictIncludeCols.exists(x => x.equalsIgnoreCase(dicExcludeCol))) {
-        val errorMsg = "DICTIONARY_EXCLUDE can not contain the same column: " + dicExcludeCol +
-                       " with DICTIONARY_INCLUDE. Please check the create table statement."
-        throw new MalformedCarbonCommandException(errorMsg)
-      }
+      // dictionary_include is not supported since 2.0
+      throw new DeprecatedFeatureException("dictionary_include")
     }
 
     // by default consider all String cols as dims and if any dictionary include isn't present then
     // add it to noDictionaryDims list. consider all dictionary excludes/include cols as dims
     fields.foreach { field =>
-      if (dictExcludeCols.exists(x => x.equalsIgnoreCase(field.column))) {
-        noDictionaryDims :+= field.column
-        dimFields += field
-      } else if (dictIncludeCols.exists(x => x.equalsIgnoreCase(field.column))) {
-        dimFields += field
-      } else if (field.dataType.get.toUpperCase.equals("TIMESTAMP") &&
 
 Review comment:
   need check this change, maybe exclude timestamp column, it will be non-dict column

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services