HyukjinKwon commented on code in PR #59184:
URL: https://github.com/apache/spark/pull/59184#discussion_r4265555094
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala:
##########
@@ -1644,24 +1644,15 @@ case class CreateFunction(
}
/**
- * The logical plan of the CREATE FUNCTION command for SQL Functions.
+ * A CREATE FUNCTION plan whose first child is the function identifier: an
+ * `UnresolvedIdentifier` before analysis and a `ResolvedIdentifier` after it.
Review Comment:
**Nit (P3):** This holds only for persistent functions. For `CREATE
TEMPORARY FUNCTION`, `SparkSqlParser` builds the command with
`ResolvedIdentifier(FakeSystemCatalog, Identifier.of(Array("session"),
funcName))` at parse time; the regenerated goldens show it as
`ResolvedIdentifier ...FakeSystemCatalog$@xxxxxxxx, session.<name>`. So a
temporary function never carries an `UnresolvedIdentifier`.
The current consumers are fine: `ApplyDefaultCollation` requires
`SupportsNamespaces`, and the builtin-name guard never applied to temporary
functions. But this doc is the contract future catalyst rules will follow. A
rule written from it could assume every CREATE FUNCTION passes through an
unresolved stage, or that a `ResolvedIdentifier` child always names an
analyzer-resolved catalog. Temporary functions would silently break both
assumptions. Maybe something like:
```scala
/**
* A CREATE FUNCTION plan whose first child is the function identifier.
Persistent functions
* start with an `UnresolvedIdentifier` that analysis resolves to a
`ResolvedIdentifier`;
* temporary functions are built by the parser with a `ResolvedIdentifier` on
* `FakeSystemCatalog` in the session namespace. Catalyst rules such as the
builtin-name check
* in `ResolveCatalogs` and `ApplyDefaultCollation` match on this trait,
because the concrete
* command lives in sql/core.
*/
```
**Verification:**
- **Inspection:** The trait Scaladoc describes both the persistent and the
temporary first-child shapes, uses only ASCII characters, and keeps lines
within 100 characters.
- **Compile:** The catalyst module still compiles and passes style checks
after the comment edit.
--
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]
