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