singhpk234 commented on code in PR #1802:
URL: https://github.com/apache/polaris/pull/1802#discussion_r2124951589


##########
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:
   I added an intersection check in case we take the map key as arg, ideally 
since this is being constructed from the object themselves i didn't add the 
check, but i think it safe to do the intersection check added that



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