-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-32401][SQL] Migrate function related commands to use UnresolvedFunc to resolve function identifier #29198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
04f629d
38756fc
f149cf7
0e9f12f
daf4d6b
27f2fb1
7924572
c662619
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3595,7 +3595,7 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging | |
| } else { | ||
| Seq(describeFuncName.getText) | ||
| } | ||
| DescribeFunctionStatement(functionName, EXTENDED != null) | ||
| DescribeFunction(UnresolvedFunc(functionName), EXTENDED != null) | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -3610,8 +3610,10 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging | |
| case Some(x) => throw new ParseException(s"SHOW $x FUNCTIONS not supported", ctx) | ||
| } | ||
| val pattern = Option(ctx.pattern).map(string(_)) | ||
| val functionName = Option(ctx.multipartIdentifier).map(visitMultipartIdentifier) | ||
| ShowFunctionsStatement(userScope, systemScope, pattern, functionName) | ||
| val functionName = Option(ctx.multipartIdentifier) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: maybe
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. or keep it
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed it to |
||
| .map(visitMultipartIdentifier) | ||
| .map(UnresolvedFunc(_)) | ||
| ShowFunctions(functionName, userScope, systemScope, pattern) | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -3624,8 +3626,8 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging | |
| */ | ||
| override def visitDropFunction(ctx: DropFunctionContext): LogicalPlan = withOrigin(ctx) { | ||
| val functionName = visitMultipartIdentifier(ctx.multipartIdentifier) | ||
| DropFunctionStatement( | ||
| functionName, | ||
| DropFunction( | ||
| UnresolvedFunc(functionName), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will analyzer give a nice error if
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nvm, this won't happen for now. |
||
| ctx.EXISTS != null, | ||
| ctx.TEMPORARY != null) | ||
| } | ||
|
|
@@ -3650,12 +3652,24 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging | |
| } | ||
| } | ||
|
|
||
| val functionIdentifier = visitMultipartIdentifier(ctx.multipartIdentifier) | ||
| CreateFunctionStatement( | ||
| functionIdentifier, | ||
| val nameParts = visitMultipartIdentifier(ctx.multipartIdentifier) | ||
| val isTemp = ctx.TEMPORARY != null | ||
| val func: LogicalPlan = if (isTemp) { | ||
| import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ | ||
| // temp func doesn't belong to any catalog and we shouldn't resolve catalog in the name. | ||
| if (nameParts.length > 2) { | ||
| throw new AnalysisException(s"Unsupported function name '${nameParts.quoted}'") | ||
| } | ||
| ResolvedFunc(nameParts.asIdentifier) | ||
|
imback82 marked this conversation as resolved.
Outdated
|
||
| } else { | ||
| UnresolvedFunc(nameParts) | ||
| } | ||
|
|
||
| CreateFunction( | ||
| func, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure how to deal with CREATE commands yet. For now, CREATE TABLE and VIEW still use statement plans. We don't need to do lookup for CREATE commands, so Can we keep it unchanged?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. Will revert this change. |
||
| string(ctx.className), | ||
| resources.toSeq, | ||
| ctx.TEMPORARY != null, | ||
| resources, | ||
| isTemp, | ||
| ctx.EXISTS != null, | ||
| ctx.REPLACE != null) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ package org.apache.spark.sql.catalyst.plans.logical | |
|
|
||
| import org.apache.spark.sql.catalyst.analysis.{NamedRelation, UnresolvedException} | ||
| import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec | ||
| import org.apache.spark.sql.catalyst.catalog.FunctionResource | ||
| import org.apache.spark.sql.catalyst.expressions.{Attribute, AttributeReference, Expression, Unevaluable} | ||
| import org.apache.spark.sql.catalyst.plans.DescribeTableSchema | ||
| import org.apache.spark.sql.connector.catalog._ | ||
|
|
@@ -517,9 +518,50 @@ case class CommentOnTable(child: LogicalPlan, comment: String) extends Command { | |
| override def children: Seq[LogicalPlan] = child :: Nil | ||
| } | ||
|
|
||
| /** | ||
| * The logical plan of the CREATE FUNCTION command that works for v2 catalogs. | ||
| */ | ||
| case class CreateFunction( | ||
| child: LogicalPlan, | ||
| className: String, | ||
| resources: Seq[FunctionResource], | ||
| isTemp: Boolean, | ||
| ignoreIfExists: Boolean, | ||
| replace: Boolean) extends Command { | ||
| override def children: Seq[LogicalPlan] = child :: Nil | ||
| } | ||
|
|
||
| /** | ||
| * The logical plan of the REFRESH FUNCTION command that works for v2 catalogs. | ||
| */ | ||
| case class RefreshFunction(child: LogicalPlan) extends Command { | ||
| override def children: Seq[LogicalPlan] = child :: Nil | ||
| } | ||
|
|
||
| /** | ||
| * The logical plan of the DESCRIBE FUNCTION command that works for v2 catalogs. | ||
| */ | ||
| case class DescribeFunction(child: LogicalPlan, isExtended: Boolean) extends Command { | ||
| override def children: Seq[LogicalPlan] = child :: Nil | ||
| } | ||
|
|
||
| /** | ||
| * The logical plan of the DROP FUNCTION command that works for v2 catalogs. | ||
| */ | ||
| case class DropFunction( | ||
| child: LogicalPlan, | ||
| ifExists: Boolean, | ||
| isTemp: Boolean) extends Command { | ||
| override def children: Seq[LogicalPlan] = child :: Nil | ||
| } | ||
|
|
||
| /** | ||
| * The logical plan of the SHOW FUNCTIONS command that works for v2 catalogs. | ||
| */ | ||
| case class ShowFunctions( | ||
| child: Option[LogicalPlan], | ||
| userScope: Boolean, | ||
| systemScope: Boolean, | ||
| pattern: Option[String]) extends Command { | ||
| override def children: Seq[LogicalPlan] = if (child.isDefined) { child.get :: Nil } else { Nil } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks! |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -156,10 +156,7 @@ private[sql] trait LookupCatalog extends Logging { | |
| } | ||
| } | ||
|
|
||
| // TODO: move function related v2 statements to the new framework. | ||
| def parseSessionCatalogFunctionIdentifier( | ||
| nameParts: Seq[String], | ||
| sql: String): FunctionIdentifier = { | ||
| def parseSessionCatalogFunctionIdentifier(nameParts: Seq[String]): FunctionIdentifier = { | ||
| if (nameParts.length == 1 && catalogManager.v1SessionCatalog.isTempFunction(nameParts.head)) { | ||
| return FunctionIdentifier(nameParts.head) | ||
| } | ||
|
|
@@ -179,7 +176,7 @@ private[sql] trait LookupCatalog extends Logging { | |
| } | ||
| } | ||
|
|
||
| case _ => throw new AnalysisException(s"$sql is only supported in v1 catalog") | ||
| case _ => throw new AnalysisException("function is only supported in v1 catalog") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. function -> function command?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is called from |
||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.