cloud-fan commented on code in PR #48546:
URL: https://github.com/apache/spark/pull/48546#discussion_r1839230145


##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationFactory.java:
##########
@@ -1180,9 +1180,33 @@ public static StringSearch getStringSearch(
    * Returns the collation ID for the given collation name.
    */
   public static int collationNameToId(String collationName) throws 
SparkException {

Review Comment:
   I don't think this is the right place to resolve the qualified collation 
name. It should be the analyzer to look up the collation from a catalog.
   
   `CollationFactor` should only handle the built-in collations and it doesn't 
need to care about the qualified collation name.
   
   Let's create a framework for collation resolution. Some ideas:
   - We add a new expression `case class UnresolvedCollation(name: Seq[String])`
   - The expression `Collate` should have a child expression to hold the 
`UnresolvedCollation`, instead of just `collationName: String`
   - We add a new analyzer rule to resolve `UnresolvedCollation` to `case class 
ResolvedCollation(name: String)`. This is a temporary solution as we have not 
introduced user-defied collation yet. `ResolvedCollation` always contains the 
name of a built-in collation for now.



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