cloud-fan commented on PR #53570:
URL: https://github.com/apache/spark/pull/53570#issuecomment-3982002613

   I don't think we need `EXTENSION_NAMESPACE`. The purpose of 
`SparkSessionExtensions` is to allow power users to overwrite built-in 
functions — they register functions with a `FunctionIdentifier` they fully 
control, and those functions simply replace the builtin at that key. There's no 
need for a separate namespace to distinguish them.
   
   A few concrete issues with the current `Extension` kind:
   
   1. **`extensionFunctionIdentifier(name)` produces 
`FunctionIdentifier(format(name))` — identical to builtins.** Extensions are 
stored at the exact same key as builtins, so the `Extension` kind has no 
storage-level distinction.
   
   2. **`SparkSessionExtensions.registerFunctions` allows users to provide 
arbitrary `FunctionIdentifier` with database/catalog.** The PR silently strips 
qualification for unqualified extensions and keeps it as-is for qualified ones, 
but qualified extensions can never be reached via `extension.func` or 
`system.extension.func` since those only look up unqualified keys. The 
abstraction is incomplete.
   
   3. **Every code path treats `Extension` and `Builtin` identically** — 
`resolveScalarFunction`, `resolveTableFunction`, `lookupFunctionType`, 
`ResolveCatalogs`, etc. all collapse them together. This adds branches and 
complexity with no behavioral difference.
   
   Suggestion: remove `Extension` from `SessionFunctionKind`, remove 
`EXTENSION_NAMESPACE` from `CatalogManager`, and let extension functions 
continue to work as they do on master — they overwrite builtins at injection 
time, no special namespace needed.
   
   ---
   _This comment was generated with [GitHub MCP](http://go/mcps)._


-- 
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]

Reply via email to