carbondata-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ravipes...@apache.org
Subject [1/2] incubator-carbondata git commit: Problem: Issue with database name case sensitivity
Date Thu, 12 Jan 2017 15:05:26 GMT
Repository: incubator-carbondata
Updated Branches:
  refs/heads/master e705aadd9 -> 9e204dde2


Problem: Issue with database name case sensitivity

Analysis: When database name is provided in any DDL/DML command, the database name is interpreted
and used in the same case as provided by the user. This leads to different behavior in windows
and unix systems as windows is case sensitive and linux systems are case insensitive.
Consider a case for create database. Lets say database name is "Carbon". While executing database
name is provided as Carbon but while deleting or using or creating table the case is changed
to "CARbOn". In these cases system will not behave correctly and if HDFS UI is checked the
database Carbon will still exist even after dropping database as the case for database name
was different in the 2 commands execution.

Fix: Always convert the database name to lower case for any DDL/DML operation while parsing
so that the behavior remains consistent.


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

Branch: refs/heads/master
Commit: 6a34ce7a6fe0441891240b25da65f119fbf3d18d
Parents: e705aad
Author: manishgupta88 <tomanishgupta18@gmail.com>
Authored: Wed Jan 11 21:55:36 2017 +0530
Committer: ravipesala <ravi.pesala@gmail.com>
Committed: Thu Jan 12 20:34:10 2017 +0530

----------------------------------------------------------------------
 ...tCreateTableWithDatabaseNameCaseChange.scala | 60 ++++++++++++++++++++
 .../spark/sql/catalyst/CarbonDDLSqlParser.scala | 25 +++++++-
 .../spark/sql/CarbonCatalystOperators.scala     |  7 +++
 .../org/apache/spark/sql/CarbonSqlParser.scala  | 26 ++++++---
 .../spark/sql/hive/CarbonStrategies.scala       |  2 +
 .../sql/parser/CarbonSpark2SqlParser.scala      | 17 ++++--
 .../spark/sql/parser/CarbonSparkSqlParser.scala | 15 ++++-
 7 files changed, 135 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-carbondata/blob/6a34ce7a/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateTableWithDatabaseNameCaseChange.scala
----------------------------------------------------------------------
diff --git a/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateTableWithDatabaseNameCaseChange.scala
b/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateTableWithDatabaseNameCaseChange.scala
new file mode 100644
index 0000000..65c0f66
--- /dev/null
+++ b/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateTableWithDatabaseNameCaseChange.scala
@@ -0,0 +1,60 @@
+/*
+ * 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.createTable
+
+import org.apache.spark.sql.common.util.QueryTest
+import org.scalatest.BeforeAndAfterAll
+
+/**
+ * test functionality related the case change for database name
+ */
+class TestCreateTableWithDatabaseNameCaseChange extends QueryTest with BeforeAndAfterAll
{
+
+  override def beforeAll {
+    sql("use default")
+    sql("drop database if exists dbCaseChange cascade")
+  }
+
+  test("test create table with database case name change") {
+    // this test case will test the creation of table for different case for database name.
+    // In hive dbName folder is always created with small case in HDFS. Carbon should behave
+    // the same way. If table creation fails during second time creation it means in HDFS
+    // separate folders are created for the matching case in commands executed.
+    sql("create database dbCaseChange")
+    sql("use DBCaseChanGe")
+    sql("create table carbonTable(a int, b string)stored by 'carbondata'")
+    sql("drop table carbonTable")
+    sql("use default")
+    sql("use dbcasechange")
+    try {
+      // table creation should be successful
+      sql("create table carbonTable(a int, b string)stored by 'carbondata'")
+      assert(true)
+    } catch {
+      case ex: Exception =>
+        assert(false)
+    }
+  }
+
+  override def afterAll {
+    sql("use default")
+    sql("drop database if exists dbCaseChange cascade")
+  }
+}

http://git-wip-us.apache.org/repos/asf/incubator-carbondata/blob/6a34ce7a/integration/spark-common/src/main/scala/org/apache/spark/sql/catalyst/CarbonDDLSqlParser.scala
----------------------------------------------------------------------
diff --git a/integration/spark-common/src/main/scala/org/apache/spark/sql/catalyst/CarbonDDLSqlParser.scala
b/integration/spark-common/src/main/scala/org/apache/spark/sql/catalyst/CarbonDDLSqlParser.scala
index b581a9f..1c1d5f1 100644
--- a/integration/spark-common/src/main/scala/org/apache/spark/sql/catalyst/CarbonDDLSqlParser.scala
+++ b/integration/spark-common/src/main/scala/org/apache/spark/sql/catalyst/CarbonDDLSqlParser.scala
@@ -644,12 +644,35 @@ abstract class CarbonDDLSqlParser extends AbstractCarbonSparkSQLParser
{
         case Token(part, Nil) => cleanIdentifier(part)
       } match {
         case Seq(tableOnly) => (None, tableOnly)
-        case Seq(databaseName, table) => (Some(databaseName), table)
+        case Seq(databaseName, table) => (Some(convertDbNameToLowerCase(databaseName)),
table)
       }
 
     (db, tableName)
   }
 
+  /**
+   * This method will convert the database name to lower case
+   *
+   * @param dbName
+   * @return String
+   */
+  protected def convertDbNameToLowerCase(dbName: String) = {
+    dbName.toLowerCase
+  }
+
+  /**
+   * This method will convert the database name to lower case
+   *
+   * @param dbName
+   * @return Option of String
+   */
+  protected def convertDbNameToLowerCase(dbName: Option[String]): Option[String] = {
+    dbName match {
+      case Some(databaseName) => Some(convertDbNameToLowerCase(databaseName))
+      case None => dbName
+    }
+  }
+
   protected def cleanIdentifier(ident: String): String = {
     ident match {
       case escapedIdentifier(i) => i

http://git-wip-us.apache.org/repos/asf/incubator-carbondata/blob/6a34ce7a/integration/spark/src/main/scala/org/apache/spark/sql/CarbonCatalystOperators.scala
----------------------------------------------------------------------
diff --git a/integration/spark/src/main/scala/org/apache/spark/sql/CarbonCatalystOperators.scala
b/integration/spark/src/main/scala/org/apache/spark/sql/CarbonCatalystOperators.scala
index c21a8f5..c1a0dc2 100644
--- a/integration/spark/src/main/scala/org/apache/spark/sql/CarbonCatalystOperators.scala
+++ b/integration/spark/src/main/scala/org/apache/spark/sql/CarbonCatalystOperators.scala
@@ -107,6 +107,13 @@ case class DropDatabase(dbName: String, isCascade: Boolean, sql: String)
   }
 }
 
+case class UseDatabase(sql: String) extends LogicalPlan with Command {
+  override def children: Seq[LogicalPlan] = Seq.empty
+  override def output: Seq[AttributeReference] = {
+    Seq()
+  }
+}
+
 case class ProjectForUpdate(
     table: UnresolvedRelation,
     columns: List[String],

http://git-wip-us.apache.org/repos/asf/incubator-carbondata/blob/6a34ce7a/integration/spark/src/main/scala/org/apache/spark/sql/CarbonSqlParser.scala
----------------------------------------------------------------------
diff --git a/integration/spark/src/main/scala/org/apache/spark/sql/CarbonSqlParser.scala b/integration/spark/src/main/scala/org/apache/spark/sql/CarbonSqlParser.scala
index b4f185f..e9738cb 100644
--- a/integration/spark/src/main/scala/org/apache/spark/sql/CarbonSqlParser.scala
+++ b/integration/spark/src/main/scala/org/apache/spark/sql/CarbonSqlParser.scala
@@ -60,7 +60,7 @@ class CarbonSqlParser() extends CarbonDDLSqlParser {
 
   protected lazy val startCommand: Parser[LogicalPlan] =
     createDatabase | dropDatabase | loadManagement | describeTable |
-    showLoads | alterTable | updateTable | deleteRecords| createTable
+    showLoads | alterTable | updateTable | deleteRecords | useDatabase | createTable
 
   protected lazy val loadManagement: Parser[LogicalPlan] =
     deleteLoadsByID | deleteLoadsByLoadDate | cleanFiles | loadDataNew
@@ -77,7 +77,7 @@ class CarbonSqlParser() extends CarbonDDLSqlParser {
           case Token("TOK_CREATEDATABASE", children) =>
             dbName = BaseSemanticAnalyzer.unescapeIdentifier(children(0).getText)
         }
-        CreateDatabase(dbName, createDbSql)
+        CreateDatabase(convertDbNameToLowerCase(dbName), createDbSql)
     }
 
   protected lazy val dropDatabase: Parser[LogicalPlan] =
@@ -98,7 +98,7 @@ class CarbonSqlParser() extends CarbonDDLSqlParser {
               case _ => // Unsupport features
             }
         }
-        DropDatabase(dbName, isCascade, dropDbSql)
+        DropDatabase(convertDbNameToLowerCase(dbName), isCascade, dropDbSql)
     }
 
   protected lazy val alterTable: Parser[LogicalPlan] =
@@ -348,7 +348,7 @@ class CarbonSqlParser() extends CarbonDDLSqlParser {
           validateOptions(optionsList)
         }
         val optionsMap = optionsList.getOrElse(List.empty[(String, String)]).toMap
-        LoadTable(databaseNameOp, tableName, filePath, Seq(), optionsMap,
+        LoadTable(convertDbNameToLowerCase(databaseNameOp), tableName, filePath, Seq(), optionsMap,
           isOverwrite.isDefined)
     }
 
@@ -357,7 +357,7 @@ class CarbonSqlParser() extends CarbonDDLSqlParser {
       case ef ~ db ~ tbl =>
         val tblIdentifier = db match {
           case Some(dbName) =>
-            TableIdentifier(tbl.toLowerCase, Some(dbName))
+            TableIdentifier(tbl.toLowerCase, Some(convertDbNameToLowerCase(dbName)))
           case None =>
             TableIdentifier(tbl.toLowerCase, None)
         }
@@ -374,7 +374,7 @@ class CarbonSqlParser() extends CarbonDDLSqlParser {
     (LIMIT ~> numericLit).? <~
     opt(";") ^^ {
       case databaseName ~ tableName ~ limit =>
-        ShowLoadsCommand(databaseName, tableName.toLowerCase(), limit)
+        ShowLoadsCommand(convertDbNameToLowerCase(databaseName), tableName.toLowerCase(),
limit)
     }
 
   protected lazy val deleteLoadsByID: Parser[LogicalPlan] =
@@ -383,7 +383,7 @@ class CarbonSqlParser() extends CarbonDDLSqlParser {
     opt(";") ^^ {
       case loadids ~ table => table match {
         case databaseName ~ tableName =>
-          DeleteLoadsById(loadids, databaseName, tableName.toLowerCase())
+          DeleteLoadsById(loadids, convertDbNameToLowerCase(databaseName), tableName.toLowerCase())
       }
     }
 
@@ -394,13 +394,17 @@ class CarbonSqlParser() extends CarbonDDLSqlParser {
       case schema ~ table ~ condition =>
         condition match {
           case dateField ~ dateValue =>
-            DeleteLoadsByLoadDate(schema, table.toLowerCase(), dateField, dateValue)
+            DeleteLoadsByLoadDate(convertDbNameToLowerCase(schema),
+              table.toLowerCase(),
+              dateField,
+              dateValue)
         }
     }
 
   protected lazy val cleanFiles: Parser[LogicalPlan] =
     CLEAN ~> FILES ~> FOR ~> TABLE ~> (ident <~ ".").? ~ ident <~ opt(";")
^^ {
-      case databaseName ~ tableName => CleanFiles(databaseName, tableName.toLowerCase())
+      case databaseName ~ tableName =>
+        CleanFiles(convertDbNameToLowerCase(databaseName), tableName.toLowerCase())
     }
 
   protected lazy val explainPlan: Parser[LogicalPlan] =
@@ -541,4 +545,8 @@ class CarbonSqlParser() extends CarbonDDLSqlParser {
       case table ~ column => column.toLowerCase
     }
 
+  protected lazy val useDatabase: Parser[LogicalPlan] =
+    USE ~> ident <~ opt(";") ^^ {
+      case databaseName => UseDatabase(s"use ${ databaseName.toLowerCase }")
+    }
 }

http://git-wip-us.apache.org/repos/asf/incubator-carbondata/blob/6a34ce7a/integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonStrategies.scala
----------------------------------------------------------------------
diff --git a/integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonStrategies.scala
b/integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonStrategies.scala
index 38109ae..f0cd33b 100644
--- a/integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonStrategies.scala
+++ b/integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonStrategies.scala
@@ -291,6 +291,8 @@ class CarbonStrategies(sqlContext: SQLContext) extends QueryPlanner[SparkPlan]
{
         } else {
           ExecutedCommand(DropDatabaseCommand(dbName, HiveNativeCommand(sql))) :: Nil
         }
+      case UseDatabase(sql) =>
+        ExecutedCommand(HiveNativeCommand(sql)) :: Nil
       case d: HiveNativeCommand =>
         try {
           val resolvedTable = sqlContext.executePlan(CarbonHiveSyntax.parse(d.sql)).optimizedPlan

http://git-wip-us.apache.org/repos/asf/incubator-carbondata/blob/6a34ce7a/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSpark2SqlParser.scala
----------------------------------------------------------------------
diff --git a/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSpark2SqlParser.scala
b/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSpark2SqlParser.scala
index 0c343e1..6ad404a 100644
--- a/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSpark2SqlParser.scala
+++ b/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSpark2SqlParser.scala
@@ -60,7 +60,8 @@ class CarbonSpark2SqlParser extends CarbonDDLSqlParser {
   protected lazy val alterTable: Parser[LogicalPlan] =
     ALTER ~> TABLE ~> (ident <~ ".").? ~ ident ~ (COMPACT ~ stringLit) <~ opt(";")
 ^^ {
       case dbName ~ table ~ (compact ~ compactType) =>
-        val altertablemodel = AlterTableModel(dbName, table, None, compactType,
+        val altertablemodel =
+          AlterTableModel(convertDbNameToLowerCase(dbName), table, None, compactType,
           Some(System.currentTimeMillis()), null)
         AlterTableCompaction(altertablemodel)
     }
@@ -78,7 +79,7 @@ class CarbonSpark2SqlParser extends CarbonDDLSqlParser {
           validateOptions(optionsList)
         }
         val optionsMap = optionsList.getOrElse(List.empty[(String, String)]).toMap
-        LoadTable(databaseNameOp, tableName, filePath, Seq(), optionsMap,
+        LoadTable(convertDbNameToLowerCase(databaseNameOp), tableName, filePath, Seq(), optionsMap,
           isOverwrite.isDefined)
     }
 
@@ -88,7 +89,7 @@ class CarbonSpark2SqlParser extends CarbonDDLSqlParser {
     opt(";") ^^ {
       case loadids ~ table => table match {
         case databaseName ~ tableName =>
-          DeleteLoadsById(loadids, databaseName, tableName.toLowerCase())
+          DeleteLoadsById(loadids, convertDbNameToLowerCase(databaseName), tableName.toLowerCase())
       }
     }
 
@@ -99,13 +100,17 @@ class CarbonSpark2SqlParser extends CarbonDDLSqlParser {
       case schema ~ table ~ condition =>
         condition match {
           case dateField ~ dateValue =>
-            DeleteLoadsByLoadDate(schema, table.toLowerCase(), dateField, dateValue)
+            DeleteLoadsByLoadDate(convertDbNameToLowerCase(schema),
+              table.toLowerCase(),
+              dateField,
+              dateValue)
         }
     }
 
   protected lazy val cleanFiles: Parser[LogicalPlan] =
     CLEAN ~> FILES ~> FOR ~> TABLE ~> (ident <~ ".").? ~ ident <~ opt(";")
^^ {
-      case databaseName ~ tableName => CleanFiles(databaseName, tableName.toLowerCase())
+      case databaseName ~ tableName =>
+        CleanFiles(convertDbNameToLowerCase(databaseName), tableName.toLowerCase())
     }
 
   protected lazy val explainPlan: Parser[LogicalPlan] =
@@ -122,6 +127,6 @@ class CarbonSpark2SqlParser extends CarbonDDLSqlParser {
     (LIMIT ~> numericLit).? <~
     opt(";") ^^ {
       case databaseName ~ tableName ~ limit =>
-        ShowLoadsCommand(databaseName, tableName.toLowerCase(), limit)
+        ShowLoadsCommand(convertDbNameToLowerCase(databaseName), tableName.toLowerCase(),
limit)
     }
 }

http://git-wip-us.apache.org/repos/asf/incubator-carbondata/blob/6a34ce7a/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala
----------------------------------------------------------------------
diff --git a/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala
b/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala
index 6f07ec5..3dcf2d9 100644
--- a/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala
+++ b/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala
@@ -148,7 +148,7 @@ class CarbonSqlAstBuilder(conf: SQLConf) extends SparkSqlAstBuilder(conf)
{
       properties.foreach(tableProperties += _)
       // prepare table model of the collected tokens
       val tableModel: TableModel = parser.prepareTableModel(ifNotExists,
-        name.database,
+        convertDbNameToLowerCase(name.database),
         name.table.toLowerCase,
         fields,
         Seq(),
@@ -162,6 +162,19 @@ class CarbonSqlAstBuilder(conf: SQLConf) extends SparkSqlAstBuilder(conf)
{
   }
 
   /**
+   * This method will convert the database name to lower case
+   *
+   * @param dbName
+   * @return Option of String
+   */
+  protected def convertDbNameToLowerCase(dbName: Option[String]): Option[String] = {
+    dbName match {
+      case Some(databaseName) => Some(databaseName.toLowerCase)
+      case None => dbName
+    }
+  }
+
+  /**
    * Parse a key-value map from a [[TablePropertyListContext]], assuming all values are specified.
    */
   private def visitPropertyKeyValues(ctx: TablePropertyListContext): Map[String, String]
= {


Mime
View raw message