Repository: spark Updated Branches: refs/heads/branch-2.0 1dad1a891 -> f63ba2210
[SPARK-15436][SQL] Remove DescribeFunction and ShowFunctions ## What changes were proposed in this pull request? This patch removes the last two commands defined in the catalyst module: DescribeFunction and ShowFunctions. They were unnecessary since the parser could just generate DescribeFunctionCommand and ShowFunctionsCommand directly. ## How was this patch tested? Created a new SparkSqlParserSuite. Author: Reynold Xin <r...@databricks.com> Closes #13292 from rxin/SPARK-15436. (cherry picked from commit 4f27b8dd58a66fca7ddd4c239e02b90c34b1cebd) Signed-off-by: Herman van Hovell <hvanhov...@questtec.nl> Project: http://git-wip-us.apache.org/repos/asf/spark/repo Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/f63ba221 Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/f63ba221 Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/f63ba221 Branch: refs/heads/branch-2.0 Commit: f63ba2210a27df1843f5007d367036dae0e0139f Parents: 1dad1a8 Author: Reynold Xin <r...@databricks.com> Authored: Wed May 25 19:17:53 2016 +0200 Committer: Herman van Hovell <hvanhov...@questtec.nl> Committed: Wed May 25 19:18:32 2016 +0200 ---------------------------------------------------------------------- .../spark/sql/catalyst/parser/AstBuilder.scala | 32 +-------- .../sql/catalyst/plans/logical/Command.scala | 25 +++++++ .../sql/catalyst/plans/logical/commands.scala | 55 ---------------- .../analysis/UnsupportedOperationsSuite.scala | 9 ++- .../sql/catalyst/parser/PlanParserSuite.scala | 30 +++------ .../spark/sql/execution/SparkSqlParser.scala | 33 +++++++++- .../spark/sql/execution/SparkStrategies.scala | 6 -- .../sql/execution/SparkSqlParserSuite.scala | 68 ++++++++++++++++++++ .../sql/execution/command/DDLCommandSuite.scala | 2 +- .../sql/hive/execution/HiveComparisonTest.scala | 6 +- 10 files changed, 145 insertions(+), 121 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/spark/blob/f63ba221/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala ---------------------------------------------------------------------- diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala index a13c03a..3473fee 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala @@ -14,6 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.apache.spark.sql.catalyst.parser import java.sql.{Date, Timestamp} @@ -82,37 +83,6 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with Logging { protected def plan(tree: ParserRuleContext): LogicalPlan = typedVisit(tree) /** - * Create a plan for a SHOW FUNCTIONS command. - */ - override def visitShowFunctions(ctx: ShowFunctionsContext): LogicalPlan = withOrigin(ctx) { - import ctx._ - if (qualifiedName != null) { - val name = visitFunctionName(qualifiedName) - ShowFunctions(name.database, Some(name.funcName)) - } else if (pattern != null) { - ShowFunctions(None, Some(string(pattern))) - } else { - ShowFunctions(None, None) - } - } - - /** - * Create a plan for a DESCRIBE FUNCTION command. - */ - override def visitDescribeFunction(ctx: DescribeFunctionContext): LogicalPlan = withOrigin(ctx) { - import ctx._ - val functionName = - if (describeFuncName.STRING() != null) { - FunctionIdentifier(string(describeFuncName.STRING()), database = None) - } else if (describeFuncName.qualifiedName() != null) { - visitFunctionName(describeFuncName.qualifiedName) - } else { - FunctionIdentifier(describeFuncName.getText, database = None) - } - DescribeFunction(functionName, EXTENDED != null) - } - - /** * Create a top-level plan with Common Table Expressions. */ override def visitQuery(ctx: QueryContext): LogicalPlan = withOrigin(ctx) { http://git-wip-us.apache.org/repos/asf/spark/blob/f63ba221/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/Command.scala ---------------------------------------------------------------------- diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/Command.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/Command.scala new file mode 100644 index 0000000..75a5b10 --- /dev/null +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/Command.scala @@ -0,0 +1,25 @@ +/* + * 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.spark.sql.catalyst.plans.logical + +/** + * A logical node that represents a non-query command to be executed by the system. For example, + * commands can be used by parsers to represent DDL operations. Commands, unlike queries, are + * eagerly executed. + */ +trait Command http://git-wip-us.apache.org/repos/asf/spark/blob/f63ba221/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/commands.scala ---------------------------------------------------------------------- diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/commands.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/commands.scala deleted file mode 100644 index 0ec3ff3..0000000 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/commands.scala +++ /dev/null @@ -1,55 +0,0 @@ -/* - * 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.spark.sql.catalyst.plans.logical - -import org.apache.spark.sql.catalyst.FunctionIdentifier -import org.apache.spark.sql.catalyst.expressions.{Attribute, AttributeReference} -import org.apache.spark.sql.types.StringType - -/** - * A logical node that represents a non-query command to be executed by the system. For example, - * commands can be used by parsers to represent DDL operations. Commands, unlike queries, are - * eagerly executed. - */ -trait Command - -/** - * Returned for the "DESCRIBE FUNCTION [EXTENDED] functionName" command. - * - * @param functionName The function to be described. - * @param isExtended True if "DESCRIBE FUNCTION EXTENDED" is used. Otherwise, false. - */ -private[sql] case class DescribeFunction( - functionName: FunctionIdentifier, - isExtended: Boolean) extends LogicalPlan with Command { - - override def children: Seq[LogicalPlan] = Seq.empty - override val output: Seq[Attribute] = Seq( - AttributeReference("function_desc", StringType, nullable = false)()) -} - -/** - * Returned for the "SHOW FUNCTIONS" command, which will list all of the - * registered function list. - */ -private[sql] case class ShowFunctions( - db: Option[String], pattern: Option[String]) extends LogicalPlan with Command { - override def children: Seq[LogicalPlan] = Seq.empty - override val output: Seq[Attribute] = Seq( - AttributeReference("function", StringType, nullable = false)()) -} http://git-wip-us.apache.org/repos/asf/spark/blob/f63ba221/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/UnsupportedOperationsSuite.scala ---------------------------------------------------------------------- diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/UnsupportedOperationsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/UnsupportedOperationsSuite.scala index 674277b..aaeee0f 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/UnsupportedOperationsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/UnsupportedOperationsSuite.scala @@ -19,7 +19,6 @@ package org.apache.spark.sql.catalyst.analysis import org.apache.spark.SparkFunSuite import org.apache.spark.sql.AnalysisException -import org.apache.spark.sql.catalyst.FunctionIdentifier import org.apache.spark.sql.catalyst.dsl.expressions._ import org.apache.spark.sql.catalyst.dsl.plans._ import org.apache.spark.sql.catalyst.encoders.ExpressionEncoder @@ -29,6 +28,12 @@ import org.apache.spark.sql.catalyst.plans._ import org.apache.spark.sql.catalyst.plans.logical._ import org.apache.spark.sql.types.IntegerType +/** A dummy command for testing unsupported operations. */ +case class DummyCommand() extends LogicalPlan with Command { + override def output: Seq[Attribute] = Nil + override def children: Seq[LogicalPlan] = Nil +} + class UnsupportedOperationsSuite extends SparkFunSuite { val attribute = AttributeReference("a", IntegerType, nullable = true)() @@ -70,7 +75,7 @@ class UnsupportedOperationsSuite extends SparkFunSuite { // Commands assertNotSupportedInStreamingPlan( "commmands", - DescribeFunction(FunctionIdentifier("func", database = None), true), + DummyCommand(), outputMode = Append, expectedMsgs = "commands" :: Nil) http://git-wip-us.apache.org/repos/asf/spark/blob/f63ba221/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala ---------------------------------------------------------------------- diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala index 5811d32..a6fad2d 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala @@ -14,6 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.apache.spark.sql.catalyst.parser import org.apache.spark.sql.Row @@ -24,17 +25,21 @@ import org.apache.spark.sql.catalyst.plans._ import org.apache.spark.sql.catalyst.plans.logical._ import org.apache.spark.sql.types.IntegerType - +/** + * Parser test cases for rules defined in [[CatalystSqlParser]] / [[AstBuilder]]. + * + * There is also SparkSqlParserSuite in sql/core module for parser rules defined in sql/core module. + */ class PlanParserSuite extends PlanTest { import CatalystSqlParser._ import org.apache.spark.sql.catalyst.dsl.expressions._ import org.apache.spark.sql.catalyst.dsl.plans._ - def assertEqual(sqlCommand: String, plan: LogicalPlan): Unit = { + private def assertEqual(sqlCommand: String, plan: LogicalPlan): Unit = { comparePlans(parsePlan(sqlCommand), plan) } - def intercept(sqlCommand: String, messages: String*): Unit = { + private def intercept(sqlCommand: String, messages: String*): Unit = { val e = intercept[ParseException](parsePlan(sqlCommand)) messages.foreach { message => assert(e.message.contains(message)) @@ -53,25 +58,6 @@ class PlanParserSuite extends PlanTest { intercept("EXPLAIN formatted SELECT 1", "Unsupported SQL statement") } - test("show functions") { - assertEqual("show functions", ShowFunctions(None, None)) - assertEqual("show functions foo", ShowFunctions(None, Some("foo"))) - assertEqual("show functions foo.bar", ShowFunctions(Some("foo"), Some("bar"))) - assertEqual("show functions 'foo\\\\.*'", ShowFunctions(None, Some("foo\\.*"))) - intercept("show functions foo.bar.baz", "Unsupported function name") - } - - test("describe function") { - assertEqual("describe function bar", - DescribeFunction(FunctionIdentifier("bar", database = None), isExtended = false)) - assertEqual("describe function extended bar", - DescribeFunction(FunctionIdentifier("bar", database = None), isExtended = true)) - assertEqual("describe function foo.bar", - DescribeFunction(FunctionIdentifier("bar", database = Option("foo")), isExtended = false)) - assertEqual("describe function extended f.bar", - DescribeFunction(FunctionIdentifier("bar", database = Option("f")), isExtended = true)) - } - test("set operations") { val a = table("a").select(star()) val b = table("b").select(star()) http://git-wip-us.apache.org/repos/asf/spark/blob/f63ba221/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---------------------------------------------------------------------- diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala index f85d606..57f534c 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala @@ -24,7 +24,7 @@ import org.antlr.v4.runtime.{ParserRuleContext, Token} import org.antlr.v4.runtime.tree.TerminalNode import org.apache.spark.sql.SaveMode -import org.apache.spark.sql.catalyst.TableIdentifier +import org.apache.spark.sql.catalyst.{FunctionIdentifier, TableIdentifier} import org.apache.spark.sql.catalyst.catalog._ import org.apache.spark.sql.catalyst.parser._ import org.apache.spark.sql.catalyst.parser.SqlBaseParser._ @@ -494,6 +494,37 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { } /** + * Create a plan for a DESCRIBE FUNCTION command. + */ + override def visitDescribeFunction(ctx: DescribeFunctionContext): LogicalPlan = withOrigin(ctx) { + import ctx._ + val functionName = + if (describeFuncName.STRING() != null) { + FunctionIdentifier(string(describeFuncName.STRING()), database = None) + } else if (describeFuncName.qualifiedName() != null) { + visitFunctionName(describeFuncName.qualifiedName) + } else { + FunctionIdentifier(describeFuncName.getText, database = None) + } + DescribeFunctionCommand(functionName, EXTENDED != null) + } + + /** + * Create a plan for a SHOW FUNCTIONS command. + */ + override def visitShowFunctions(ctx: ShowFunctionsContext): LogicalPlan = withOrigin(ctx) { + import ctx._ + if (qualifiedName != null) { + val name = visitFunctionName(qualifiedName) + ShowFunctionsCommand(name.database, Some(name.funcName)) + } else if (pattern != null) { + ShowFunctionsCommand(None, Some(string(pattern))) + } else { + ShowFunctionsCommand(None, None) + } + } + + /** * Create a [[CreateFunctionCommand]] command. * * For example: http://git-wip-us.apache.org/repos/asf/spark/blob/f63ba221/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala ---------------------------------------------------------------------- diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala index c46cecc..e405252 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala @@ -416,12 +416,6 @@ private[sql] abstract class SparkStrategies extends QueryPlanner[SparkPlan] { c.child) ExecutedCommandExec(cmd) :: Nil - case logical.ShowFunctions(db, pattern) => - ExecutedCommandExec(ShowFunctionsCommand(db, pattern)) :: Nil - - case logical.DescribeFunction(function, extended) => - ExecutedCommandExec(DescribeFunctionCommand(function, extended)) :: Nil - case _ => Nil } } http://git-wip-us.apache.org/repos/asf/spark/blob/f63ba221/sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala ---------------------------------------------------------------------- diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala new file mode 100644 index 0000000..e2858bb --- /dev/null +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala @@ -0,0 +1,68 @@ +/* + * 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.spark.sql.execution + +import org.apache.spark.sql.catalyst.FunctionIdentifier +import org.apache.spark.sql.catalyst.parser.ParseException +import org.apache.spark.sql.catalyst.plans.PlanTest +import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan +import org.apache.spark.sql.execution.command.{DescribeFunctionCommand, ShowFunctionsCommand} +import org.apache.spark.sql.internal.SQLConf + +/** + * Parser test cases for rules defined in [[SparkSqlParser]]. + * + * See [[org.apache.spark.sql.catalyst.parser.PlanParserSuite]] for rules + * defined in the Catalyst module. + */ +class SparkSqlParserSuite extends PlanTest { + + private lazy val parser = new SparkSqlParser(new SQLConf) + + private def assertEqual(sqlCommand: String, plan: LogicalPlan): Unit = { + comparePlans(parser.parsePlan(sqlCommand), plan) + } + + private def intercept(sqlCommand: String, messages: String*): Unit = { + val e = intercept[ParseException](parser.parsePlan(sqlCommand)) + messages.foreach { message => + assert(e.message.contains(message)) + } + } + + test("show functions") { + assertEqual("show functions", ShowFunctionsCommand(None, None)) + assertEqual("show functions foo", ShowFunctionsCommand(None, Some("foo"))) + assertEqual("show functions foo.bar", ShowFunctionsCommand(Some("foo"), Some("bar"))) + assertEqual("show functions 'foo\\\\.*'", ShowFunctionsCommand(None, Some("foo\\.*"))) + intercept("show functions foo.bar.baz", "Unsupported function name") + } + + test("describe function") { + assertEqual("describe function bar", + DescribeFunctionCommand(FunctionIdentifier("bar", database = None), isExtended = false)) + assertEqual("describe function extended bar", + DescribeFunctionCommand(FunctionIdentifier("bar", database = None), isExtended = true)) + assertEqual("describe function foo.bar", + DescribeFunctionCommand( + FunctionIdentifier("bar", database = Option("foo")), isExtended = false)) + assertEqual("describe function extended f.bar", + DescribeFunctionCommand(FunctionIdentifier("bar", database = Option("f")), isExtended = true)) + } + +} http://git-wip-us.apache.org/repos/asf/spark/blob/f63ba221/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala ---------------------------------------------------------------------- diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala index eab1f55..850fca5 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala @@ -33,7 +33,7 @@ import org.apache.spark.sql.types.{IntegerType, StringType, StructType} // TODO: merge this with DDLSuite (SPARK-14441) class DDLCommandSuite extends PlanTest { - private val parser = new SparkSqlParser(new SQLConf) + private lazy val parser = new SparkSqlParser(new SQLConf) private def assertUnsupported(sql: String, containsThesePhrases: Seq[String] = Seq()): Unit = { val e = intercept[ParseException] { http://git-wip-us.apache.org/repos/asf/spark/blob/f63ba221/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveComparisonTest.scala ---------------------------------------------------------------------- diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveComparisonTest.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveComparisonTest.scala index b12f3aa..65d53de 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveComparisonTest.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveComparisonTest.scala @@ -29,7 +29,7 @@ import org.apache.spark.sql.catalyst.SQLBuilder import org.apache.spark.sql.catalyst.planning.PhysicalOperation import org.apache.spark.sql.catalyst.plans.logical._ import org.apache.spark.sql.catalyst.util._ -import org.apache.spark.sql.execution.command.{DescribeTableCommand, ExplainCommand, SetCommand, ShowColumnsCommand} +import org.apache.spark.sql.execution.command._ import org.apache.spark.sql.hive.{InsertIntoHiveTable => LogicalInsertIntoHiveTable} import org.apache.spark.sql.hive.test.{TestHive, TestHiveQueryExecution} @@ -414,8 +414,8 @@ abstract class HiveComparisonTest // We will ignore the ExplainCommand, ShowFunctions, DescribeFunction if ((!hiveQuery.logical.isInstanceOf[ExplainCommand]) && - (!hiveQuery.logical.isInstanceOf[ShowFunctions]) && - (!hiveQuery.logical.isInstanceOf[DescribeFunction]) && + (!hiveQuery.logical.isInstanceOf[ShowFunctionsCommand]) && + (!hiveQuery.logical.isInstanceOf[DescribeFunctionCommand]) && preparedHive != catalyst) { val hivePrintOut = s"== HIVE - ${preparedHive.size} row(s) ==" +: preparedHive --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org
