[SPARK-10104][SQL] Consolidate different forms of table identifiers #8453

cloud-fan · 2015-08-26T05:56:49Z

Right now, we have QualifiedTableName, TableIdentifier, and Seq[String] to represent table identifiers. We should only have one form and TableIdentifier is the best one because it provides methods to get table name, database name, return unquoted string, and return quoted string.

SparkQA · 2015-08-26T06:05:20Z

Test build #41604 has finished for PR 8453 at commit 17b7406.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class LogisticRegressionModel @Since("1.3.0") (
- class SVMModel @Since("1.1.0") (
- class BinaryClassificationMetrics @Since("1.3.0") (
- class MulticlassMetrics @Since("1.1.0") (predictionAndLabels: RDD[(Double, Double)])
- class MultilabelMetrics @Since("1.2.0") (predictionAndLabels: RDD[(Array[Double], Array[Double])])
- class RegressionMetrics @Since("1.2.0") (
- class FPGrowthModel[Item: ClassTag] @Since("1.3.0") (
- class FreqItemset[Item] @Since("1.3.0") (
- class FreqSequence[Item] @Since("1.5.0") (
- class PrefixSpanModel[Item] @Since("1.5.0") (
- class DenseMatrix @Since("1.3.0") (
- class SparseMatrix @Since("1.3.0") (
- class DenseVector @Since("1.0.0") (
- class SparseVector @Since("1.0.0") (
- class BlockMatrix @Since("1.3.0") (
- class CoordinateMatrix @Since("1.0.0") (
- class IndexedRowMatrix @Since("1.0.0") (
- class RowMatrix @Since("1.0.0") (
- abstract class SetOperation(left: LogicalPlan, right: LogicalPlan) extends BinaryNode
- case class Union(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)
- case class Intersect(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)
- case class Except(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)
- case class QualifiedTableName(database: String, name: String)

SparkQA · 2015-08-26T06:14:58Z

Test build #41605 has finished for PR 8453 at commit e8d88e9.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class QualifiedTableName(database: String, name: String)

SparkQA · 2015-08-26T07:40:46Z

Test build #41607 has finished for PR 8453 at commit 9f8b314.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class QualifiedTableName(database: String, name: String)

SparkQA · 2015-08-26T07:48:46Z

Test build #41608 has finished for PR 8453 at commit de2736f.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class QualifiedTableName(database: String, name: String)

SparkQA · 2015-08-26T08:08:41Z

Test build #41610 has finished for PR 8453 at commit 63f2f07.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class QualifiedTableName(database: String, name: String)

SparkQA · 2015-08-26T08:23:44Z

Test build #41612 has finished for PR 8453 at commit 9051701.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class QualifiedTableName(database: String, name: String)

SparkQA · 2015-08-26T12:06:51Z

Test build #41617 has finished for PR 8453 at commit 1692b53.

This patch fails SparkR unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class QualifiedTableName(database: String, name: String)

SparkQA · 2015-08-26T15:45:06Z

Test build #41622 has finished for PR 8453 at commit 49b8d3d.

This patch fails SparkR unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class QualifiedTableName(database: String, name: String)

cloud-fan · 2015-08-26T16:11:49Z

cc @yhuai

SparkQA · 2015-08-26T18:33:00Z

Test build #41631 has finished for PR 8453 at commit fbd03df.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class QualifiedTableName(database: String, name: String)

yhuai · 2015-08-27T04:14:11Z

@cloud-fan Thank you for working on it. Since we are pretty close to 1.5 release, let's wait until the release and get this merged at the beginning of 1.6 release cycle.

SparkQA · 2015-10-03T07:30:47Z

Test build #43207 has finished for PR 8453 at commit 3be0e4b.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class SimpleCatalog(val conf: CatalystConf) extends Catalog
- trait ExtendedCatalog extends SimpleCatalog

cloud-fan · 2015-10-03T08:24:46Z

retest this please.

SparkQA · 2015-10-03T10:09:52Z

Test build #43208 has finished for PR 8453 at commit 9bc2db1.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class SimpleCatalog(val conf: CatalystConf) extends Catalog
- trait ExtendedCatalog extends SimpleCatalog

SparkQA · 2015-10-03T10:18:02Z

Test build #43209 has finished for PR 8453 at commit 9bc2db1.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class SimpleCatalog(val conf: CatalystConf) extends Catalog
- trait ExtendedCatalog extends SimpleCatalog

SparkQA · 2015-10-03T22:44:45Z

Test build #43219 has finished for PR 8453 at commit 71a7d54.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- final class ChiSqSelector(override val uid: String)
- final class QuantileDiscretizer(override val uid: String)
- class SimpleCatalog(val conf: CatalystConf) extends Catalog
- trait ExtendedCatalog extends SimpleCatalog

davies · 2015-10-12T17:39:08Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/TableIdentifier.scala


-  def toSeq: Seq[String] = database.toSeq :+ table
+  def quotedString: String = database.map(db => s"`$db`.`$table`").getOrElse(s"`$table`")


$db may already have . in it

This is the same behaviour of the original implementation def quotedString: String = toSeq.map("" + _ + "``").mkString(".")`.

cloud-fan · 2015-10-12T22:24:36Z

retest this please.

SparkQA · 2015-10-12T22:39:17Z

Test build #43594 has finished for PR 8453 at commit 71a7d54.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class SimpleCatalog(val conf: CatalystConf) extends Catalog
- trait ExtendedCatalog extends SimpleCatalog

SparkQA · 2015-10-13T01:03:36Z

Test build #43601 has finished for PR 8453 at commit 71a7d54.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class SimpleCatalog(val conf: CatalystConf) extends Catalog
- trait ExtendedCatalog extends SimpleCatalog

SparkQA · 2015-10-13T01:45:55Z

Test build #43605 has finished for PR 8453 at commit 1d89c37.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class SimpleCatalog(val conf: CatalystConf) extends Catalog
- trait ExtendedCatalog extends SimpleCatalog

SparkQA · 2015-10-13T04:17:08Z

Test build #43613 has finished for PR 8453 at commit 26ee534.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class SimpleCatalog(val conf: CatalystConf) extends Catalog
- trait ExtendedCatalog extends SimpleCatalog
- case class QualifiedTableName(database: String, name: String)

SparkQA · 2015-10-13T08:51:01Z

Test build #43627 has finished for PR 8453 at commit 2714fc0.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class SimpleCatalog(val conf: CatalystConf) extends Catalog
- trait ExtendedCatalog extends SimpleCatalog
- case class QualifiedTableName(database: String, name: String)

davies · 2015-10-13T17:49:53Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/TableIdentifier.scala

-  def withDatabase(database: String): TableIdentifier = this.copy(database = Some(database))
+private[sql] case class TableIdentifier(table: String, database: Option[String]) {
+  override def toString: String = {
+    if (table.contains('.') || database.exists(_.contains('.'))) {


. is not the only character that need to be quoted, see http://stackoverflow.com/questions/4200351/what-characters-are-valid-in-an-sql-server-database-name

SparkQA · 2015-10-13T18:55:34Z

Test build #43659 has finished for PR 8453 at commit 8dde0d9.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class QualifiedTableName(database: String, name: String)

SparkQA · 2015-10-13T21:08:34Z

Test build #43661 has finished for PR 8453 at commit b6a1f97.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class QualifiedTableName(database: String, name: String)

davies · 2015-10-13T21:24:27Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/TableIdentifier.scala


-  def toSeq: Seq[String] = database.toSeq :+ table
+  override def toString: String = {
+    if (table.contains('.') || database.exists(_.contains('.'))) {


There are some other character that need quote.

SparkQA · 2015-10-14T00:23:01Z

Test build #43681 has finished for PR 8453 at commit b60ee53.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class QualifiedTableName(database: String, name: String)

SparkQA · 2015-10-14T19:32:58Z

Test build #43725 has finished for PR 8453 at commit 6ad40d1.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- def getPath = path.getOrElse(sys.error("Constructors must start at a class type"))
- case class ExprId(id: Long, jvmId: UUID)
- case class WrapOption(optionType: DataType, child: Expression)
- class GenericArrayData(val array: Array[Any]) extends ArrayData
- trait QueryExecutionListener
- class ExecutionListenerManager extends Logging
- case class QualifiedTableName(database: String, name: String)

SparkQA · 2015-10-14T22:06:31Z

Test build #43733 has finished for PR 8453 at commit 5cfb8cc.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- def getPath = path.getOrElse(sys.error("Constructors must start at a class type"))
- case class WrapOption(optionType: DataType, child: Expression)
- class GenericArrayData(val array: Array[Any]) extends ArrayData
- trait QueryExecutionListener
- class ExecutionListenerManager extends Logging
- case class QualifiedTableName(database: String, name: String)

SparkQA · 2015-10-14T22:32:49Z

Test build #43735 has finished for PR 8453 at commit 230970c.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- def getPath = path.getOrElse(sys.error("Constructors must start at a class type"))
- case class WrapOption(optionType: DataType, child: Expression)
- class GenericArrayData(val array: Array[Any]) extends ArrayData
- trait QueryExecutionListener
- class ExecutionListenerManager extends Logging
- case class QualifiedTableName(database: String, name: String)

davies · 2015-10-14T23:05:20Z

LGTM, merging this into master.

cloud-fan force-pushed the table-name branch from 17b7406 to e8d88e9 Compare August 26, 2015 06:00

cloud-fan force-pushed the table-name branch 2 times, most recently from de2736f to 63f2f07 Compare August 26, 2015 07:37

cloud-fan force-pushed the table-name branch from 9051701 to 1692b53 Compare August 26, 2015 09:37

cloud-fan force-pushed the table-name branch from 49b8d3d to fbd03df Compare August 26, 2015 16:10

cloud-fan force-pushed the table-name branch from fbd03df to 3be0e4b Compare October 3, 2015 07:19

cloud-fan force-pushed the table-name branch from 3be0e4b to 9bc2db1 Compare October 3, 2015 08:21

cloud-fan force-pushed the table-name branch 2 times, most recently from b73a696 to 71a7d54 Compare October 3, 2015 20:39

davies reviewed Oct 12, 2015
View reviewed changes

cloud-fan force-pushed the table-name branch from 71a7d54 to 1d89c37 Compare October 13, 2015 01:30

cloud-fan force-pushed the table-name branch from 1d89c37 to 26ee534 Compare October 13, 2015 04:02

cloud-fan force-pushed the table-name branch from 26ee534 to 2714fc0 Compare October 13, 2015 06:54

davies reviewed Oct 13, 2015
View reviewed changes

cloud-fan force-pushed the table-name branch from 2714fc0 to 8dde0d9 Compare October 13, 2015 18:51

Consolidate different forms of table identifiers

b6a1f97

cloud-fan force-pushed the table-name branch from 8dde0d9 to b6a1f97 Compare October 13, 2015 19:09

davies reviewed Oct 13, 2015
View reviewed changes

use quoted string for TableIdentifier.toString

b60ee53

Merge remote-tracking branch 'origin/master' into table-name

5cfb8cc

cloud-fan force-pushed the table-name branch from 6ad40d1 to 5cfb8cc Compare October 14, 2015 20:16

cloud-fan added 2 commits October 14, 2015 13:26

style fix

86cd7f9

fix a stupid mistake...

230970c

asfgit closed this in 56d7da1 Oct 14, 2015

cloud-fan deleted the table-name branch October 14, 2015 23:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-10104][SQL] Consolidate different forms of table identifiers #8453

[SPARK-10104][SQL] Consolidate different forms of table identifiers #8453

cloud-fan commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

cloud-fan commented Aug 26, 2015

SparkQA commented Aug 26, 2015

yhuai commented Aug 27, 2015

SparkQA commented Oct 3, 2015

cloud-fan commented Oct 3, 2015

SparkQA commented Oct 3, 2015

SparkQA commented Oct 3, 2015

SparkQA commented Oct 3, 2015

davies Oct 12, 2015

cloud-fan Oct 12, 2015

cloud-fan commented Oct 12, 2015

SparkQA commented Oct 12, 2015

SparkQA commented Oct 13, 2015

SparkQA commented Oct 13, 2015

SparkQA commented Oct 13, 2015

SparkQA commented Oct 13, 2015

davies Oct 13, 2015

SparkQA commented Oct 13, 2015

SparkQA commented Oct 13, 2015

davies Oct 13, 2015

SparkQA commented Oct 14, 2015

SparkQA commented Oct 14, 2015

SparkQA commented Oct 14, 2015

SparkQA commented Oct 14, 2015

davies commented Oct 14, 2015


		def toSeq: Seq[String] = database.toSeq :+ table
		def quotedString: String = database.map(db => s"`$db`.`$table`").getOrElse(s"`$table`")

[SPARK-10104][SQL] Consolidate different forms of table identifiers #8453

[SPARK-10104][SQL] Consolidate different forms of table identifiers #8453

Conversation

cloud-fan commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

SparkQA commented Aug 26, 2015

cloud-fan commented Aug 26, 2015

SparkQA commented Aug 26, 2015

yhuai commented Aug 27, 2015

SparkQA commented Oct 3, 2015

cloud-fan commented Oct 3, 2015

SparkQA commented Oct 3, 2015

SparkQA commented Oct 3, 2015

SparkQA commented Oct 3, 2015

davies Oct 12, 2015

Choose a reason for hiding this comment

cloud-fan Oct 12, 2015

Choose a reason for hiding this comment

cloud-fan commented Oct 12, 2015

SparkQA commented Oct 12, 2015

SparkQA commented Oct 13, 2015

SparkQA commented Oct 13, 2015

SparkQA commented Oct 13, 2015

SparkQA commented Oct 13, 2015

davies Oct 13, 2015

Choose a reason for hiding this comment

SparkQA commented Oct 13, 2015

SparkQA commented Oct 13, 2015

davies Oct 13, 2015

Choose a reason for hiding this comment

SparkQA commented Oct 14, 2015

SparkQA commented Oct 14, 2015

SparkQA commented Oct 14, 2015

SparkQA commented Oct 14, 2015

davies commented Oct 14, 2015