dimas-b commented on code in PR #1802:
URL: https://github.com/apache/polaris/pull/1802#discussion_r2124725360


##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java:
##########
@@ -80,114 +99,123 @@ public static String 
generateDeleteQueryForEntityPolicyMappingRecords(
     return generateDeleteQuery(ModelPolicyMappingRecord.class, queryParams);
   }
 
-  public static String generateSelectQueryWithEntityIds(
+  public static PreparedQuery generateSelectQueryWithEntityIds(
       @Nonnull String realmId, @Nonnull List<PolarisEntityId> entityIds) {
     if (entityIds.isEmpty()) {
       throw new IllegalArgumentException("Empty entity ids");
     }
-    StringBuilder condition = new StringBuilder("(catalog_id, id) IN (");
-    for (PolarisEntityId entityId : entityIds) {
-      String in = "(" + entityId.getCatalogId() + ", " + entityId.getId() + 
")";
-      condition.append(in);
-      condition.append(",");
+
+    String placeholders = entityIds.stream().map(e -> "(?, 
?)").collect(Collectors.joining(", "));
+    List<Object> params = new ArrayList<>();
+    for (PolarisEntityId id : entityIds) {
+      params.add(id.getCatalogId());
+      params.add(id.getId());
     }
-    // extra , removed
-    condition.deleteCharAt(condition.length() - 1);
-    condition.append(")");
-    condition.append(" AND realm_id = '").append(realmId).append("'");
+    params.add(realmId);
 
-    return generateSelectQuery(new ModelEntity(), " WHERE " + condition);
+    String where = " WHERE (catalog_id, id) IN (" + placeholders + ") AND 
realm_id = ?";
+    return new PreparedQuery(generateSelectQuery(new ModelEntity(), 
where).getSql(), params);
   }
 
-  public static <T> String generateInsertQuery(
+  public static <T> PreparedQuery generateInsertQuery(
       @Nonnull Converter<T> entity, @Nonnull String realmId) {
     String tableName = getTableName(entity.getClass());
     Map<String, Object> obj = entity.toMap();
     List<String> columnNames = new ArrayList<>(obj.keySet());
-    List<String> values =
-        new ArrayList<>(obj.values().stream().map(val -> "'" + val.toString() 
+ "'").toList());
+    List<Object> parameters = new ArrayList<>(obj.values());
+
     columnNames.add("realm_id");
-    values.add("'" + realmId + "'");
+    parameters.add(realmId);
 
     String columns = String.join(", ", columnNames);
-    String valuesString = String.join(", ", values);
+    String placeholders = columnNames.stream().map(c -> 
"?").collect(Collectors.joining(", "));
 
-    return "INSERT INTO " + tableName + " (" + columns + ") VALUES (" + 
valuesString + ")";
+    String sql = "INSERT INTO " + tableName + " (" + columns + ") VALUES (" + 
placeholders + ")";
+    return new PreparedQuery(sql, parameters);
   }
 
-  public static <T> String generateUpdateQuery(
+  public static <T> PreparedQuery generateUpdateQuery(
       @Nonnull Converter<T> entity, @Nonnull Map<String, Object> whereClause) {
     String tableName = getTableName(entity.getClass());
     Map<String, Object> obj = entity.toMap();
+
     List<String> setClauses = new ArrayList<>();
-    List<String> columnNames = new ArrayList<>(obj.keySet());
-    List<String> values = obj.values().stream().map(val -> "'" + 
val.toString() + "'").toList();
+    List<Object> parameters = new ArrayList<>();
 
-    for (int i = 0; i < columnNames.size(); i++) {
-      setClauses.add(columnNames.get(i) + " = " + values.get(i)); // 
Placeholders
+    for (Map.Entry<String, Object> entry : obj.entrySet()) {
+      setClauses.add(entry.getKey() + " = ?");
+      parameters.add(entry.getValue());
     }
 
-    String setClausesString = String.join(", ", setClauses);
+    List<String> whereConditions = new ArrayList<>();
+    for (Map.Entry<String, Object> entry : whereClause.entrySet()) {
+      whereConditions.add(entry.getKey() + " = ?");
+      parameters.add(entry.getValue());
+    }
+
+    String sql = "UPDATE " + tableName + " SET " + String.join(", ", 
setClauses);
+    if (!whereConditions.isEmpty()) {
+      sql += " WHERE " + String.join(" AND ", whereConditions);
+    }
 
-    return "UPDATE " + tableName + " SET " + setClausesString + 
generateWhereClause(whereClause);
+    return new PreparedQuery(sql, parameters);
   }
 
-  public static String generateDeleteQuery(
+  public static PreparedQuery generateDeleteQuery(
       @Nonnull Class<?> entityClass, @Nonnull Map<String, Object> whereClause) 
{

Review Comment:
   Perhaps SQL generation ought to be delegated to each entity class. This way 
the set of columns can be clearly linked to the compile-time java object fields.



-- 
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: issues-unsubscr...@polaris.apache.org

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

Reply via email to