You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/06/10 18:30:38 UTC

[GitHub] [spark] dtenedor commented on a diff in pull request #36771: [SPARK-39383][SQL] Support DEFAULT columns in ALTER TABLE ADD COLUMNS to V2 data sources

dtenedor commented on code in PR #36771:
URL: https://github.com/apache/spark/pull/36771#discussion_r894798568


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ResolveDefaultColumnsUtil.scala:
##########
@@ -95,7 +95,7 @@ object ResolveDefaultColumns {
     if (SQLConf.get.enableDefaultColumns) {
       val allowedTableProviders: Array[String] =
         SQLConf.get.getConf(SQLConf.DEFAULT_COLUMN_ALLOWED_PROVIDERS)
-          .toLowerCase().split(",").map(_.trim)
+          .toLowerCase().split(",").map(_.trim) ++ Seq("")

Review Comment:
   Good question: we include the empty string here to allow the 'tableProvider' argument to be empty. This is only empty in tests, for example when using the InMemoryTableCatalog. In all other cases, the code requires a non-empty table provider to be given to this method call. I added this information to a comment here to help explain.
   



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