dimas-b commented on code in PR #1802:
URL: https://github.com/apache/polaris/pull/1802#discussion_r2129637207
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java:
##########
@@ -21,193 +21,211 @@
import com.google.common.annotations.VisibleForTesting;
import jakarta.annotation.Nonnull;
import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
-import org.apache.polaris.core.entity.PolarisBaseEntity;
+import java.util.stream.Collectors;
import org.apache.polaris.core.entity.PolarisEntityCore;
import org.apache.polaris.core.entity.PolarisEntityId;
-import org.apache.polaris.core.entity.PolarisEntityType;
-import org.apache.polaris.core.policy.PolicyEntity;
-import org.apache.polaris.persistence.relational.jdbc.models.Converter;
import org.apache.polaris.persistence.relational.jdbc.models.ModelEntity;
import org.apache.polaris.persistence.relational.jdbc.models.ModelGrantRecord;
-import
org.apache.polaris.persistence.relational.jdbc.models.ModelPolicyMappingRecord;
-import
org.apache.polaris.persistence.relational.jdbc.models.ModelPrincipalAuthenticationData;
+/**
+ * Utility class to generate parameterized SQL queries (SELECT, INSERT,
UPDATE, DELETE). Ensures
+ * consistent SQL generation and protects against injection by managing
parameters separately.
+ */
public class QueryGenerator {
+ private final DatabaseType databaseType;
- public static <T> String generateSelectQuery(
- @Nonnull Converter<T> entity, @Nonnull Map<String, Object> whereClause) {
- return generateSelectQuery(entity, generateWhereClause(whereClause));
+ public QueryGenerator(DatabaseType databaseType) {
+ this.databaseType = databaseType;
}
- public static String generateDeleteQueryForEntityGrantRecords(
- @Nonnull PolarisEntityCore entity, @Nonnull String realmId) {
- String granteeCondition =
- String.format(
- "grantee_id = %s AND grantee_catalog_id = %s", entity.getId(),
entity.getCatalogId());
- String securableCondition =
- String.format(
- "securable_id = %s AND securable_catalog_id = %s",
- entity.getId(), entity.getCatalogId());
-
- String whereClause =
- " WHERE ("
- + granteeCondition
- + " OR "
- + securableCondition
- + ") AND realm_id = '"
- + realmId
- + "'";
- return generateDeleteQuery(ModelGrantRecord.class, whereClause);
+ /**
+ * @return The configured database type.
+ */
+ public DatabaseType getDatabaseType() {
+ return databaseType;
}
- public static String generateDeleteQueryForEntityPolicyMappingRecords(
- @Nonnull PolarisBaseEntity entity, @Nonnull String realmId) {
- Map<String, Object> queryParams = new HashMap<>();
- if (entity.getType() == PolarisEntityType.POLICY) {
- PolicyEntity policyEntity = PolicyEntity.of(entity);
- queryParams.put("policy_type_code", policyEntity.getPolicyTypeCode());
- queryParams.put("policy_catalog_id", policyEntity.getCatalogId());
- queryParams.put("policy_id", policyEntity.getId());
- } else {
- queryParams.put("target_catalog_id", entity.getCatalogId());
- queryParams.put("target_id", entity.getId());
- }
- queryParams.put("realm_id", realmId);
+ /** A container for the SQL string and the ordered parameter values. */
+ public record PreparedQuery(String sql, List<Object> parameters) {}
+
+ /** A container for the query fragment SQL string and the ordered parameter
values. */
+ record QueryFragment(String sql, List<Object> parameters) {}
+
+ /**
+ * Generates a SELECT query with projection and filtering.
+ *
+ * @param projections List of columns to retrieve.
+ * @param tableName Target table name.
+ * @param whereClause Column-value pairs used in WHERE filtering.
+ * @return A parameterized SELECT query.
+ * @throws IllegalArgumentException if any whereClause column isn't in
projections.
+ */
+ public PreparedQuery generateSelectQuery(
+ @Nonnull List<String> projections,
+ @Nonnull String tableName,
+ @Nonnull Map<String, Object> whereClause) {
+ checkInvalidColumns(projections, whereClause);
+ QueryFragment where = generateWhereClause(whereClause);
+ PreparedQuery query = generateSelectQuery(projections, tableName,
where.sql());
+ return new PreparedQuery(query.sql(), where.parameters());
+ }
- return generateDeleteQuery(ModelPolicyMappingRecord.class, queryParams);
+ /**
+ * Builds a DELETE query to remove grant records for a given entity.
+ *
+ * @param entity The target entity (either grantee or securable).
+ * @param realmId The associated realm.
+ * @return A DELETE query removing all grants for this entity.
+ */
+ public PreparedQuery generateDeleteQueryForEntityGrantRecords(
+ @Nonnull PolarisEntityCore entity, @Nonnull String realmId) {
+ String where =
+ " WHERE ((grantee_id = ? AND grantee_catalog_id = ?) OR (securable_id
= ? AND securable_catalog_id = ?)) AND realm_id = ?";
Review Comment:
nit: maybe use multi-line `"""` strings with shorter lines for ease of
reading (in IDE)?
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/models/ModelEntity.java:
##########
@@ -178,8 +200,13 @@ public Map<String, Object> toMap() {
map.put("purge_timestamp", this.getPurgeTimestamp());
map.put("to_purge_timestamp", this.getToPurgeTimestamp());
map.put("last_update_timestamp", this.getLastUpdateTimestamp());
- map.put("properties", this.getProperties());
- map.put("internal_properties", this.getInternalProperties());
+ if (databaseType.equals(DatabaseType.POSTGRES)) {
+ map.put("properties", toJsonbPGobject(this.getProperties()));
Review Comment:
suggestion: use an interface for converting values to driver-specific types
and provide different implementations per `DatabaseType` (it could even be
obtained from `DatabaseType.converter()` perhaps) instead of `if`s.
--
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]
