[SPARK-56750] default path config

[srielau]

What changes were proposed in this pull request?

This PR implements SPARK-56750 by adding a default SQL PATH (parallel to the default catalog) and wiring path-driven semantics through function and namespace resolution when spark.sql.path.enabled is true.

Core feature

• New config spark.sql.defaultPath (SQLConf.DEFAULT_PATH):

  • String session conf (default empty), accepts the full SET PATH grammar.
  • When PATH is enabled and no explicit SET PATH was issued, this value becomes the effective SQL PATH (mirroring how currentCatalog falls back to spark.sql.defaultCatalog).
  • SET PATH = DEFAULT_PATH expands to this configured default.
  • An inner DEFAULT_PATH token inside the conf value resolves to the Spark built-in default ordering (cycle-safe expansion).
  • The conf is validated for syntax at set time; redundant entries are tolerated at lookup time (first-match resolution makes them harmless).

• Shared SET PATH grammar: A new singlePathElementList rule and parsePathElements parser entry let spark.sql.defaultPath reuse the exact SET PATH grammar without re-implementation.

• CatalogManager PATH materialization: A single sessionPathEntries resolves stored SET PATH, parsed DEFAULT_PATH, and the seeded defaultPathOrder fallback into one effective path. CURRENT_PATH() and unqualified function/relation resolution all read from this single source of truth.

Path-driven function resolution

• Fast-path kinds derived from the live PATH: SessionCatalog's unqualified-lookup fast path receives an ordered list of system.builtin / system.session kinds from the effective PATH (in path order) via a path-driven provider. Replaces the previous reliance on spark.sql.functionResolution.sessionOrder (which is now used solely as a seed for defaultPathOrder and may eventually be retired).

• The path is the path: current_path is treated like current_catalog and current_schema — we don't distinguish whether it was set via SET PATH, via the spark.sql.defaultPath conf, or by the seeded default. Whatever is on the path is searched as written. Placing system.session after a user catalog is the user's authorization for unqualified temp functions to resolve through that position.

• Security check via path order: Creating a temp function with a builtin's name is blocked when system.session is searched before system.builtin. The "no injection of temp over persistent" property is enforced by path order itself: when a user catalog precedes system.session, persistent resolution wins for unqualified names.

• COUNT(*) → COUNT(1) rewrite gate: The analyzer rewrite skips when a temp count would shadow the builtin under the live PATH. Shares the same canonical predicate (FunctionResolution.isSessionBeforeBuiltinInPath) as the security check.

Layering

• Resolution-order logic lives at the Strategy layer (FunctionResolution), not Coordination (CatalogManager). CatalogManager provides path materialization (sqlResolutionPathEntries) and pure data helpers (systemFunctionKindsFromPath) — it doesn't decide resolution order. SessionCatalog's default kinds provider derives from conf.systemPathOrder (the seeded default path) and never branches on the legacy resolution-order conf.

Deadlock-safe SessionCatalog lookups

• lookupBuiltinOrTempTableFunction and lookupFunctionInfo are intentionally not synchronized on SessionCatalog. The path-driven kinds provider may acquire CatalogManager.synchronized; another thread holding that lock can call into SessionCatalog (e.g. via setCurrentNamespace), so synchronizing the public lookup methods would introduce lock-order inversion. Tighter synchronized blocks are kept around the temp-view-context post-processing where they're actually needed.

Duplicate validation

• SET PATH rejects static duplicates (literal-vs-literal, repeated current_schema / current_database) at statement time as a typo guard.
• A literal that happens to match the live current_schema is not flagged: the duplicate is USE SCHEMA-state-dependent and harmless under first-match resolution.
• spark.sql.defaultPath runs no semantic duplicate check at lookup time — the previous lookup-time validate could wedge a session by throwing on every unqualified resolution after USE SCHEMA collided a literal with current_schema.

DROP TEMPORARY FUNCTION correctness

• Unqualified DROP TEMPORARY FUNCTION <builtin> without IF EXISTS raises FORBIDDEN_OPERATION against the builtin name as captured.
• IF EXISTS remains a no-op when no temp function exists.
• Qualified temporary namespaces (session.foo, system.session.foo) always target the temp registry.

PathElement AST

• New PathElement AST in catalyst (org.apache.spark.sql.connector.catalog.PathElement, private[sql]) with expand and validateNoStaticDuplicates helpers shared between SetPathCommand and CatalogManager.confDefaultPathEntries.

Performance

• CatalogManager.confDefaultPathEntries caches expanded entries by (trimmed conf value, sessionFunctionResolutionOrder); on conf-stable sessions the lookup hot path is one AtomicReference.get. CurrentSchemaEntry markers are preserved unresolved so the cache stays valid across USE SCHEMA.

Tests

• SetPathSuite (47 tests) covers: lazy fallback, explicit override, SET PATH = DEFAULT_PATH, cycle break, invalid value, PATH disabled, path-driven security, user-only path, duplicate tolerance for DEFAULT_PATH, static duplicate rejection at SET PATH, the explicit-interleaved path case (system.builtin, <user>, system.session resolving a temp), and the leading-system-kinds else-branch.
• FunctionQualificationSuite and RelationQualificationSuite continue to pass unchanged (130/130 across the three suites).

Why are the changes needed?

Users need an OSS way to define a default SQL PATH before any explicit SET PATH, mirroring how spark.sql.defaultCatalog provides a session default for the current catalog. The change also fixes a semantic gap where path-driven function-resolution shortcuts (the COUNT(*) rewrite gate, the temp-vs-builtin security check) ignored SET PATH and only consulted the legacy spark.sql.functionResolution.sessionOrder proxy — so SET PATH = system.session, system.builtin, ... did not block creating a temp count until the order conf was also flipped.

Does this PR introduce any user-facing change?

Yes.

• New conf: spark.sql.defaultPath.
• With spark.sql.path.enabled=true, sessions without explicit SET PATH resolve using the configured default path.
• SET PATH = DEFAULT_PATH and CURRENT_PATH() reflect the configured default.
• Function resolution order for unqualified names follows the effective PATH (post-SET PATH, with DEFAULT_PATH and defaultPathOrder fallbacks). The temp-vs-builtin security check and the COUNT(*) → COUNT(1) rewrite gate now reflect SET PATH changes.
• Subtle behavior change in the previously-impossible "temp-only function in sessionFunctionResolutionOrder=last" case: the unqualified call now resolves to the temp instead of UNRESOLVED_ROUTINE. This matches the documented intent of "builtin → persistent → session". The "no temp injection over persistent" property is preserved by path order.

How was this patch tested?

• build/sbt "sql/testOnly org.apache.spark.sql.SetPathSuite"
• build/sbt "sql/testOnly *FunctionQualificationSuite *RelationQualificationSuite"
• build/sbt "sql/testOnly *DDLSuite -- -z \"drop built-in function\""
• build/sbt "catalyst/Compile/compile" "sql/Compile/compile" "sql/Test/compile"

Was this patch authored or co-authored using generative AI tooling?

Yes. Some implementation and tests were iterated with coding-assistant tooling; the author reviewed and owns the final patch.

[srielau]

(stale debug review — see the AI-assisted self-review further down for the actual content)

[srielau]

(stale debug review — see the AI-assisted self-review further down for the actual content)

[srielau]

(stale debug review — see the AI-assisted self-review further down for the actual content)

[srielau]

(stale debug review — see the AI-assisted self-review further down for the actual content)

[srielau]

(stale debug review — see the AI-assisted self-review further down for the actual content)

[srielau]

AI-assisted self-review. The high-level direction (mirror spark.sql.defaultCatalog for PATH; align routine-resolution shortcuts with the live PATH) is sound. Findings below are mostly correctness/perf concerns and a few naming/docs nits — I've flagged the deadlock + test-default + last-mode behavior change as the higher-priority items.

Summary

• Prior state: SET PATH exists, but with no admin-side default — every session must SET PATH itself, and SET PATH = DEFAULT_PATH only resolves to a fixed defaultPathOrder. Function-resolution shortcuts (1-part lookup, the security gate that blocks temp-shadowing-builtin, the count(*)→count(1) gate) read spark.sql.legacy.sessionFunctionResolutionOrder directly rather than the live PATH, so a SET PATH = system.session, system.builtin, ... doesn't actually flip those gates.
• Design approach: new spark.sql.defaultPath mirrors spark.sql.defaultCatalog — session-level conf, accepts the full SET PATH grammar via a reused entry rule (singlePathElementList + parseSqlPathElements), with an isWorkspaceDefaultExpansion cycle break for inner DEFAULT_PATH tokens. Routine-resolution shortcuts now read CatalogManager.leadingSystemFunctionKinds / isSessionBeforeBuiltinInPath, both derived from the effective PATH.
• Key decisions: (1) parse the conf on every read, no caching (see A3); (2) path-derive the system-kinds shortcut order rather than keep the legacy switch (see A4 for the "last" mode consequence); (3) wire kinds via a var callback set from the CatalogManager constructor body (see A1/A2); (4) validate SchemaInPath arity at expansion time, not parse time (see A8).
• Implementation sketch: parser adds singlePathElementList + parseSqlPathElements; AST PathElement moves into catalyst (connector.catalog) so CatalogManager can call PathElement.expand from its conf-fallback path while SetPathCommand still calls it from run(); CatalogManager grows workspaceDefaultPathEntries, leadingSystemFunctionKinds, isSessionBeforeBuiltinInPath; SessionCatalog exposes sessionFunctionKindsProvider (var) wired post-construction.

Inline comments are split across this review and the next (was hitting an API issue with a single 12-comment payload).

[srielau]

(continuation of the AI-assisted self-review — see the parent review above for the full summary; this entry hosts inline comments 7-12 because the GitHub API rejected the 12-comment payload as a single review)

[srielau]

Review follow-up (SPARK-56750 / #55717)

Thanks for the detailed review — here is how the latest commits address it.

Correctness / threading

• Deadlock (SC ⇄ CM): Removed synchronized from SessionCatalog.lookupBuiltinOrTempTableFunction so the temp metadata path does not hold SessionCatalog while invoking sessionFunctionKindsProvider → CatalogManager (re-entrant synchronized on CM is still possible; the AB-BA lock order with setCurrentNamespace is avoided).
• sessionFunctionKindsProvider default: Restored the SESSION_FUNCTION_RESOLUTION_ORDER-based default when no CatalogManager wiring is present (direct SessionCatalog in tests).
• @volatile on the provider var for safe publication after CatalogManager ctor wiring.

PATH → builtin/session “shortcut” semantics

• \"last\" mode / DESCRIBE / quick-check: System kinds are taken from the prefix before the first user-catalog entry when that prefix contains any system.* entries (legacy builtin-only shortcut preserved).
• If that prefix has no system entries but the full path does (e.g. user schema before system.builtin), kinds come from the full path (no magic default list).
• Removed the implicit Seq(Builtin, Temp) fallback when no system segments match; added a test that a user-only PATH does not resolve unqualified builtins (UNRESOLVED_ROUTINE for abs).
• isSystemBuiltinPathEntry / isSystemSessionPathEntry: Now case-insensitive so stored paths like System.Builtin still classify as system entries (fixes empty kinds + current_path() for case-preserved literals).

Performance / naming / validation

• spark.sql.defaultPath: Renamed workspace* → confDefaultPath* / isConfDefaultExpansion; parseSqlPathElements → parsePathElements on AbstractSqlParser.
• Parse cache: Cached parsed PathElement list by conf string (ANTLR only); expand + duplicate validation still run on each materialization so CURRENT_SCHEMA / live catalog stay correct.
• Duplicates: Shared PathElement.validateNoDuplicateResolvedPath for SET PATH and conf default materialization (same rules as command run).
• Single-part schema in PATH: Validated in AstBuilder.visitPathElement (fail at parse/analysis with path position), not on every expand.
• DEFAULT_PATH: .checkValue parses via parsePathElements; doc string fixed (no broken [[...]] in user-facing text; references spark.sql.functionResolution.sessionOrder by key).

DROP TEMPORARY FUNCTION IF EXISTS session.count (cleanup in tests)

• This was not the analyzer binding session.count to the builtin. DropFunctionCommand stripped to count and then threw cannotDropBuiltinFuncError when no temp existed but a builtin collided — before calling dropTempFunction, which made IF EXISTS cleanup fail incorrectly.
• Fix: Removed that guard; dropTempFunction only touches the temp registry and already honors ignoreIfNotExists. Tests use SQL DROP TEMPORARY FUNCTION IF EXISTS session.count again.

Non-action / follow-up

• “Coordination vs strategy” note on where leadingSystemFunctionKinds lives: acknowledged; left as-is for this PR to avoid a larger refactor (happy to move toward FunctionResolution in a follow-up if reviewers want that split explicitly).

If anything above should be tweaked before merge, call it out inline and I will adjust.

[srielau]

Review (SPARK-56750 / default PATH conf)

Overall

Strong direction: spark.sql.defaultPath mirrors default-catalog style admin defaults; shared PATH grammar for conf vs SET PATH avoids drift; DEFAULT_PATH cycle break, duplicate validation shared with SET PATH, and case-insensitive system.* path entries are all solid.

Wiring unqualified function shortcuts (COUNT rewrite, temp-vs-builtin security) to the live effective PATH via CatalogManager fixes a real semantic gap vs spark.sql.functionResolution.sessionOrder alone.

Deadlock / locking

Removing synchronized from the outer lookupBuiltinOrTempTableFunction / lookupFunctionInfo while keeping narrower sync for temp/view-context paths is a reasonable fix for SessionCatalog ⇄ CatalogManager lock order. Follow-up suggestion: a one-line Scaladoc note that a single lookup is best-effort consistent if PATH or temp registry changes mid-flight—sets expectations for reviewers and future readers.

Other follow-ups (non-blocking)

• confDefaultPathElementCache: confirm any path that mutates spark.sql.defaultPath without a full catalog reset still invalidates or replaces the cache as needed (session SET likely OK).
• SQLConf.DEFAULT_PATH doc: double-check the empty-default story matches code (defaultPathOrder / sessionFunctionResolutionOrder vs sessionPathEntries when PATH is on and both stored path and conf are empty).
• MiMa / API: if PathElement moved packages, note any connector/binary compatibility expectations (if it was previously only private[sql], probably fine).

DDL fix

Scoping DROP TEMPORARY FUNCTION builtin collision to unqualified + !ifExists and delegating to dropTempFunction for IF EXISTS matches the described bug and looks correct.

Tests

SetPathSuite coverage for lazy fallback, overrides, SET PATH = DEFAULT_PATH, cycle break, invalid conf, PATH off, PATH-driven security, user-only PATH (no implicit builtins), and duplicates is appropriate.

No blockers from this pass; approve with minor doc/concurrency nits above if useful.