srielau commented on code in PR #55569:
URL: https://github.com/apache/spark/pull/55569#discussion_r3161612547
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogManager.scala:
##########
@@ -349,4 +350,30 @@ private[sql] object CatalogManager {
compact(JArray(entries.map(parts =>
JArray(parts.map(JString(_)).toList)).toList))
}
+
+ /**
+ * Parse a stored frozen path string from view/function metadata.
+ * Returns None if the payload is malformed.
+ */
+ def deserializePathEntries(storedPathStr: String): Option[Seq[Seq[String]]]
= {
+ import org.json4s.JsonAST.{JArray, JString}
+ import org.json4s.jackson.JsonMethods.parse
+
+ Try(parse(storedPathStr)).toOption match {
+ case Some(JArray(entries)) if entries.nonEmpty =>
+ val converted = entries.foldLeft(Option(Seq.empty[Seq[String]])) {
(acc, entry) =>
+ acc.flatMap { collected =>
+ entry match {
+ case JArray(parts) if parts.nonEmpty =>
+ val strings = parts.collect { case JString(s) => s }
+ if (strings.size == parts.size) Some(collected :+ strings)
+ else None
+ case _ => None
+ }
+ }
+ }
+ converted.filter(_.nonEmpty)
Review Comment:
Good catch, thanks. Updated in 05810472088 by removing the unreachable
`filter(_.nonEmpty)` and simplifying the branch.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogManager.scala:
##########
@@ -349,4 +350,30 @@ private[sql] object CatalogManager {
compact(JArray(entries.map(parts =>
JArray(parts.map(JString(_)).toList)).toList))
}
+
+ /**
+ * Parse a stored frozen path string from view/function metadata.
+ * Returns None if the payload is malformed.
+ */
+ def deserializePathEntries(storedPathStr: String): Option[Seq[Seq[String]]]
= {
+ import org.json4s.JsonAST.{JArray, JString}
+ import org.json4s.jackson.JsonMethods.parse
+
+ Try(parse(storedPathStr)).toOption match {
+ case Some(JArray(entries)) if entries.nonEmpty =>
+ val converted = entries.foldLeft(Option(Seq.empty[Seq[String]])) {
(acc, entry) =>
+ acc.flatMap { collected =>
+ entry match {
+ case JArray(parts) if parts.nonEmpty =>
+ val strings = parts.collect { case JString(s) => s }
+ if (strings.size == parts.size) Some(collected :+ strings)
+ else None
+ case _ => None
+ }
+ }
+ }
+ converted.filter(_.nonEmpty)
+ case _ => None
Review Comment:
Implemented in 05810472088. `deserializePathEntries` now logs warnings for
malformed payloads (parse failure, non-array root, malformed entries), while
keeping empty-array payloads valid and silent.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -224,7 +225,10 @@ object AnalysisContext {
def withAnalysisContext[A](function: SQLFunction)(f: => A): A = {
val originContext = value.get()
- val context = originContext.copy(collation = function.collation)
+ val context = originContext.copy(
+ resolutionPathEntries = function.functionStoredResolutionPath
+ .flatMap(CatalogManager.deserializePathEntries),
+ collation = function.collation)
Review Comment:
Confirmed and documented in 05810472088. I added an inline comment in
`withAnalysisContext(function)` to make this intent explicit: function-body
analysis must not inherit caller-pinned path and should use only function
metadata.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/SQLFunctionSuite.scala:
##########
@@ -111,4 +112,117 @@ class SQLFunctionSuite extends SharedSparkSession {
)
}
}
+
+ test("SPARK-56639: SQL function uses frozen SQL path") {
+ withSQLConf(SQLConf.PATH_ENABLED.key -> "true") {
+ withDatabase("path_func_db_a", "path_func_db_b") {
+ withTable("path_func_db_a.frozen_t", "path_func_db_b.frozen_t") {
+ withUserDefinedFunction("frozen_fn" -> false) {
+ sql("USE default")
+ sql("CREATE DATABASE path_func_db_a")
+ sql("CREATE DATABASE path_func_db_b")
+ sql("CREATE TABLE path_func_db_a.frozen_t USING parquet AS SELECT
10 AS id")
+ sql("CREATE TABLE path_func_db_b.frozen_t USING parquet AS SELECT
20 AS id")
+ try {
+ sql("SET PATH = spark_catalog.path_func_db_a, system.builtin")
+ sql(
+ """
+ |CREATE FUNCTION frozen_fn()
+ |RETURNS INT
+ |RETURN (SELECT MAX(id) FROM frozen_t)
+ |""".stripMargin)
+ sql("SET PATH = spark_catalog.path_func_db_b, system.builtin")
+
+ checkAnswer(sql("SELECT MAX(id) FROM frozen_t"), Row(20))
+ checkAnswer(sql("SELECT default.frozen_fn()"), Row(10))
+ } finally {
+ sql("SET PATH = DEFAULT_PATH")
+ }
+ }
+ }
+ }
+ }
+ }
+
+ test("SPARK-56639: SQL table function uses frozen SQL path") {
+ withSQLConf(SQLConf.PATH_ENABLED.key -> "true") {
+ withDatabase("path_tvf_db_a", "path_tvf_db_b") {
+ withTable("path_tvf_db_a.frozen_t", "path_tvf_db_b.frozen_t") {
+ withUserDefinedFunction("frozen_tvf" -> false) {
+ sql("USE default")
+ sql("CREATE DATABASE path_tvf_db_a")
+ sql("CREATE DATABASE path_tvf_db_b")
+ sql("CREATE TABLE path_tvf_db_a.frozen_t USING parquet AS SELECT
100 AS id")
+ sql("CREATE TABLE path_tvf_db_b.frozen_t USING parquet AS SELECT
200 AS id")
+ try {
+ sql("SET PATH = spark_catalog.path_tvf_db_a, system.builtin")
+ sql(
+ """
+ |CREATE FUNCTION frozen_tvf()
+ |RETURNS TABLE(id INT)
+ |RETURN SELECT MAX(id) AS id FROM frozen_t
+ |""".stripMargin)
+ sql("SET PATH = spark_catalog.path_tvf_db_b, system.builtin")
+
+ checkAnswer(sql("SELECT MAX(id) FROM frozen_t"), Row(200))
+ checkAnswer(sql("SELECT * FROM default.frozen_tvf()"), Row(100))
+ } finally {
+ sql("SET PATH = DEFAULT_PATH")
+ }
+ }
+ }
+ }
+ }
+ }
+
+ test("SPARK-56639: current_schema/current_path in SQL functions use invoker
context") {
Review Comment:
Agreed. Updated in 05810472088 by adding one-line regression-guard comments
in both `SQLFunctionSuite` and `SQLViewSuite` so these tests are clearly scoped
to preventing frozen-path leakage into `CURRENT_SCHEMA/CURRENT_PATH`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
