eric-maynard commented on code in PR #1824:
URL: https://github.com/apache/polaris/pull/1824#discussion_r2138490854


##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java:
##########
@@ -154,15 +151,12 @@ private void persistEntity(
       Connection connection,
       QueryAction queryAction)
       throws SQLException {
-    ModelEntity modelEntity = ModelEntity.fromEntity(entity);
+    ModelEntity modelEntity = ModelEntity.fromEntity(entity, realmId);

Review Comment:
   nit: Normally we pass in realmId first



##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/DatasourceOperations.java:
##########
@@ -54,8 +54,8 @@ public class DatasourceOperations {
   private static final String SERIALIZATION_FAILURE_SQL_CODE = "40001";
 
   private final DataSource datasource;
-  private final RelationalJdbcConfiguration relationalJdbcConfiguration;
   private final DatabaseType databaseType;
+  private final RelationalJdbcConfiguration relationalJdbcConfiguration;

Review Comment:
   nit: spurious change?



##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/SQLConstants.java:
##########
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.persistence.relational.jdbc;
+
+import java.util.stream.Collectors;
+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;
+
+public class SQLConstants {
+
+  public static final String SCHEMA = "POLARIS_SCHEMA";
+
+  public static final String ENTITY_LOOKUP_BY_CATALOG_ID_ID =

Review Comment:
   Can we name these a little different? I was confused by the `ENTITY_` 
prefix. Maybe something like
   `QUERY_TEMPLATE_LOOKUP_BY_CATALOG_ID`?



##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/models/ModelGrantRecord.java:
##########
@@ -29,14 +29,25 @@
 public class ModelGrantRecord implements Converter<PolarisGrantRecord> {
   public static final String TABLE_NAME = "GRANT_RECORDS";
 
+  public static final List<String> PK_COLUMNS =
+      List.of(
+          "realm_id",
+          "securable_catalog_id",
+          "securable_id",
+          "grantee_catalog_id",
+          "grantee_id",
+          "privilege_code");
+
   public static final List<String> ALL_COLUMNS =
       List.of(
+          "realm_id",
           "securable_catalog_id",
           "securable_id",
           "grantee_catalog_id",
           "grantee_id",
           "privilege_code");

Review Comment:
   Can we add some check that PK_COLUMNS are a subset of ALL_COLUMNS?



##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/models/ModelGrantRecord.java:
##########
@@ -29,14 +29,25 @@
 public class ModelGrantRecord implements Converter<PolarisGrantRecord> {
   public static final String TABLE_NAME = "GRANT_RECORDS";
 
+  public static final List<String> PK_COLUMNS =
+      List.of(
+          "realm_id",
+          "securable_catalog_id",
+          "securable_id",
+          "grantee_catalog_id",
+          "grantee_id",
+          "privilege_code");
+
   public static final List<String> ALL_COLUMNS =
       List.of(
+          "realm_id",
           "securable_catalog_id",
           "securable_id",
           "grantee_catalog_id",
           "grantee_id",
           "privilege_code");

Review Comment:
   To here and other classes



##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/SQLConstants.java:
##########
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.persistence.relational.jdbc;
+
+import java.util.stream.Collectors;
+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;
+
+public class SQLConstants {
+
+  public static final String SCHEMA = "POLARIS_SCHEMA";
+
+  public static final String ENTITY_LOOKUP_BY_CATALOG_ID_ID =
+      "SELECT "
+          + String.join(", ", ModelEntity.ALL_COLUMNS)
+          + " FROM "
+          + SCHEMA
+          + "."
+          + ModelEntity.TABLE_NAME
+          + " WHERE catalog_id = ? AND id = ? AND realm_id = ?";
+
+  public static final String ENTITY_LOOKUP_BY_CATALOG_ID_ID_TYPE_CODE =
+      "SELECT "
+          + String.join(", ", ModelEntity.ALL_COLUMNS)
+          + " FROM "
+          + SCHEMA
+          + "."
+          + ModelEntity.TABLE_NAME
+          + " WHERE catalog_id = ? AND id = ? AND type_code = ? AND realm_id = 
?";
+
+  public static final String ENTITY_INSERT_QUERY =
+      "INSERT INTO "
+          + SCHEMA
+          + "."
+          + ModelEntity.TABLE_NAME
+          + "("
+          + String.join(", ", ModelEntity.ALL_COLUMNS)
+          + ") VALUES ("
+          + ModelEntity.ALL_COLUMNS.stream().map(c -> 
"?").collect(Collectors.joining(", "))
+          + " )";
+
+  public static final String ENTITY_DELETE_QUERY =
+      "DELETE FROM "
+          + SCHEMA
+          + "."
+          + ModelEntity.TABLE_NAME
+          + " WHERE "
+          + String.join(" = ? AND ", ModelEntity.PK_COLUMNS)
+          + " = ?";
+
+  public static final String ENTITY_LOOKUP_BY_NAME_QUERY =
+      "SELECT "
+          + String.join(", ", ModelEntity.ALL_COLUMNS)
+          + " FROM "
+          + SCHEMA
+          + "."
+          + ModelEntity.TABLE_NAME
+          + " WHERE catalog_id = ? AND parent_id = ? AND type_code = ? "
+          + "AND name = ? AND realm_id = ?";
+
+  public static final String ENTITY_LOOKUP_BY_PARENT_ID_QUERY =
+      "SELECT "
+          + String.join(", ", ModelEntity.ALL_COLUMNS)
+          + " FROM "
+          + SCHEMA
+          + "."
+          + ModelEntity.TABLE_NAME
+          + " WHERE catalog_id = ? AND parent_id = ? "
+          + "AND realm_id = ?";
+
+  public static final String ENTITY_LOOKUP_BY_PARENT_ID_WITH_TYPE_CODE_QUERY =
+      "SELECT "
+          + String.join(", ", ModelEntity.ALL_COLUMNS)
+          + " FROM "
+          + SCHEMA
+          + "."
+          + ModelEntity.TABLE_NAME
+          + " WHERE catalog_id = ? AND parent_id = ? AND type_code = ? "
+          + "AND realm_id = ?";
+
+  public static final String ENTITY_LIST_QUERY =
+      "SELECT "
+          + String.join(", ", ModelEntity.ALL_COLUMNS)
+          + " FROM "
+          + SCHEMA
+          + "."
+          + ModelEntity.TABLE_NAME
+          + " WHERE catalog_id = ? AND parent_id = ? AND type_code = ? AND 
realm_id = ?";
+
+  public static final String ENTITY_LIST_BY_CATALOG_ID_AND_ID_QUERY =
+      "SELECT "
+          + String.join(", ", ModelEntity.ALL_COLUMNS)
+          + " FROM "
+          + SCHEMA
+          + "."
+          + ModelEntity.TABLE_NAME
+          + " WHERE (catalog_id, id) IN (%s) AND realm_id = ?";
+
+  public static final String ENTITY_LOOKUP_FOR_GRANT_RECORD_VERSION_QUERY =
+      "SELECT "
+          + String.join(", ", ModelEntity.ALL_COLUMNS)
+          + " FROM "
+          + SCHEMA
+          + "."
+          + ModelEntity.TABLE_NAME
+          + " WHERE catalog_id = ? AND id = ? AND realm_id = ? ";
+
+  public static final String ENTITY_UPDATE_QUERY =
+      "UPDATE "
+          + SCHEMA
+          + "."
+          + ModelEntity.TABLE_NAME
+          + " SET "
+          + ModelEntity.ALL_COLUMNS.stream().map(c -> c + " = 
?").collect(Collectors.joining(", "))
+          + " WHERE catalog_id = ? AND id = ? AND entity_version = ? AND 
realm_id = ? ";
+
+  public static final String ENTITY_DELETE_ALL_QUERY =
+      "DELETE FROM " + SCHEMA + "." + ModelEntity.TABLE_NAME + " WHERE 
realm_id = ?";
+
+  public static final String GRANT_RECORD_INSERT_QUERY =
+      "INSERT INTO "
+          + SCHEMA
+          + "."
+          + ModelGrantRecord.TABLE_NAME
+          + "("
+          + String.join(", ", ModelGrantRecord.ALL_COLUMNS)
+          + ") VALUES ("
+          + ModelGrantRecord.ALL_COLUMNS.stream().map(c -> 
"?").collect(Collectors.joining(", "))
+          + " )";
+
+  public static final String GRANT_RECORD_LOOKUP_BY_PK_QUERY =
+      "SELECT "
+          + String.join(", ", ModelGrantRecord.ALL_COLUMNS)
+          + " FROM "
+          + SCHEMA
+          + "."
+          + ModelGrantRecord.TABLE_NAME
+          + " WHERE "
+          + String.join(" = ? AND ", ModelGrantRecord.PK_COLUMNS)
+          + " = ?";
+
+  public static final String GRANT_RECORD_LOOKUP_BY_SECURABLE_QUERY =
+      "SELECT "
+          + String.join(", ", ModelGrantRecord.ALL_COLUMNS)
+          + " FROM "
+          + SCHEMA
+          + "."
+          + ModelGrantRecord.TABLE_NAME
+          + " WHERE securable_catalog_id = ? AND securable_id = ? AND realm_id 
= ? ";
+
+  public static final String GRANT_RECORD_LOOKUP_BY_GRANTEE_QUERY =
+      "SELECT "
+          + String.join(", ", ModelGrantRecord.ALL_COLUMNS)
+          + " FROM "
+          + SCHEMA
+          + "."
+          + ModelGrantRecord.TABLE_NAME
+          + " WHERE grantee_catalog_id = ? AND grantee_id = ? AND realm_id = ? 
";
+
+  public static final String GRANT_RECORD_DELETE_QUERY_FOR_ENTITY =
+      "DELETE FROM "
+          + SCHEMA
+          + "."
+          + ModelGrantRecord.TABLE_NAME
+          + " WHERE ((grantee_id = ? AND grantee_catalog_id = ?) OR 
(securable_id = ? AND securable_catalog_id = ?)) AND realm_id = ?";
+
+  public static final String GRANT_RECORD_DELETE_QUERY =
+      "DELETE FROM "
+          + SCHEMA
+          + "."
+          + ModelGrantRecord.TABLE_NAME
+          + " WHERE "
+          + String.join(" = ? AND ", ModelGrantRecord.PK_COLUMNS)
+          + " = ?";
+
+  public static final String GRANT_RECORD_DELETE_ALL_QUERY =
+      "DELETE FROM " + SCHEMA + "." + ModelGrantRecord.TABLE_NAME + " WHERE 
realm_id = ?";
+
+  public static final String PRINCIPAL_AUTHENTICATION_DATA_INSERT_QUERY =
+      "INSERT INTO "
+          + SCHEMA
+          + "."
+          + ModelPrincipalAuthenticationData.TABLE_NAME
+          + "("
+          + String.join(", ", ModelPrincipalAuthenticationData.ALL_COLUMNS)
+          + ") VALUES ("
+          + ModelPrincipalAuthenticationData.ALL_COLUMNS.stream()
+              .map(c -> "?")
+              .collect(Collectors.joining(", "))
+          + " )";
+
+  public static final String 
PRINCIPAL_AUTHENTICATION_DATA_LOOKUP_BY_PRIMARY_KET_QUERY =
+      "SELECT "
+          + String.join(", ", ModelPrincipalAuthenticationData.ALL_COLUMNS)
+          + " FROM "
+          + SCHEMA
+          + "."
+          + ModelPrincipalAuthenticationData.TABLE_NAME
+          + " WHERE "
+          + String.join(" = ? AND ", 
ModelPrincipalAuthenticationData.PK_COLUMNS)
+          + " = ?";
+
+  public static final String PRINCIPAL_AUTHENTICATION_DATA_UPDATE_QUERY =

Review Comment:
   Sorry, the same comment on name applies to all these I guess...



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