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


##########
sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala:
##########
@@ -134,9 +132,6 @@ class ResolveSessionCatalog(val catalogManager: 
CatalogManager)
       AlterDatabasePropertiesCommand(db, properties)
 
     case SetNamespaceLocation(DatabaseInSessionCatalog(db), location) if 
conf.useV1Command =>
-      if (StringUtils.isEmpty(location)) {

Review Comment:
   I'm not very comfortable relying on external library's implementation. Can't 
we do an explicit empty/null check at the Spark side? Just like the old code 
did.



##########
sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala:
##########
@@ -134,9 +132,6 @@ class ResolveSessionCatalog(val catalogManager: 
CatalogManager)
       AlterDatabasePropertiesCommand(db, properties)
 
     case SetNamespaceLocation(DatabaseInSessionCatalog(db), location) if 
conf.useV1Command =>
-      if (StringUtils.isEmpty(location)) {

Review Comment:
   I'm not very comfortable relying on external library's implementation. Can't 
we do an explicit empty/null check at the Spark side? Just like the previous 
code did.



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