HyukjinKwon commented on code in PR #59184:
URL: https://github.com/apache/spark/pull/59184#discussion_r4262013906


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala:
##########
@@ -1644,24 +1644,11 @@ case class CreateFunction(
 }
 
 /**
- * The logical plan of the CREATE FUNCTION command for SQL Functions.
+ * Used to apply ApplyDefaultCollation to CreateUserDefinedFunctionCommand

Review Comment:
   **Nit (P3):** This Scaladoc names only the `ApplyDefaultCollation` hook. The 
trait also carries a structural contract that other rules depend on:
   
   - `ResolveCatalogs` reads `children.head` as the function's 
`UnresolvedIdentifier` for the builtin-name guard.
   - `ApplyDefaultCollation` matches `children.headOption` against a 
`ResolvedIdentifier`.
   
   Neither member makes that contract explicit. A future implementer could 
reasonably put the identifier somewhere other than the first child, and the 
guard and collation defaulting would then stop applying with no error. Could 
the doc state why the trait exists and what it requires? For example:
   
   ```scala
   /**
    * A CREATE FUNCTION plan whose first child is the function identifier: an
    * `UnresolvedIdentifier` before analysis and a `ResolvedIdentifier` after 
it.
    * 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 builtin-name guard 
and the default-collation consumers and the first-child identifier contract, 
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]

Reply via email to