Re: [PR] [SPARK-49002][SQL] Consistently handle invalid locations in WAREHOUSE/SCHEMA/TABLE/PARTITION/DIRECTORY [spark]

["cloud-fan (via GitHub)" <gi...@apache.org>]

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


##########
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:
   This caused a regression in our environment, because it's a low-level API that is used in many places. The problematic one is in `HiveClientImpl` where we get the schema/table location string from HMS. We shouldn't perform the empty string check if that malformed location string is already in HMS. Spark should still be able to get it and display it in places like `DESCRIBE` command.
   
   I think it's better to perform validations explicitly, instead of in a API like `stringToURI` which should simply perform the conversion.
   
   Shall we revert this change and revisit it?  @yaooqinn 



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org