rxin's comments.

Author: yhuai

sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala

@@ -20,7 +20,7 @@ package org.apache.spark.sql.hive
 import java.io.IOException
 import java.util.{List => JList}
 
-import com.google.common.cache.{CacheLoader, CacheBuilder}
+import com.google.common.cache.{LoadingCache, CacheLoader, CacheBuilder}
 
 import org.apache.hadoop.util.ReflectionUtils
 import org.apache.hadoop.hive.metastore.TableType
@@ -54,46 +54,47 @@ private[hive] class HiveMetastoreCatalog(hive: HiveContext) extends Catalog with
 
   // TODO: Use this everywhere instead of tuples or databaseName, tableName,.
   /** A fully qualified identifier for a table (i.e., database.tableName) */
-  case class TableIdent(database: String, name: String) {
-    def toLowerCase = TableIdent(database.toLowerCase, name.toLowerCase)
+  case class QualifiedTableName(database: String, name: String) {
+    def toLowerCase = QualifiedTableName(database.toLowerCase, name.toLowerCase)
   }
 
   /** A cache of Spark SQL data source tables that have been accessed. */
-  protected[hive] val cachedDataSourceTables = CacheBuilder.newBuilder()
-    .maximumSize(1000)
-    .build(
-      new CacheLoader[TableIdent, LogicalPlan]() {
-        override def load(in: TableIdent): LogicalPlan = {
-          logDebug(s"Creating new cached data source for $in")
-          val table = client.getTable(in.database, in.name)
-          val schemaString = table.getProperty("spark.sql.sources.schema")
-          val userSpecifiedSchema =
-            if (schemaString == null) {
-              None
-            } else {
-              Some(DataType.fromJson(schemaString).asInstanceOf[StructType])
-            }
-          // It does not appear that the ql client for the metastore has a way to enumerate all the
-          // SerDe properties directly...
-          val options = table.getTTable.getSd.getSerdeInfo.getParameters.toMap
-
-          val resolvedRelation =
-            ResolvedDataSource(
-              hive,
-              userSpecifiedSchema,
-              table.getProperty("spark.sql.sources.provider"),
-              options)
-
-          LogicalRelation(resolvedRelation.relation)
-        }
-      })
+  protected[hive] val cachedDataSourceTables: LoadingCache[QualifiedTableName, LogicalPlan] = {
+    val cacheLoader = new CacheLoader[QualifiedTableName, LogicalPlan]() {
+      override def load(in: QualifiedTableName): LogicalPlan = {
+        logDebug(s"Creating new cached data source for $in")
+        val table = client.getTable(in.database, in.name)
+        val schemaString = table.getProperty("spark.sql.sources.schema")
+        val userSpecifiedSchema =
+          if (schemaString == null) {
+            None
+          } else {
+            Some(DataType.fromJson(schemaString).asInstanceOf[StructType])
+          }
+        // It does not appear that the ql client for the metastore has a way to enumerate all the
+        // SerDe properties directly...
+        val options = table.getTTable.getSd.getSerdeInfo.getParameters.toMap
+
+        val resolvedRelation =
+          ResolvedDataSource(
+            hive,
+            userSpecifiedSchema,
+            table.getProperty("spark.sql.sources.provider"),
+            options)
+
+        LogicalRelation(resolvedRelation.relation)
+      }
+    }
+
+    CacheBuilder.newBuilder().maximumSize(1000).build(cacheLoader)
+  }
 
   def refreshTable(databaseName: String, tableName: String): Unit = {
-    cachedDataSourceTables.refresh(TableIdent(databaseName, tableName).toLowerCase)
+    cachedDataSourceTables.refresh(QualifiedTableName(databaseName, tableName).toLowerCase)
   }
 
   def invalidateTable(databaseName: String, tableName: String): Unit = {
-    cachedDataSourceTables.invalidate(TableIdent(databaseName, tableName).toLowerCase)
+    cachedDataSourceTables.invalidate(QualifiedTableName(databaseName, tableName).toLowerCase)
   }
 
   val caseSensitive: Boolean = false
@@ -143,7 +144,7 @@ private[hive] class HiveMetastoreCatalog(hive: HiveContext) extends Catalog with
     val table = client.getTable(databaseName, tblName)
 
     if (table.getProperty("spark.sql.sources.provider") != null) {
-      cachedDataSourceTables(TableIdent(databaseName, tblName).toLowerCase)
+      cachedDataSourceTables(QualifiedTableName(databaseName, tblName).toLowerCase)
     } else if (table.isView) {
       // if the unresolved relation is from hive view
       // parse the text into logic node.

sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/commands.scala

@@ -53,7 +53,9 @@ case class DropTable(
   override def run(sqlContext: SQLContext) = {
     val hiveContext = sqlContext.asInstanceOf[HiveContext]
     val ifExistsClause = if (ifExists) "IF EXISTS " else ""
-    try hiveContext.tryUncacheQuery(hiveContext.table(tableName)) catch {
+    try {
+      hiveContext.tryUncacheQuery(hiveContext.table(tableName))
+    } catch {
       case _: org.apache.hadoop.hive.ql.metadata.InvalidTableException =>
     }
     hiveContext.invalidateTable(tableName)

sql/hive/src/test/scala/org/apache/spark/sql/hive/MetastoreDataSourcesSuite.scala

@@ -49,7 +49,6 @@ class MetastoreDataSourcesSuite extends QueryTest with BeforeAndAfterEach {
     checkAnswer(
       sql("SELECT * FROM jsonTable"),
       jsonFile("src/test/resources/data/files/sample.json").collect().toSeq)
-
   }
 
   test ("persistent JSON table with a user specified schema") {
@@ -71,7 +70,6 @@ class MetastoreDataSourcesSuite extends QueryTest with BeforeAndAfterEach {
     checkAnswer(
       sql("SELECT a, b, `c_!@(3)`, `<d>`.`d!`, `<d>`.`=` FROM jsonTable"),
       sql("SELECT a, b, `c_!@(3)`, `<d>`.`d!`, `<d>`.`=` FROM expectedJsonTable").collect().toSeq)
-
   }
 
   test ("persistent JSON table with a user specified schema with a subset of fields") {
@@ -99,7 +97,6 @@ class MetastoreDataSourcesSuite extends QueryTest with BeforeAndAfterEach {
     checkAnswer(
       sql("SELECT b, `<d>`.`=` FROM jsonTable"),
       sql("SELECT b, `<d>`.`=` FROM expectedJsonTable").collect().toSeq)
-
   }
 
   test("resolve shortened provider names") {