huaxingao commented on code in PR #3584:
URL: https://github.com/apache/polaris/pull/3584#discussion_r2776445611


##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java:
##########
@@ -192,6 +192,65 @@ public static PreparedQuery generateUpdateQuery(
     return new PreparedQuery(sql, bindingParams);
   }
 
+  /**
+   * Builds an UPDATE query that updates only the specified columns and 
supports richer WHERE
+   * predicates (equality, greater-than, less-than, IS NULL, IS NOT NULL).
+   *
+   * <p>Callers should prefer passing an ordered map (e.g. {@link 
java.util.LinkedHashMap}) for the
+   * set clause so generated SQL and parameter order are stable.

Review Comment:
   Fixed. Thanks for the suggestion!



##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java:
##########
@@ -209,6 +268,32 @@ public static PreparedQuery generateDeleteQuery(
         "DELETE FROM " + getFullyQualifiedTableName(tableName) + where.sql(), 
where.parameters());
   }
 
+  /**
+   * Builds a DELETE query that supports richer WHERE predicates (equality, 
greater-than, less-than,
+   * IS NULL, IS NOT NULL).
+   */
+  public static PreparedQuery generateDeleteQuery(
+      @Nonnull List<String> tableColumns,
+      @Nonnull String tableName,
+      @Nonnull Map<String, Object> whereEquals,
+      @Nonnull Map<String, Object> whereGreater,
+      @Nonnull Map<String, Object> whereLess,
+      @Nonnull Set<String> whereIsNull,
+      @Nonnull Set<String> whereIsNotNull) {
+    Set<String> columns = new HashSet<>(tableColumns);
+    validateColumns(columns, whereEquals.keySet());
+    validateColumns(columns, whereGreater.keySet());
+    validateColumns(columns, whereLess.keySet());
+    validateColumns(columns, whereIsNull);
+    validateColumns(columns, whereIsNotNull);

Review Comment:
   You are right. Removed. 



##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java:
##########
@@ -192,6 +192,65 @@ public static PreparedQuery generateUpdateQuery(
     return new PreparedQuery(sql, bindingParams);
   }
 
+  /**
+   * Builds an UPDATE query that updates only the specified columns and 
supports richer WHERE
+   * predicates (equality, greater-than, less-than, IS NULL, IS NOT NULL).
+   *
+   * <p>Callers should prefer passing an ordered map (e.g. {@link 
java.util.LinkedHashMap}) for the
+   * set clause so generated SQL and parameter order are stable.
+   *
+   * @param tableColumns List of valid table columns.
+   * @param tableName Target table.
+   * @param setClause Column-value pairs to update.
+   * @param whereEquals Column-value pairs used in WHERE equality filtering.
+   * @param whereGreater Column-value pairs used in WHERE greater-than 
filtering.
+   * @param whereLess Column-value pairs used in WHERE less-than filtering.
+   * @param whereIsNull Columns that must be NULL.
+   * @param whereIsNotNull Columns that must be NOT NULL.
+   * @return UPDATE query with parameter bindings.
+   */
+  public static PreparedQuery generateUpdateQuery(
+      @Nonnull List<String> tableColumns,
+      @Nonnull String tableName,
+      @Nonnull Map<String, Object> setClause,
+      @Nonnull Map<String, Object> whereEquals,
+      @Nonnull Map<String, Object> whereGreater,
+      @Nonnull Map<String, Object> whereLess,
+      @Nonnull Set<String> whereIsNull,
+      @Nonnull Set<String> whereIsNotNull) {
+    if (setClause.isEmpty()) {
+      throw new IllegalArgumentException("Empty setClause");
+    }
+
+    Set<String> columns = new HashSet<>(tableColumns);
+    validateColumns(columns, setClause.keySet());
+    validateColumns(columns, whereEquals.keySet());
+    validateColumns(columns, whereGreater.keySet());
+    validateColumns(columns, whereLess.keySet());
+    validateColumns(columns, whereIsNull);
+    validateColumns(columns, whereIsNotNull);

Review Comment:
   Removed



##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java:
##########
@@ -231,22 +316,55 @@ static QueryFragment generateWhereClause(
       @Nonnull Set<String> tableColumns,
       @Nonnull Map<String, Object> whereEquals,
       @Nonnull Map<String, Object> whereGreater) {
+    return generateWhereClauseExtended(
+        tableColumns, whereEquals, whereGreater, Map.of(), Set.of(), Set.of());
+  }
+
+  private static void validateColumns(
+      @Nonnull Set<String> tableColumns, @Nonnull Set<String> columns) {
+    for (String column : columns) {
+      if (!tableColumns.contains(column) && !column.equals("realm_id")) {
+        throw new IllegalArgumentException("Invalid query column: " + column);
+      }
+    }
+  }
+
+  @VisibleForTesting
+  static QueryFragment generateWhereClauseExtended(
+      @Nonnull Set<String> tableColumns,
+      @Nonnull Map<String, Object> whereEquals,
+      @Nonnull Map<String, Object> whereGreater,
+      @Nonnull Map<String, Object> whereLess,
+      @Nonnull Set<String> whereIsNull,
+      @Nonnull Set<String> whereIsNotNull) {
+    // Preserve the original behavior of rejecting unknown columns. This is 
used by SELECT query
+    // generation too, not only by callers of the extended UPDATE/DELETE 
helpers.

Review Comment:
   Removed.



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

Reply via email to