Copilot commented on code in PR #9095:
URL: https://github.com/apache/seatunnel/pull/9095#discussion_r2030363246


##########
seatunnel-connectors-v2/connector-iceberg/src/main/java/org/apache/seatunnel/connectors/seatunnel/iceberg/utils/ExpressionUtils.java:
##########
@@ -66,6 +68,12 @@ public class ExpressionUtils {
                     .append(ISO_LOCAL_TIME)
                     .toFormatter();
 
+    public static Expression parseWhereClauseToIcebergExpression(String 
whereClauseStr)

Review Comment:
   [nitpick] The method prepends 'DELETE FROM t WHERE ' to the provided filter 
clause; it would be beneficial to add a clarifying comment or consider a more 
direct parsing approach to handle where clauses.



##########
seatunnel-connectors-v2/connector-iceberg/src/main/java/org/apache/seatunnel/connectors/seatunnel/iceberg/source/enumerator/scan/IcebergScanContext.java:
##########
@@ -73,13 +79,27 @@ public static IcebergScanContext scanContext(
                 .useSnapshotTimestamp(tableConfig.getUseSnapshotTimestamp())
                 .caseSensitive(sourceConfig.isCaseSensitive())
                 .schema(schema)
-                .filter(tableConfig.getFilter())
+                .filter(getFilter(tableConfig.getFilter()))
                 .splitSize(tableConfig.getSplitSize())
                 .splitLookback(tableConfig.getSplitLookback())
                 .splitOpenFileCost(tableConfig.getSplitOpenFileCost())
                 .build();
     }
 
+    private static Expression getFilter(String whereClause) {
+        if (StringUtils.isNotBlank(whereClause)) {
+            try {
+                Expression expression =
+                        
ExpressionUtils.parseWhereClauseToIcebergExpression(whereClause);
+                return expression;
+            } catch (JSQLParserException e) {
+                log.warning(
+                        "Failed to parse where clause to iceberg expression: " 
+ e.getMessage());
+            }

Review Comment:
   Consider propagating the JSQLParserException for invalid filter clauses 
instead of defaulting to an always true expression; silently falling back may 
hide configuration errors.
   ```suggestion
       private static Expression getFilter(String whereClause) throws 
JSQLParserException {
           if (StringUtils.isNotBlank(whereClause)) {
               Expression expression =
                       
ExpressionUtils.parseWhereClauseToIcebergExpression(whereClause);
               return expression;
   ```



-- 
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: commits-unsubscr...@seatunnel.apache.org

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

Reply via email to