cloud-fan commented on code in PR #55717:
URL: https://github.com/apache/spark/pull/55717#discussion_r3206700788
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -1981,14 +1981,15 @@ class Analyzer(
* This is used for special syntax transformations (e.g., COUNT(*) ->
COUNT(1)) that
* should only apply to builtin functions, not to user-defined functions.
*
- * In legacy mode (sessionOrder="first"), temp functions shadow builtins,
so an
- * unqualified name that matches a temp function should NOT be treated as
builtin.
+ * When the effective SQL PATH puts `system.session` before
`system.builtin`, temp
+ * functions shadow builtins, so an unqualified name that matches a temp
function
+ * should NOT be treated as builtin.
*/
private def matchesFunctionName(nameParts: Seq[String], expectedName:
String): Boolean = {
if (!FunctionResolution.isUnqualifiedOrBuiltinFunctionName(nameParts,
expectedName)) {
return false
}
- if (nameParts.size == 1 && conf.sessionFunctionResolutionOrder ==
"first") {
+ if (nameParts.size == 1 &&
functionResolution.isSessionBeforeBuiltinInPath) {
Review Comment:
**Single-pass resolver lacks the path-driven gate.** This gate
(`isSessionBeforeBuiltinInPath`) is now path-driven in the iterative analyzer,
so a `SET PATH = system.session, system.builtin, ...` correctly skips
`count(*)→count(1)` rewrite for a temp `count`. The single-pass equivalent --
`FunctionResolverUtils.normalizeCountExpression`, called from
`FunctionResolver` / `HigherOrderFunctionResolver` -- has no such gate;
`isCount` only checks the name. Pre-existing divergence (single-pass also
didn't honor `sessionFunctionResolutionOrder == "first"`), but the PR makes the
iterative gate easier to trip via `SET PATH`, so the asymmetry becomes more
reachable. Out of scope to fix here, or worth a follow-up?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogManager.scala:
##########
@@ -138,8 +149,59 @@ class CatalogManager(
private var _sessionPath: Option[Seq[SessionPathEntry]] = None
- /** Returns the raw stored session path entries, or None if no path is set.
*/
- def sessionPathEntries: Option[Seq[SessionPathEntry]] = synchronized {
_sessionPath }
+ /**
+ * Cache for [[confDefaultPathEntries]]: stores the expanded
[[SessionPathEntry]] list keyed
+ * on the trimmed [[SQLConf.DEFAULT_PATH]] string and the
[[SQLConf.SESSION_FUNCTION_RESOLUTION_ORDER]]
+ * value (the only conf that affects the expansion of `DEFAULT_PATH` /
`SYSTEM_PATH` tokens).
+ * `CurrentSchemaEntry` markers are preserved unresolved so the cache stays
valid across
+ * `USE SCHEMA`.
+ */
+ private val confDefaultPathCache =
+ new AtomicReference[Option[(String, String, Seq[SessionPathEntry])]](None)
+
+ /**
+ * Returns the effective session path entries: the explicit `SET PATH` value
if stored,
+ * else the parsed [[SQLConf.DEFAULT_PATH]] conf if non-empty (mirroring how
+ * [[currentCatalog]] falls back to [[SQLConf.DEFAULT_CATALOG]]). Returns
`None` when
+ * [[SQLConf.PATH_ENABLED]] is false or both sources are empty.
+ */
+ def sessionPathEntries: Option[Seq[SessionPathEntry]] = synchronized {
+ if (!conf.pathEnabled) None
+ else _sessionPath.orElse(confDefaultPathEntries)
+ }
+
+ /** Raw `_sessionPath` (post-`SET PATH`), without the
[[SQLConf.DEFAULT_PATH]] fallback. */
+ def storedSessionPathEntries: Option[Seq[SessionPathEntry]] = synchronized {
_sessionPath }
+
+ /**
+ * Parsed and expanded [[SQLConf.DEFAULT_PATH]] value, or `None` when the
conf is empty.
+ * Reuses the SET PATH grammar via [[CatalystSqlParser.parsePathElements]].
An inner
+ * `DEFAULT_PATH` token resolves to the spark-builtin default ordering
(cycle break).
+ *
+ * Unlike `SET PATH`, this does NOT run a duplicate check: lookup uses
first-match
+ * resolution, so any redundant entry (including ones that only collide
after a later
+ * `USE SCHEMA`) is dead code rather than an error. Cached so the hot path
is a single
+ * atomic load on conf-stable sessions.
+ */
+ def confDefaultPathEntries: Option[Seq[SessionPathEntry]] = {
+ val confValue = conf.defaultPath
+ if (confValue == null || confValue.trim.isEmpty) {
+ confDefaultPathCache.set(None)
+ None
+ } else {
+ val trimmed = confValue.trim
+ val sessionOrder = conf.sessionFunctionResolutionOrder
+ val expanded = confDefaultPathCache.get() match {
+ case Some((k, ord, cached)) if k == trimmed && ord == sessionOrder =>
cached
+ case _ =>
+ val elements = CatalystSqlParser.parsePathElements(trimmed)
+ val computed = PathElement.expand(elements, conf, this,
isConfDefaultExpansion = true)
Review Comment:
**`PathRef` reaching `DEFAULT_PATH` produces stale cache hits.** `PathRef`
(`PATH` keyword) is meaningful in `SET PATH` as a composition idiom (`SET PATH
= PATH, schema` = append) -- but it's nonsense inside `spark.sql.defaultPath`
(the conf *is* the baseline path; there's no prior path to splice in). And
allowing it produces a concrete cache bug: `confDefaultPathCache` keys on
`(trimmed conf, sessionFunctionResolutionOrder)`, but `PathElement.expand` for
`PathRef` reads `storedSessionPathEntries.getOrElse(defaultPathExpansion)` --
sensitive to `_sessionPath` at compute time, which isn't in the key. Reachable
via:
1. `spark.sql.defaultPath = "PATH, system.builtin"`
2. `SET PATH = X` (so `_sessionPath = Some([X])`)
3. `SET PATH = DEFAULT_PATH` → cache miss → captures `PathRef → [X]`
4. `SET PATH = Y`
5. `SET PATH = DEFAULT_PATH` → cache HIT → still returns `[X,
system.builtin]`
Cleanest fix: reject `PathRef` in `DEFAULT_PATH` at conf SET time. Extend
the existing `.checkValue` on `SQLConf.DEFAULT_PATH`
(`SQLConf.scala:2487-2492`) to also fail when the parsed elements contain
`PathRef`. Same pattern as the recently-moved `SchemaInPath` arity check --
surface the user's mistake at SET-conf time with the conf key in the error, not
later via a stale cache hit. Kills the TOCTOU at the source rather than
papering over it with a smarter cache key.
##########
sql/core/src/test/scala/org/apache/spark/sql/SetPathSuite.scala:
##########
@@ -568,4 +570,233 @@ class SetPathSuite extends SharedSparkSession {
}
}
}
+
+ // --- spark.sql.defaultPath (SQLConf.DEFAULT_PATH) ---
+ // The conf carries the SET PATH grammar; sessionPathEntries falls back to
it lazily
+ // when no `SET PATH` has been issued, mirroring how `currentCatalog` falls
back to
+ // [[SQLConf.DEFAULT_CATALOG]].
+
+ test("DEFAULT_PATH conf: lazy fallback when no SET PATH issued") {
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "true",
+ SQLConf.DEFAULT_PATH.key -> "spark_catalog.default, system.builtin") {
+ val catalogManager = spark.sessionState.catalogManager
+ val priorSessionPath = catalogManager.storedSessionPathEntries
+ catalogManager.clearSessionPath()
+ try {
+ val entries = pathEntries(currentPath())
+ assert(entries == Seq("spark_catalog.default", "system.builtin"),
+ s"Expected DEFAULT_PATH conf to drive current_path(); got: $entries")
+ assert(catalogManager.storedSessionPathEntries.isEmpty,
+ "DEFAULT_PATH lookup must not write to the in-memory stored session
path")
+ } finally {
+ catalogManager.clearSessionPath()
+ priorSessionPath.foreach(catalogManager.setSessionPath)
+ }
+ }
+ }
+
+ test("DEFAULT_PATH conf: explicit SET PATH overrides the conf") {
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "true",
+ SQLConf.DEFAULT_PATH.key -> "system.builtin, system.session") {
+ val catalogManager = spark.sessionState.catalogManager
+ val priorSessionPath = catalogManager.storedSessionPathEntries
+ try {
+ sql("SET PATH = system.session, system.builtin")
+ val entries = pathEntries(currentPath())
+ assert(entries == Seq("system.session", "system.builtin"),
+ s"Expected SET PATH to win over DEFAULT_PATH conf; got: $entries")
+ } finally {
+ catalogManager.clearSessionPath()
+ priorSessionPath.foreach(catalogManager.setSessionPath)
+ }
+ }
+ }
+
+ test("DEFAULT_PATH conf: SET PATH = DEFAULT_PATH expands to the conf value")
{
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "true",
+ SQLConf.DEFAULT_PATH.key -> "system.session, system.builtin,
current_schema") {
+ val catalogManager = spark.sessionState.catalogManager
+ val priorSessionPath = catalogManager.storedSessionPathEntries
+ try {
+ sql("SET PATH = DEFAULT_PATH")
+ val entries = pathEntries(currentPath())
+ assert(entries.head.contains("system.session"),
+ s"DEFAULT_PATH expansion should follow conf order (session first);
got: $entries")
+ assert(catalogManager.storedSessionPathEntries.isDefined,
+ "After SET PATH the in-memory stored session path should be
populated")
+ } finally {
+ catalogManager.clearSessionPath()
+ priorSessionPath.foreach(catalogManager.setSessionPath)
+ }
+ }
+ }
+
+ test("DEFAULT_PATH conf: cycle break -- inner DEFAULT_PATH falls back to
builtin order") {
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "true",
+ SQLConf.DEFAULT_PATH.key -> "DEFAULT_PATH",
+ // Pin order conf to "first" so the spark-builtin default ordering is
observable.
+ SQLConf.SESSION_FUNCTION_RESOLUTION_ORDER.key -> "first") {
+ val catalogManager = spark.sessionState.catalogManager
+ val priorSessionPath = catalogManager.storedSessionPathEntries
+ catalogManager.clearSessionPath()
+ try {
+ val entries = pathEntries(currentPath())
+ assert(entries.head.contains("system.session"),
+ s"Inner DEFAULT_PATH should resolve to builtin order seeded by the
order conf " +
+ s"('first' -> session leading); got: $entries")
+ } finally {
+ catalogManager.clearSessionPath()
+ priorSessionPath.foreach(catalogManager.setSessionPath)
+ }
+ }
+ }
+
+ test("DEFAULT_PATH conf: invalid value rejected on SET
spark.sql.defaultPath") {
+ withPathEnabled {
+ val e = intercept[SparkIllegalArgumentException] {
+ sql("SET spark.sql.defaultPath = this is not a path")
+ }
+ assert(e.getCondition.startsWith("INVALID_CONF_VALUE"), e.getMessage)
+ }
+ }
+
+ test("DEFAULT_PATH conf: PATH disabled returns no fallback") {
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "false",
+ SQLConf.DEFAULT_PATH.key -> "system.session, system.builtin") {
+ val catalogManager = spark.sessionState.catalogManager
+ assert(catalogManager.sessionPathEntries.isEmpty,
+ "DEFAULT_PATH conf must not take effect when PATH is disabled")
+ }
+ }
+
+ // --- Path-driven security check (built on the lazy DEFAULT_PATH fallback)
---
+ // The "block temp function shadowing builtin" check is now driven by the
live PATH, so
+ // changes via SET PATH or DEFAULT_PATH take effect even when the legacy
order conf is
+ // left at its default.
+
+ test("path-driven security check: SET PATH putting session before builtin
blocks temp " +
+ "function with a builtin name") {
+ withPathEnabled {
+ val catalogManager = spark.sessionState.catalogManager
+ val priorSessionPath = catalogManager.storedSessionPathEntries
+ try {
+ // Default `sessionFunctionResolutionOrder` is "second" (builtin
first), but SET PATH
+ // overrides that to put session first. The security check must
reflect the live path.
+ sql("SET PATH = system.session, system.builtin")
+ val e = intercept[AnalysisException] {
+ sql("CREATE TEMPORARY FUNCTION count() RETURNS INT RETURN 1")
+ }
+ assert(e.getCondition == "ROUTINE_ALREADY_EXISTS", e.getMessage)
+ } finally {
+ sql("DROP TEMPORARY FUNCTION IF EXISTS session.count")
+ catalogManager.clearSessionPath()
+ priorSessionPath.foreach(catalogManager.setSessionPath)
+ }
+ }
+ }
+
+ test("path-driven security check: DEFAULT_PATH conf putting session before
builtin " +
+ "blocks temp function with a builtin name (no SET PATH issued)") {
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "true",
+ SQLConf.DEFAULT_PATH.key -> "system.session, system.builtin") {
+ val catalogManager = spark.sessionState.catalogManager
+ val priorSessionPath = catalogManager.storedSessionPathEntries
+ catalogManager.clearSessionPath()
+ try {
+ // Order conf is left at its default ("second"). The path-driven gate
must read
+ // DEFAULT_PATH and fire the security check for unqualified
temp/builtin collisions.
+ val e = intercept[AnalysisException] {
+ sql("CREATE TEMPORARY FUNCTION count() RETURNS INT RETURN 1")
+ }
+ assert(e.getCondition == "ROUTINE_ALREADY_EXISTS", e.getMessage)
+ } finally {
+ sql("DROP TEMPORARY FUNCTION IF EXISTS session.count")
+ catalogManager.clearSessionPath()
+ priorSessionPath.foreach(catalogManager.setSessionPath)
+ }
+ }
+ }
+
+ test("PATH enabled: SET PATH with only user schemas does not implicitly
resolve builtins") {
+ withPathEnabled {
+ sql("CREATE SCHEMA IF NOT EXISTS only_user_on_path")
+ try {
+ sql("SET PATH = spark_catalog.only_user_on_path")
+ val e = intercept[AnalysisException] {
+ sql("SELECT abs(-1)").collect()
+ }
+ assert(e.getCondition == "UNRESOLVED_ROUTINE", e.getMessage)
+ } finally {
+ sql("DROP SCHEMA IF EXISTS only_user_on_path")
+ }
+ }
+ }
+
+ test("PATH enabled: explicit SET PATH with system.session AFTER a user
catalog still " +
+ "reaches temp functions") {
+ // Explicit paths are honored as written: placing `system.session` after a
user catalog
+ // is the user's authorization for unqualified temp functions to resolve.
Contrast with
+ // the implicit (no SET PATH, no DEFAULT_PATH) form, which preserves the
security property
+ // of the seeded default path.
+ withPathEnabled {
+ sql("CREATE SCHEMA IF NOT EXISTS path_interleaved_user")
+ try {
+ sql("CREATE TEMPORARY FUNCTION path_interleaved_temp() RETURNS INT
RETURN 7")
+ try {
+ sql("SET PATH = system.builtin, spark_catalog.path_interleaved_user,
system.session")
+ checkAnswer(sql("SELECT path_interleaved_temp()"), Row(7))
+ } finally {
+ sql("DROP TEMPORARY FUNCTION IF EXISTS path_interleaved_temp")
+ }
+ } finally {
+ sql("DROP SCHEMA IF EXISTS path_interleaved_user")
+ }
+ }
+ }
+
+ test("PATH enabled: SET PATH with user schema before system.builtin still
resolves builtins") {
+ // Exercises the `leadingSystemFunctionKinds` else-branch in
CatalogManager: prefix before
+ // the first user catalog entry has no system entries, so kinds are taken
from the full path.
Review Comment:
Stale comment: references `leadingSystemFunctionKinds` (renamed to
`systemFunctionKindsFromPath` in `758ea643f11`) and an "else-branch" in
`CatalogManager`, but the current `systemFunctionKindsFromPath` is a flat
`flatMap` over the path with no else-branch. The test still exercises a useful
case, but the comment misleads the reader.
```suggestion
// Exercises systemFunctionKindsFromPath with a user-catalog entry
preceding
// system.builtin: the helper flat-scans the path, so Builtin still
appears
// in the kinds list and unqualified `abs` resolves.
```
##########
sql/core/src/test/scala/org/apache/spark/sql/SetPathSuite.scala:
##########
@@ -568,4 +570,233 @@ class SetPathSuite extends SharedSparkSession {
}
}
}
+
+ // --- spark.sql.defaultPath (SQLConf.DEFAULT_PATH) ---
+ // The conf carries the SET PATH grammar; sessionPathEntries falls back to
it lazily
+ // when no `SET PATH` has been issued, mirroring how `currentCatalog` falls
back to
+ // [[SQLConf.DEFAULT_CATALOG]].
+
+ test("DEFAULT_PATH conf: lazy fallback when no SET PATH issued") {
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "true",
+ SQLConf.DEFAULT_PATH.key -> "spark_catalog.default, system.builtin") {
+ val catalogManager = spark.sessionState.catalogManager
+ val priorSessionPath = catalogManager.storedSessionPathEntries
+ catalogManager.clearSessionPath()
+ try {
+ val entries = pathEntries(currentPath())
+ assert(entries == Seq("spark_catalog.default", "system.builtin"),
+ s"Expected DEFAULT_PATH conf to drive current_path(); got: $entries")
+ assert(catalogManager.storedSessionPathEntries.isEmpty,
+ "DEFAULT_PATH lookup must not write to the in-memory stored session
path")
+ } finally {
+ catalogManager.clearSessionPath()
+ priorSessionPath.foreach(catalogManager.setSessionPath)
+ }
+ }
+ }
+
+ test("DEFAULT_PATH conf: explicit SET PATH overrides the conf") {
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "true",
+ SQLConf.DEFAULT_PATH.key -> "system.builtin, system.session") {
+ val catalogManager = spark.sessionState.catalogManager
+ val priorSessionPath = catalogManager.storedSessionPathEntries
+ try {
+ sql("SET PATH = system.session, system.builtin")
+ val entries = pathEntries(currentPath())
+ assert(entries == Seq("system.session", "system.builtin"),
+ s"Expected SET PATH to win over DEFAULT_PATH conf; got: $entries")
+ } finally {
+ catalogManager.clearSessionPath()
+ priorSessionPath.foreach(catalogManager.setSessionPath)
+ }
+ }
+ }
+
+ test("DEFAULT_PATH conf: SET PATH = DEFAULT_PATH expands to the conf value")
{
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "true",
+ SQLConf.DEFAULT_PATH.key -> "system.session, system.builtin,
current_schema") {
+ val catalogManager = spark.sessionState.catalogManager
+ val priorSessionPath = catalogManager.storedSessionPathEntries
+ try {
+ sql("SET PATH = DEFAULT_PATH")
+ val entries = pathEntries(currentPath())
+ assert(entries.head.contains("system.session"),
+ s"DEFAULT_PATH expansion should follow conf order (session first);
got: $entries")
+ assert(catalogManager.storedSessionPathEntries.isDefined,
+ "After SET PATH the in-memory stored session path should be
populated")
+ } finally {
+ catalogManager.clearSessionPath()
+ priorSessionPath.foreach(catalogManager.setSessionPath)
+ }
+ }
+ }
+
+ test("DEFAULT_PATH conf: cycle break -- inner DEFAULT_PATH falls back to
builtin order") {
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "true",
+ SQLConf.DEFAULT_PATH.key -> "DEFAULT_PATH",
+ // Pin order conf to "first" so the spark-builtin default ordering is
observable.
+ SQLConf.SESSION_FUNCTION_RESOLUTION_ORDER.key -> "first") {
+ val catalogManager = spark.sessionState.catalogManager
+ val priorSessionPath = catalogManager.storedSessionPathEntries
+ catalogManager.clearSessionPath()
+ try {
+ val entries = pathEntries(currentPath())
+ assert(entries.head.contains("system.session"),
+ s"Inner DEFAULT_PATH should resolve to builtin order seeded by the
order conf " +
+ s"('first' -> session leading); got: $entries")
+ } finally {
+ catalogManager.clearSessionPath()
+ priorSessionPath.foreach(catalogManager.setSessionPath)
+ }
+ }
+ }
+
+ test("DEFAULT_PATH conf: invalid value rejected on SET
spark.sql.defaultPath") {
+ withPathEnabled {
+ val e = intercept[SparkIllegalArgumentException] {
+ sql("SET spark.sql.defaultPath = this is not a path")
+ }
+ assert(e.getCondition.startsWith("INVALID_CONF_VALUE"), e.getMessage)
+ }
+ }
+
+ test("DEFAULT_PATH conf: PATH disabled returns no fallback") {
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "false",
+ SQLConf.DEFAULT_PATH.key -> "system.session, system.builtin") {
+ val catalogManager = spark.sessionState.catalogManager
+ assert(catalogManager.sessionPathEntries.isEmpty,
+ "DEFAULT_PATH conf must not take effect when PATH is disabled")
+ }
+ }
+
+ // --- Path-driven security check (built on the lazy DEFAULT_PATH fallback)
---
+ // The "block temp function shadowing builtin" check is now driven by the
live PATH, so
+ // changes via SET PATH or DEFAULT_PATH take effect even when the legacy
order conf is
+ // left at its default.
+
+ test("path-driven security check: SET PATH putting session before builtin
blocks temp " +
+ "function with a builtin name") {
+ withPathEnabled {
+ val catalogManager = spark.sessionState.catalogManager
+ val priorSessionPath = catalogManager.storedSessionPathEntries
+ try {
+ // Default `sessionFunctionResolutionOrder` is "second" (builtin
first), but SET PATH
+ // overrides that to put session first. The security check must
reflect the live path.
+ sql("SET PATH = system.session, system.builtin")
+ val e = intercept[AnalysisException] {
+ sql("CREATE TEMPORARY FUNCTION count() RETURNS INT RETURN 1")
+ }
+ assert(e.getCondition == "ROUTINE_ALREADY_EXISTS", e.getMessage)
+ } finally {
+ sql("DROP TEMPORARY FUNCTION IF EXISTS session.count")
+ catalogManager.clearSessionPath()
+ priorSessionPath.foreach(catalogManager.setSessionPath)
+ }
+ }
+ }
+
+ test("path-driven security check: DEFAULT_PATH conf putting session before
builtin " +
+ "blocks temp function with a builtin name (no SET PATH issued)") {
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "true",
+ SQLConf.DEFAULT_PATH.key -> "system.session, system.builtin") {
+ val catalogManager = spark.sessionState.catalogManager
+ val priorSessionPath = catalogManager.storedSessionPathEntries
+ catalogManager.clearSessionPath()
+ try {
+ // Order conf is left at its default ("second"). The path-driven gate
must read
+ // DEFAULT_PATH and fire the security check for unqualified
temp/builtin collisions.
+ val e = intercept[AnalysisException] {
+ sql("CREATE TEMPORARY FUNCTION count() RETURNS INT RETURN 1")
+ }
+ assert(e.getCondition == "ROUTINE_ALREADY_EXISTS", e.getMessage)
+ } finally {
+ sql("DROP TEMPORARY FUNCTION IF EXISTS session.count")
+ catalogManager.clearSessionPath()
+ priorSessionPath.foreach(catalogManager.setSessionPath)
+ }
+ }
+ }
+
+ test("PATH enabled: SET PATH with only user schemas does not implicitly
resolve builtins") {
+ withPathEnabled {
+ sql("CREATE SCHEMA IF NOT EXISTS only_user_on_path")
+ try {
+ sql("SET PATH = spark_catalog.only_user_on_path")
+ val e = intercept[AnalysisException] {
+ sql("SELECT abs(-1)").collect()
+ }
+ assert(e.getCondition == "UNRESOLVED_ROUTINE", e.getMessage)
+ } finally {
+ sql("DROP SCHEMA IF EXISTS only_user_on_path")
+ }
+ }
+ }
+
+ test("PATH enabled: explicit SET PATH with system.session AFTER a user
catalog still " +
+ "reaches temp functions") {
+ // Explicit paths are honored as written: placing `system.session` after a
user catalog
+ // is the user's authorization for unqualified temp functions to resolve.
Contrast with
+ // the implicit (no SET PATH, no DEFAULT_PATH) form, which preserves the
security property
+ // of the seeded default path.
+ withPathEnabled {
+ sql("CREATE SCHEMA IF NOT EXISTS path_interleaved_user")
+ try {
+ sql("CREATE TEMPORARY FUNCTION path_interleaved_temp() RETURNS INT
RETURN 7")
+ try {
+ sql("SET PATH = system.builtin, spark_catalog.path_interleaved_user,
system.session")
+ checkAnswer(sql("SELECT path_interleaved_temp()"), Row(7))
+ } finally {
+ sql("DROP TEMPORARY FUNCTION IF EXISTS path_interleaved_temp")
+ }
+ } finally {
+ sql("DROP SCHEMA IF EXISTS path_interleaved_user")
+ }
+ }
+ }
+
+ test("PATH enabled: SET PATH with user schema before system.builtin still
resolves builtins") {
+ // Exercises the `leadingSystemFunctionKinds` else-branch in
CatalogManager: prefix before
+ // the first user catalog entry has no system entries, so kinds are taken
from the full path.
+ withPathEnabled {
+ sql("CREATE SCHEMA IF NOT EXISTS path_user_before_builtin")
+ try {
+ sql("SET PATH = spark_catalog.path_user_before_builtin,
system.builtin")
+ // `abs` is a builtin; if the else-branch did not surface Builtin in
the kinds,
+ // unqualified `abs(-1)` would fail with UNRESOLVED_ROUTINE.
Review Comment:
Same renaming nit as the comment above -- there is no "else-branch" anymore.
```suggestion
// `abs` is a builtin; if Builtin did not appear in the kinds list,
// unqualified `abs(-1)` would fail with UNRESOLVED_ROUTINE.
```
--
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]
