MaxGekk commented on a change in pull request #31475:
URL: https://github.com/apache/spark/pull/31475#discussion_r577632407



##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsDelete.java
##########
@@ -68,4 +69,15 @@ default boolean canDeleteWhere(Filter[] filters) {
    * @throws IllegalArgumentException If the delete is rejected due to 
required effort
    */
   void deleteWhere(Filter[] filters);
+
+  Filter[] ALWAYS_TRUE_FILTER = new Filter[] { new AlwaysTrue() };

Review comment:
       I wouldn't do that because of:
   - I believe interfaces should be independent from internals as much as 
possible.
   - ALWAYS_TRUE_FILTER can be used in other methods of the interface like 
`deleteWhere()`. For example, when an implementation overrides 
`truncateTable()` but an user want to delete all rows via `deleteWhere()`, so, 
he/she can re-use the constant.
   - Other interfaces have constants too, for example 
https://github.com/apache/spark/blob/ec1560af251d2c3580f5bccfabc750f1c7af09df/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableCatalog.java#L47
 . I don't see much difference between `PROP_LOCATION` and  
`ALWAYS_TRUE_FILTER`.




----------------------------------------------------------------
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to