jerryshao commented on code in PR #14375: URL: https://github.com/apache/gravitino/pull/14375#discussion_r4438556537
########## core/src/main/java/org/apache/gravitino/policy/PolicyBindingMigration.java: ########## @@ -0,0 +1,349 @@ +/* + * 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.gravitino.policy; + +import com.google.common.base.Preconditions; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import org.apache.gravitino.MetadataObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Offline conversion of legacy object-policy relations into server-managed tag relations. */ +public final class PolicyBindingMigration { + + private static final Logger LOG = LoggerFactory.getLogger(PolicyBindingMigration.class); + private final Connection connection; + private final PolicyObjectBindingService objectBindings; + + /** + * Creates a migrator using an exclusively owned connection to an upgraded metadata database. + * + * @param connection A connection with auto-commit disabled. All servers must be stopped. + */ + public PolicyBindingMigration(Connection connection) { + this.connection = connection; + this.objectBindings = new PolicyObjectBindingService(connection); + } + + /** + * Checks the upgrade prerequisites and reports pending work without modifying the database. + * + * @return The number of active legacy bindings. + * @throws SQLException If the schema or a legacy binding is invalid. + */ + public long dryRun() throws SQLException { + requireTargetSchema(); + validateHiddenTags(); + if (!hasLegacyTable(connection)) { + LOG.info("Precheck passed: no legacy policy binding table exists"); + return 0; + } + long count = 0; + for (long policyId : pendingPolicies()) { + long metalakeId = metalakeId(policyId); + // Reject collisions before any policy is migrated. + objectBindings.findHiddenTagId(policyId, metalakeId); + for (Binding binding : bindings(policyId)) { + objectBindings.validateObject(metalakeId, binding.objectId, objectType(binding.type)); + count++; + } + } + LOG.info("Precheck passed: {} legacy policy bindings require migration", count); + return count; + } + + /** + * Migrates each policy in one transaction and verifies the replacement before retiring old rows. + * + * <p>A failed policy is rolled back completely. Previously committed policies remain migrated; + * rerunning the tool resumes with the remaining active legacy rows. + * + * @return The number of migrated legacy bindings. + * @throws SQLException If validation, migration, or verification fails. + */ + public long migrate() throws SQLException { + Preconditions.checkState(!connection.getAutoCommit(), "Migration requires a transaction"); + dryRun(); + if (!hasLegacyTable(connection)) { + return 0; + } + long migrated = 0; + for (long policyId : pendingPolicies()) { + try { + List<Binding> bindings = bindings(policyId); + long tagId = -1; + for (Binding binding : bindings) { + tagId = + PolicyManager.addPolicyToObject( Review Comment: This branch does not contain #14326's current head `c63d29ecd`: `de4f58a7b` deletes `PolicyObjectBindingService` (used at L46) and replaces the static JDBC `PolicyManager.addPolicyToObject(Connection, ...)` with an EntityStore instance method that commits through `SessionUtils`. After a rebase this won't compile, and the single-connection "verify then retire" transaction needs to be redone inside one `SessionUtils` transaction that also soft-deletes the legacy rows. ########## core/src/main/java/org/apache/gravitino/policy/PolicyBindingMigration.java: ########## @@ -0,0 +1,349 @@ +/* + * 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.gravitino.policy; + +import com.google.common.base.Preconditions; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import org.apache.gravitino.MetadataObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Offline conversion of legacy object-policy relations into server-managed tag relations. */ +public final class PolicyBindingMigration { + + private static final Logger LOG = LoggerFactory.getLogger(PolicyBindingMigration.class); + private final Connection connection; + private final PolicyObjectBindingService objectBindings; + + /** + * Creates a migrator using an exclusively owned connection to an upgraded metadata database. + * + * @param connection A connection with auto-commit disabled. All servers must be stopped. + */ + public PolicyBindingMigration(Connection connection) { + this.connection = connection; + this.objectBindings = new PolicyObjectBindingService(connection); + } + + /** + * Checks the upgrade prerequisites and reports pending work without modifying the database. + * + * @return The number of active legacy bindings. + * @throws SQLException If the schema or a legacy binding is invalid. + */ + public long dryRun() throws SQLException { Review Comment: Fail-closed is too tight for an upgrade gate. `dryRun()` (also called by `migrate()`) rejects the entire run on a single missing, soft-deleted, or foreign-metalake object, missing policy, or non-canonical type. `validateHiddenTags()` (L229) also fails on any live `reserved_*` tag that is not a canonical hidden policy tag. Combined with the startup check in `JDBCBackend`, and with the guide saying not to edit relation tables, the only way out is dropping the legacy table, which silently loses unmigrated scope. Please skip-or-report bad rows (policy id, object id, type), narrow the reserved-name check to canonical hidden tags, and add an explicit, logged break-glass. ########## core/src/main/java/org/apache/gravitino/policy/PolicyBindingMigration.java: ########## @@ -0,0 +1,349 @@ +/* + * 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.gravitino.policy; + +import com.google.common.base.Preconditions; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import org.apache.gravitino.MetadataObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Offline conversion of legacy object-policy relations into server-managed tag relations. */ +public final class PolicyBindingMigration { + + private static final Logger LOG = LoggerFactory.getLogger(PolicyBindingMigration.class); + private final Connection connection; + private final PolicyObjectBindingService objectBindings; + + /** + * Creates a migrator using an exclusively owned connection to an upgraded metadata database. + * + * @param connection A connection with auto-commit disabled. All servers must be stopped. + */ + public PolicyBindingMigration(Connection connection) { + this.connection = connection; + this.objectBindings = new PolicyObjectBindingService(connection); + } + + /** + * Checks the upgrade prerequisites and reports pending work without modifying the database. + * + * @return The number of active legacy bindings. + * @throws SQLException If the schema or a legacy binding is invalid. + */ + public long dryRun() throws SQLException { + requireTargetSchema(); + validateHiddenTags(); + if (!hasLegacyTable(connection)) { + LOG.info("Precheck passed: no legacy policy binding table exists"); + return 0; + } + long count = 0; + for (long policyId : pendingPolicies()) { + long metalakeId = metalakeId(policyId); + // Reject collisions before any policy is migrated. + objectBindings.findHiddenTagId(policyId, metalakeId); + for (Binding binding : bindings(policyId)) { + objectBindings.validateObject(metalakeId, binding.objectId, objectType(binding.type)); + count++; + } + } + LOG.info("Precheck passed: {} legacy policy bindings require migration", count); + return count; + } + + /** + * Migrates each policy in one transaction and verifies the replacement before retiring old rows. + * + * <p>A failed policy is rolled back completely. Previously committed policies remain migrated; + * rerunning the tool resumes with the remaining active legacy rows. + * + * @return The number of migrated legacy bindings. + * @throws SQLException If validation, migration, or verification fails. + */ + public long migrate() throws SQLException { + Preconditions.checkState(!connection.getAutoCommit(), "Migration requires a transaction"); + dryRun(); + if (!hasLegacyTable(connection)) { + return 0; + } + long migrated = 0; + for (long policyId : pendingPolicies()) { + try { + List<Binding> bindings = bindings(policyId); + long tagId = -1; + for (Binding binding : bindings) { + tagId = + PolicyManager.addPolicyToObject( + connection, + policyId, + binding.objectId, + objectType(binding.type), + binding.auditInfo); + } + verifyReplacement(policyId, tagId, bindings); + LOG.info("Verified {} replacement bindings for policy {}", bindings.size(), policyId); + int cleaned = + execute( + "UPDATE policy_relation_meta SET deleted_at = ?, last_version = current_version + 1," + + " current_version = current_version + 1 WHERE policy_id = ? AND deleted_at = 0", + System.currentTimeMillis(), + policyId); + if (cleaned != bindings.size()) { + throw new SQLException("Legacy bindings changed during migration; stop all servers"); + } + connection.commit(); + migrated += cleaned; + LOG.info("Committed policy {}: {} migrated and retired bindings", policyId, cleaned); + } catch (SQLException | RuntimeException e) { + connection.rollback(); + throw e; + } + } + verify(); + return migrated; + } + + /** + * Verifies that no active legacy bindings remain and all hidden tags are valid. + * + * @throws SQLException If migration is incomplete or hidden bindings are invalid. + */ + public void verify() throws SQLException { + requireTargetSchema(); + checkMigrationComplete(connection); + validateHiddenTags(); + LOG.info("Policy binding migration verification passed"); + } + + /** + * Refuses server startup when active legacy bindings still need migration. + * + * @param connection The metadata database connection. + * @throws SQLException If the migration is incomplete or its status cannot be read. + */ + public static void checkMigrationComplete(Connection connection) throws SQLException { + if (!hasLegacyTable(connection)) { + return; + } + try (PreparedStatement statement = + connection.prepareStatement( + "SELECT id FROM policy_relation_meta WHERE deleted_at = 0"); Review Comment: This query has no `LIMIT`, and there is no index whose leading column is `deleted_at`. The legacy table is kept forever after migration, so every server start full-scans it. Please add `LIMIT 1` plus an index (or a one-row completion marker). ########## core/src/main/java/org/apache/gravitino/policy/PolicyBindingMigrationTool.java: ########## @@ -0,0 +1,106 @@ +/* + * 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.gravitino.policy; + +import com.google.common.base.Preconditions; +import java.sql.Connection; +import java.sql.DriverManager; +import org.apache.gravitino.Config; +import org.apache.gravitino.Configs; +import org.apache.gravitino.storage.relational.JDBCBackend.JDBCBackendType; +import org.apache.gravitino.storage.relational.database.H2Database; +import org.apache.gravitino.utils.JdbcUrlUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Command-line entry point for offline policy binding migration. */ +public final class PolicyBindingMigrationTool { + + private static final Logger LOG = LoggerFactory.getLogger(PolicyBindingMigrationTool.class); + + private PolicyBindingMigrationTool() {} + + /** + * Runs the offline migration against the configured metadata store. + * + * @param args No arguments to migrate, or one of --dry-run, --verify, --help. + * @throws Exception If configuration, precheck, migration, or verification fails. + */ + public static void main(String[] args) throws Exception { + String mode = mode(args); + if ("--help".equals(mode)) { + LOG.info( + "Usage: bin/migrate-policy-bindings.sh [--dry-run | --verify | --help]." + + " Stop all servers, back up the database, and apply schema upgrades first."); + return; + } + Config config = new Config() {}; + config.loadFromFile("gravitino.conf"); + Preconditions.checkArgument( + Configs.RELATIONAL_ENTITY_STORE.equals(config.get(Configs.ENTITY_STORE)) + && Configs.DEFAULT_ENTITY_RELATIONAL_STORE.equals( + config.get(Configs.ENTITY_RELATIONAL_STORE)), + "Policy migration requires the JDBC relational entity store"); + String url = config.get(Configs.ENTITY_RELATIONAL_JDBC_BACKEND_URL); + String driver = config.get(Configs.ENTITY_RELATIONAL_JDBC_BACKEND_DRIVER); + JdbcUrlUtils.validateJdbcConfig(driver, url, config.getAllConfig()); + if (JDBCBackendType.fromURI(url) == JDBCBackendType.H2) { + url = H2Database.getConnectionUrl(config); Review Comment: Offline is documented but not enforced. The H2 URL keeps `AUTO_SERVER=TRUE`, so a running server can share the file, and the per-policy `cleaned != bindings.size()` check only rolls back after the fact. Please take an advisory lock or fail if other connections are present. ########## bin/migrate-policy-bindings.sh.template: ########## @@ -0,0 +1,30 @@ +#!/bin/bash +# +# 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. +# + +set -eo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=bin/common.sh.template +. "${SCRIPT_DIR}/common.sh" +check_java_version +addJarInDir "${GRAVITINO_HOME}/libs" +cd "${GRAVITINO_HOME}" +exec "${JAVA_RUNNER}" -Dlog4j.configurationFile=policy-binding-migration-log4j2.properties -cp "${GRAVITINO_CLASSPATH}" \ Review Comment: nit: Other launchers pass `-Dlog4j2.configurationFile=file://...`. Please use an explicit `classpath:` URL here so a lookup miss doesn't fall back to `conf/log4j2.properties`. Also, `--help` is printed via the logger, so a missing log config gives a stack trace and no usage; printing usage to stdout would be safer. ########## core/src/main/java/org/apache/gravitino/policy/PolicyBindingMigration.java: ########## @@ -0,0 +1,349 @@ +/* + * 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.gravitino.policy; + +import com.google.common.base.Preconditions; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import org.apache.gravitino.MetadataObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Offline conversion of legacy object-policy relations into server-managed tag relations. */ +public final class PolicyBindingMigration { + + private static final Logger LOG = LoggerFactory.getLogger(PolicyBindingMigration.class); + private final Connection connection; + private final PolicyObjectBindingService objectBindings; + + /** + * Creates a migrator using an exclusively owned connection to an upgraded metadata database. + * + * @param connection A connection with auto-commit disabled. All servers must be stopped. + */ + public PolicyBindingMigration(Connection connection) { + this.connection = connection; + this.objectBindings = new PolicyObjectBindingService(connection); + } + + /** + * Checks the upgrade prerequisites and reports pending work without modifying the database. + * + * @return The number of active legacy bindings. + * @throws SQLException If the schema or a legacy binding is invalid. + */ + public long dryRun() throws SQLException { + requireTargetSchema(); + validateHiddenTags(); + if (!hasLegacyTable(connection)) { + LOG.info("Precheck passed: no legacy policy binding table exists"); + return 0; + } + long count = 0; + for (long policyId : pendingPolicies()) { + long metalakeId = metalakeId(policyId); + // Reject collisions before any policy is migrated. + objectBindings.findHiddenTagId(policyId, metalakeId); + for (Binding binding : bindings(policyId)) { + objectBindings.validateObject(metalakeId, binding.objectId, objectType(binding.type)); + count++; + } + } + LOG.info("Precheck passed: {} legacy policy bindings require migration", count); + return count; + } + + /** + * Migrates each policy in one transaction and verifies the replacement before retiring old rows. + * + * <p>A failed policy is rolled back completely. Previously committed policies remain migrated; + * rerunning the tool resumes with the remaining active legacy rows. + * + * @return The number of migrated legacy bindings. + * @throws SQLException If validation, migration, or verification fails. + */ + public long migrate() throws SQLException { + Preconditions.checkState(!connection.getAutoCommit(), "Migration requires a transaction"); + dryRun(); + if (!hasLegacyTable(connection)) { + return 0; + } + long migrated = 0; + for (long policyId : pendingPolicies()) { + try { + List<Binding> bindings = bindings(policyId); + long tagId = -1; + for (Binding binding : bindings) { + tagId = + PolicyManager.addPolicyToObject( + connection, + policyId, + binding.objectId, + objectType(binding.type), + binding.auditInfo); + } + verifyReplacement(policyId, tagId, bindings); + LOG.info("Verified {} replacement bindings for policy {}", bindings.size(), policyId); + int cleaned = + execute( + "UPDATE policy_relation_meta SET deleted_at = ?, last_version = current_version + 1," + + " current_version = current_version + 1 WHERE policy_id = ? AND deleted_at = 0", + System.currentTimeMillis(), + policyId); + if (cleaned != bindings.size()) { + throw new SQLException("Legacy bindings changed during migration; stop all servers"); + } + connection.commit(); + migrated += cleaned; + LOG.info("Committed policy {}: {} migrated and retired bindings", policyId, cleaned); + } catch (SQLException | RuntimeException e) { + connection.rollback(); + throw e; + } + } + verify(); + return migrated; + } + + /** + * Verifies that no active legacy bindings remain and all hidden tags are valid. + * + * @throws SQLException If migration is incomplete or hidden bindings are invalid. + */ + public void verify() throws SQLException { + requireTargetSchema(); + checkMigrationComplete(connection); + validateHiddenTags(); + LOG.info("Policy binding migration verification passed"); + } + + /** + * Refuses server startup when active legacy bindings still need migration. + * + * @param connection The metadata database connection. + * @throws SQLException If the migration is incomplete or its status cannot be read. + */ + public static void checkMigrationComplete(Connection connection) throws SQLException { + if (!hasLegacyTable(connection)) { + return; + } + try (PreparedStatement statement = + connection.prepareStatement( + "SELECT id FROM policy_relation_meta WHERE deleted_at = 0"); + ResultSet rows = statement.executeQuery()) { + if (rows.next()) { + throw new SQLException( + "Legacy policy bindings require offline migration. Stop all servers," + + " back up the database, and run bin/migrate-policy-bindings.sh before starting Gravitino."); + } + } + } + + private static boolean hasLegacyTable(Connection connection) throws SQLException { + // Match exact names in the current schema: '_' is a wildcard in JDBC metadata patterns. + try (ResultSet tables = + connection + .getMetaData() + .getTables( + connection.getCatalog(), connection.getSchema(), "%", new String[] {"TABLE"})) { + while (tables.next()) { + if ("policy_relation_meta".equalsIgnoreCase(tables.getString("TABLE_NAME"))) { + return true; + } + } + } + return false; + } + + private void requireTargetSchema() throws SQLException { + // These reads also fail clearly if the 2.0 schema upgrade has not been applied. + count("SELECT COUNT(allowed_values) FROM tag_meta WHERE 1 = 0"); + count("SELECT COUNT(hidden) FROM tag_meta WHERE 1 = 0"); + count("SELECT COUNT(tag_value) FROM tag_relation_meta WHERE 1 = 0"); + count("SELECT COUNT(selector) FROM policy_tag_relation_meta WHERE 1 = 0"); + } + + private List<Long> pendingPolicies() throws SQLException { + List<Long> ids = new ArrayList<>(); + try (PreparedStatement statement = + connection.prepareStatement( + "SELECT DISTINCT policy_id FROM policy_relation_meta WHERE deleted_at = 0 ORDER BY policy_id"); + ResultSet rows = statement.executeQuery()) { + while (rows.next()) { + ids.add(rows.getLong(1)); + } + } + return ids; + } + + private long metalakeId(long policyId) throws SQLException { + try (PreparedStatement statement = + prepare( + "SELECT p.metalake_id FROM policy_meta p JOIN metalake_meta m" + + " ON p.metalake_id = m.metalake_id WHERE p.policy_id = ?" + + " AND p.deleted_at = 0 AND m.deleted_at = 0", + policyId); + ResultSet rows = statement.executeQuery()) { + if (!rows.next()) { + throw new SQLException("Legacy binding references missing policy or metalake: " + policyId); + } + return rows.getLong(1); + } + } + + private List<Binding> bindings(long policyId) throws SQLException { + List<Binding> bindings = new ArrayList<>(); + try (PreparedStatement statement = + prepare( + "SELECT metadata_object_id, metadata_object_type, audit_info FROM policy_relation_meta" + + " WHERE policy_id = ? AND deleted_at = 0 ORDER BY id", + policyId); + ResultSet rows = statement.executeQuery()) { + while (rows.next()) { + bindings.add(new Binding(rows.getLong(1), rows.getString(2), rows.getString(3))); + } + } + return bindings; + } + + private void validateHiddenTags() throws SQLException { + // Inspect names in Java so LIKE wildcard escaping cannot hide a reserved-name collision. + try (PreparedStatement statement = + connection.prepareStatement( + "SELECT tag_id, tag_name, metalake_id FROM tag_meta WHERE deleted_at = 0"); + ResultSet rows = statement.executeQuery()) { + while (rows.next()) { + String name = rows.getString(2); + if (!HiddenPolicyTag.isHidden(name)) { + continue; + } + long policyId; + try { + policyId = HiddenPolicyTag.policyId(name); + } catch (IllegalArgumentException e) { + throw new SQLException("Invalid reserved tag name: " + name, e); + } + long tagId = rows.getLong(1); + long metalakeId = rows.getLong(3); + if (metalakeId(policyId) != metalakeId + || objectBindings.findHiddenTagId(policyId, metalakeId) != tagId) { + throw new SQLException("Invalid hidden tag identity: " + tagId); + } + if (count( + "SELECT COUNT(*) FROM policy_tag_relation_meta WHERE tag_id = ?" + + " AND deleted_at = 0 AND (policy_id <> ? OR selector IS NOT NULL)", + tagId, + policyId) + != 0) { + throw new SQLException("Unexpected policy or selector for hidden tag " + tagId); + } + if (count( + "SELECT COUNT(*) FROM policy_tag_relation_meta WHERE tag_id = ?" + + " AND policy_id = ? AND deleted_at = 0", + tagId, + policyId) + != 1) { + throw new SQLException("Missing policy relation for hidden tag " + tagId); + } + if (count( + "SELECT COUNT(*) FROM tag_relation_meta WHERE tag_id = ?" + + " AND deleted_at = 0 AND tag_value <> ''", + tagId) + != 0) { + throw new SQLException("Unexpected assignment value for hidden tag " + tagId); + } + } + } + } + + private void verifyReplacement(long policyId, long tagId, List<Binding> bindings) Review Comment: For large policies, `bindings()` loads every row into memory, and this runs one `COUNT(*)` per binding inside the open transaction. Please batch the inserts and verify with one grouped query. ########## core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java: ########## @@ -126,6 +129,17 @@ public class JDBCBackend implements RelationalBackend, SupportsOrphanedRelationC public void initialize(Config config) { jdbcDatabase = startJDBCDatabaseIfNecessary(config); SqlSessionFactoryHelper.getInstance().init(config); + try (Connection connection = + SqlSessionFactoryHelper.getInstance() + .getSqlSessionFactory() + .getConfiguration() + .getEnvironment() + .getDataSource() + .getConnection()) { + PolicyBindingMigration.checkMigrationComplete(connection); Review Comment: This refuses startup while any `policy_relation_meta.deleted_at = 0` row remains, so one unmigratable row becomes a server outage. See the comment on `dryRun()`; please provide a documented, logged way to bypass this gate. -- 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]
