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