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


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogUtils.scala:
##########
@@ -280,7 +281,15 @@ object CatalogUtils {
    * @return the URI of the path
    */
   def stringToURI(str: String): URI = {

Review Comment:
   Let's be more serious about breaking changes. Yes, we can ask the downstream 
applications to work around breaking changes, but this has cost and we should 
do it with caution.
   
   I don't think a refactor/code cleanup is justified for a breaking change 
(and there is no legacy config to restore). I suggest we revert it and use a 
more explicit way to perform the empty location check for places that need this 
check.
   
   cc @dongjoon-hyun @gatorsmile 



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