xianyinxin commented on a change in pull request #25115: [SPARK-28351][SQL] Support DELETE in DataSource V2 URL: https://github.com/apache/spark/pull/25115#discussion_r309584414
########## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/sources/filters.scala ########## @@ -36,10 +36,18 @@ abstract class Filter { */ def references: Array[String] + def sql: String + protected def findReferences(value: Any): Array[String] = value match { case f: Filter => f.references case _ => Array.empty } + + protected def quoteIdentifier(name: String): String = { Review comment: This code is borrowed from `org.apache.spark.sql.catalyst.util.quoteIdentifier` which is a package util, while `CatalogV2Implicits.quoted` is not a public util function. We'd better unify the two, I think. However, this code is introduced by the needs in the delete test case. Now the test code is updated according to your suggestion below, which left this function (sources.filter.sql) unused. I have removed this function in the latest code. If we need this function in future (like translating filters to sql string in jdbc), we then submit a new pr. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org