-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-33692][SQL] View should use captured catalog and namespace to lookup function #30662
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 4 commits
f3c2fd6
c104cfe
00e32d6
ef7c6b2
d87f249
11523f4
a8c7722
ab440e3
b30272c
632f3d6
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 |
|---|---|---|
|
|
@@ -1488,12 +1488,28 @@ class SessionCatalog( | |
| // We probably shouldn't use a single FunctionRegistry to register all three kinds of functions | ||
| // (built-in, temp, and external). | ||
| if (name.database.isEmpty && functionRegistry.functionExists(name)) { | ||
| // This function has been already loaded into the function registry. | ||
| return functionRegistry.lookupFunction(name, children) | ||
| val isResolvingView = AnalysisContext.get.catalogAndNamespace.nonEmpty | ||
| // We lookup function without database in two cases: | ||
| // 1. the function is not a temporary function | ||
| // 2. the function is a temporary function but we are not resolving view | ||
| if (!isTemporaryFunction(name) || !isResolvingView) { | ||
| // This function has been already loaded into the function registry. | ||
| return functionRegistry.lookupFunction(name, children) | ||
| } | ||
| } | ||
|
|
||
| // Get the database from AnalysisContext if it's defined, otherwise, use current database | ||
| val currentDatabase = AnalysisContext.get.catalogAndNamespace match { | ||
| case Seq() => getCurrentDatabase | ||
| case Seq(_, db) => db | ||
| case Seq(catalog, namespace @ _*) => | ||
| throw new AnalysisException( | ||
| s"Unsupported catalog ${catalog} and " + | ||
|
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. we can just say: |
||
| s"namespace '${namespace.mkString("[", ".", "]")}' for function '$name'") | ||
| } | ||
|
|
||
| // If the name itself is not qualified, add the current database to it. | ||
| val database = formatDatabaseName(name.database.getOrElse(getCurrentDatabase)) | ||
| val database = formatDatabaseName(name.database.getOrElse(currentDatabase)) | ||
| val qualifiedName = name.copy(database = Some(database)) | ||
|
|
||
| if (functionRegistry.functionExists(qualifiedName)) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ import org.apache.spark.sql.catalyst.TableIdentifier | |
| import org.apache.spark.sql.catalyst.catalog.{CatalogStorageFormat, CatalogTable, CatalogTableType} | ||
| import org.apache.spark.sql.execution.SQLViewSuite | ||
| import org.apache.spark.sql.hive.test.TestHiveSingleton | ||
| import org.apache.spark.sql.internal.SQLConf | ||
| import org.apache.spark.sql.types.{NullType, StructType} | ||
|
|
||
| /** | ||
|
|
@@ -72,9 +73,13 @@ class HiveSQLViewSuite extends SQLViewSuite with TestHiveSingleton { | |
| withTable("tab1") { | ||
| (1 to 10).map(i => s"$i").toDF("id").write.saveAsTable("tab1") | ||
|
|
||
| // temporary view | ||
| sql(s"CREATE TEMPORARY VIEW tempView1 AS SELECT $tempFunctionName(id) from tab1") | ||
| checkAnswer(sql("select count(*) FROM tempView1"), Row(10)) | ||
| // TODO: temporary function support for temporary view with sql text stored will | ||
|
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. yeah, but there are also test failure in udf-inner-join.sql related to temp function in temp views. I changed to another way to find these test cases. could you check again? |
||
| // be fixed in another PR | ||
| withSQLConf(SQLConf.STORE_ANALYZED_PLAN_FOR_VIEW.key -> "true") { | ||
| // temporary view | ||
| sql(s"CREATE TEMPORARY VIEW tempView1 AS SELECT $tempFunctionName(id) from tab1") | ||
| checkAnswer(sql("select count(*) FROM tempView1"), Row(10)) | ||
| } | ||
|
|
||
| // permanent view | ||
| val e = intercept[AnalysisException] { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the function is built-inThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are also external functions, in
isTemporaryFunction: