You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by hv...@apache.org on 2016/10/10 04:52:51 UTC

spark git commit: [SPARK-17832][SQL] TableIdentifier.quotedString creates un-parseable names when name contains a backtick

Repository: spark
Updated Branches:
  refs/heads/master 8a6bbe095 -> 26fbca480


[SPARK-17832][SQL] TableIdentifier.quotedString creates un-parseable names when name contains a backtick

## What changes were proposed in this pull request?

The `quotedString` method in `TableIdentifier` and `FunctionIdentifier` produce an illegal (un-parseable) name when the name contains a backtick. For example:
```
import org.apache.spark.sql.catalyst.parser.CatalystSqlParser._
import org.apache.spark.sql.catalyst.TableIdentifier
import org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute
val complexName = TableIdentifier("`weird`table`name", Some("`d`b`1"))
parseTableIdentifier(complexName.unquotedString) // Does not work
parseTableIdentifier(complexName.quotedString) // Does not work
parseExpression(complexName.unquotedString) // Does not work
parseExpression(complexName.quotedString) // Does not work
```
We should handle the backtick properly to make `quotedString` parseable.

## How was this patch tested?
Add new testcases in `TableIdentifierParserSuite` and `ExpressionParserSuite`.

Author: jiangxingbo <ji...@gmail.com>

Closes #15403 from jiangxb1987/backtick.


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

Branch: refs/heads/master
Commit: 26fbca480604ba258f97b9590cfd6dda1ecd31db
Parents: 8a6bbe0
Author: jiangxingbo <ji...@gmail.com>
Authored: Sun Oct 9 21:52:46 2016 -0700
Committer: Herman van Hovell <hv...@databricks.com>
Committed: Sun Oct 9 21:52:46 2016 -0700

----------------------------------------------------------------------
 .../org/apache/spark/sql/catalyst/identifiers.scala      | 11 +++++++++--
 .../sql/catalyst/parser/ExpressionParserSuite.scala      | 11 ++++++++++-
 .../sql/catalyst/parser/TableIdentifierParserSuite.scala | 10 ++++++++++
 3 files changed, 29 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/spark/blob/26fbca48/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/identifiers.scala
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/identifiers.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/identifiers.scala
index d7b48ce..834897b 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/identifiers.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/identifiers.scala
@@ -17,7 +17,6 @@
 
 package org.apache.spark.sql.catalyst
 
-
 /**
  * An identifier that optionally specifies a database.
  *
@@ -29,8 +28,16 @@ sealed trait IdentifierWithDatabase {
 
   def database: Option[String]
 
+  /*
+   * Escapes back-ticks within the identifier name with double-back-ticks.
+   */
+  private def quoteIdentifier(name: String): String = name.replace("`", "``")
+
   def quotedString: String = {
-    if (database.isDefined) s"`${database.get}`.`$identifier`" else s"`$identifier`"
+    val replacedId = quoteIdentifier(identifier)
+    val replacedDb = database.map(quoteIdentifier(_))
+
+    if (replacedDb.isDefined) s"`${replacedDb.get}`.`$replacedId`" else s"`$replacedId`"
   }
 
   def unquotedString: String = {

http://git-wip-us.apache.org/repos/asf/spark/blob/26fbca48/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala
index 0fb1138..17cfc81 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala
@@ -18,7 +18,7 @@ package org.apache.spark.sql.catalyst.parser
 
 import java.sql.{Date, Timestamp}
 
-import org.apache.spark.sql.catalyst.{FunctionIdentifier, TableIdentifier}
+import org.apache.spark.sql.catalyst.FunctionIdentifier
 import org.apache.spark.sql.catalyst.analysis.{UnresolvedAttribute, _}
 import org.apache.spark.sql.catalyst.expressions._
 import org.apache.spark.sql.catalyst.plans.PlanTest
@@ -535,4 +535,13 @@ class ExpressionParserSuite extends PlanTest {
     // ".123BD" should not be treated as token of type BIGDECIMAL_LITERAL
     assertEqual("a.123BD_column", UnresolvedAttribute("a.123BD_column"))
   }
+
+  test("SPARK-17832 function identifier contains backtick") {
+    val complexName = FunctionIdentifier("`ba`r", Some("`fo`o"))
+    assertEqual(complexName.quotedString, UnresolvedAttribute("`fo`o.`ba`r"))
+    intercept(complexName.unquotedString, "mismatched input")
+    // Function identifier contains countious backticks should be treated correctly.
+    val complexName2 = FunctionIdentifier("ba``r", Some("fo``o"))
+    assertEqual(complexName2.quotedString, UnresolvedAttribute("fo``o.ba``r"))
+  }
 }

http://git-wip-us.apache.org/repos/asf/spark/blob/26fbca48/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala
index 793be89..7d46011 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala
@@ -104,4 +104,14 @@ class TableIdentifierParserSuite extends SparkFunSuite {
     // ".123BD" should not be treated as token of type BIGDECIMAL_LITERAL
     assert(parseTableIdentifier("a.123BD_LIST") == TableIdentifier("123BD_LIST", Some("a")))
   }
+
+  test("SPARK-17832 table identifier - contains backtick") {
+    val complexName = TableIdentifier("`weird`table`name", Some("`d`b`1"))
+    assert(complexName === parseTableIdentifier("```d``b``1`.```weird``table``name`"))
+    assert(complexName === parseTableIdentifier(complexName.quotedString))
+    intercept[ParseException](parseTableIdentifier(complexName.unquotedString))
+    // Table identifier contains countious backticks should be treated correctly.
+    val complexName2 = TableIdentifier("x``y", Some("d``b"))
+    assert(complexName2 === parseTableIdentifier(complexName2.quotedString))
+  }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org