[SPARK-54807] Allow qualified names for built-in and session functions #53570
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Persistent functions now cached with unqualified keys for compatibility - Temporary functions use composite keys (session.funcName) - Both can coexist in the same registry - Views correctly exclude temp functions from resolution Issue: View test still failing - needs investigation into function builder resolution
- Persistent functions now stored with qualified keys (catalog.db.func) - Prevents conflicts when multiple databases have same function name - Temporary functions still use composite keys (session.func) Known issues: - View function resolution test still failing - Possible function listing regressions to investigate
Added extensive debug logging to understand why views capture wrong function class. Ready for detailed tracing.
**THE BUG:** In resolveBuiltinOrTempFunctionInternal, the 'isBuiltin' parameter was incorrectly checking if the temp/builtin identifier existed in the session registry, instead of checking the static FunctionRegistry.builtin. This caused lookupTempFuncWithViewContext to treat temp functions as builtins, bypassing view context checks and allowing temp functions created AFTER a view to incorrectly shadow the persistent function that the view should use. **THE FIX:** Changed the isBuiltin check to use FunctionRegistry.builtin.functionExists and TableFunctionRegistry.builtin.functionExists directly, matching master's behavior. **TEST RESULTS:** ✅ All 62 tests pass (PersistedViewTestSuite + FunctionQualificationSuite) ✅ SPARK-33692 view test now passes ✅ View correctly uses MyDoubleAvg and ignores temp MyDoubleSum
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Allow reference of built in functions with qualifiers builtin or system.builtin and temporary functions as session or system.session.
Why are the changes needed?
This extension allows users to disambiguate fucntion references and prepare for a following search path config.
Does this PR introduce any user-facing change?
Yes
How was this patch tested?
Added new tests
Was this patch authored or co-authored using generative AI tooling?
Yes: Claude Sonnet