yuqi1129 commented on code in PR #14493:
URL: https://github.com/apache/gravitino/pull/14493#discussion_r4439926741
##########
catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlSchemaOperations.java:
##########
@@ -40,12 +40,36 @@
import org.apache.gravitino.catalog.jdbc.config.JdbcConfig;
import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter;
import org.apache.gravitino.catalog.jdbc.operation.JdbcDatabaseOperations;
+import org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
+import org.apache.gravitino.exceptions.NonEmptySchemaException;
import org.apache.gravitino.meta.AuditInfo;
/** Database operations for PostgreSQL. */
public class PostgreSqlSchemaOperations extends JdbcDatabaseOperations {
+ private static final String EXTERNAL_DEPENDENTS_SQL =
+ "WITH RECURSIVE dependents(classid, objid, objsubid) AS ( "
+ + " SELECT 'pg_catalog.pg_namespace'::regclass::oid, oid, 0 "
+ + " FROM pg_catalog.pg_namespace WHERE nspname = ? "
+ + " UNION "
+ + " SELECT next_object.classid, next_object.objid,
next_object.objsubid "
+ + " FROM dependents current_object "
+ + " CROSS JOIN LATERAL ( "
+ + " SELECT d.classid, d.objid, d.objsubid FROM
pg_catalog.pg_depend d "
+ + " WHERE d.refclassid = current_object.classid AND d.refobjid =
current_object.objid "
+ + " AND (current_object.objsubid = 0 OR d.refobjsubid =
current_object.objsubid) "
+ + " UNION "
+ + " SELECT d.refclassid, d.refobjid, d.refobjsubid FROM
pg_catalog.pg_depend d "
+ + " WHERE d.classid = current_object.classid AND d.objid =
current_object.objid "
+ + " AND d.objsubid = current_object.objsubid AND d.deptype =
'i' "
+ + " ) next_object "
+ + ") "
+ + "SELECT DISTINCT pg_catalog.pg_describe_object(d.classid, d.objid,
d.objsubid) AS dependent "
+ + "FROM dependents d "
+ + "CROSS JOIN LATERAL pg_catalog.pg_identify_object(d.classid,
d.objid, d.objsubid) info "
+ + "WHERE info.schema IS NOT NULL AND info.schema <> ?; ";
Review Comment:
Fixed on the original branch in
[f7622e853d](https://github.com/yuqi1129/gravitino/commit/f7622e853d41c02fff1237ac809ce976b81aa149).
Schema-less automatic subobjects now resolve through their owning object, so
an external nextval default blocks the drop and remains usable after rollback.
Added testCascadeDropPreservesExternalColumnDefault; removing the owner
traversal makes it fail.
##########
catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlSchemaOperations.java:
##########
@@ -144,6 +168,67 @@ public String generateDropDatabaseSql(String schema,
boolean cascade) {
return sqlBuilder.toString();
}
+ @Override
+ protected void dropDatabase(String schema, boolean cascade) {
+ if (!cascade) {
+ super.dropDatabase(schema, false);
+ return;
+ }
+ try (Connection connection = getConnection()) {
+ connection.setAutoCommit(false);
+ try {
+ // Keep existing relations stable while checking dependencies and
executing the drop.
+ try (PreparedStatement relations =
+ connection.prepareStatement(
+ "SELECT c.relname FROM pg_catalog.pg_class c JOIN
pg_catalog.pg_namespace n "
+ + "ON c.relnamespace = n.oid WHERE n.nspname = ? AND
c.relkind IN ('r', 'p') ORDER BY c.oid")) {
Review Comment:
Fixed on the original branch in
[f7622e853d](https://github.com/yuqi1129/gravitino/commit/f7622e853d41c02fff1237ac809ce976b81aa149).
The transaction executes native DROP first, then validates the still-visible
catalog through a snapshot exported after DROP before committing; rejected
drops roll back. Tests cover dependencies on existing/new views committed
before DROP, dependencies committed while DROP waits, and new references
blocked until commit. Reverting to pre-DROP validation makes both interleaving
regressions fail.
##########
catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlSchemaOperations.java:
##########
@@ -40,12 +40,36 @@
import org.apache.gravitino.catalog.jdbc.config.JdbcConfig;
import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter;
import org.apache.gravitino.catalog.jdbc.operation.JdbcDatabaseOperations;
+import org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
+import org.apache.gravitino.exceptions.NonEmptySchemaException;
import org.apache.gravitino.meta.AuditInfo;
/** Database operations for PostgreSQL. */
public class PostgreSqlSchemaOperations extends JdbcDatabaseOperations {
+ private static final String EXTERNAL_DEPENDENTS_SQL =
+ "WITH RECURSIVE dependents(classid, objid, objsubid) AS ( "
+ + " SELECT 'pg_catalog.pg_namespace'::regclass::oid, oid, 0 "
+ + " FROM pg_catalog.pg_namespace WHERE nspname = ? "
+ + " UNION "
+ + " SELECT next_object.classid, next_object.objid,
next_object.objsubid "
+ + " FROM dependents current_object "
+ + " CROSS JOIN LATERAL ( "
+ + " SELECT d.classid, d.objid, d.objsubid FROM
pg_catalog.pg_depend d "
+ + " WHERE d.refclassid = current_object.classid AND d.refobjid =
current_object.objid "
+ + " AND (current_object.objsubid = 0 OR d.refobjsubid =
current_object.objsubid) "
+ + " UNION "
+ + " SELECT d.refclassid, d.refobjid, d.refobjsubid FROM
pg_catalog.pg_depend d "
+ + " WHERE d.classid = current_object.classid AND d.objid =
current_object.objid "
+ + " AND d.objsubid = current_object.objsubid AND d.deptype =
'i' "
+ + " ) next_object "
+ + ") "
+ + "SELECT DISTINCT pg_catalog.pg_describe_object(d.classid, d.objid,
d.objsubid) AS dependent "
+ + "FROM dependents d "
+ + "CROSS JOIN LATERAL pg_catalog.pg_identify_object(d.classid,
d.objid, d.objsubid) info "
+ + "WHERE info.schema IS NOT NULL AND info.schema <> ?; ";
Review Comment:
Addressed by 1d76b1d50d and retained on the original branch in
[f7622e853d](https://github.com/yuqi1129/gravitino/commit/f7622e853d41c02fff1237ac809ce976b81aa149).
pg_toast is excluded only from final schema-boundary reporting, not from
dependency traversal. testCascadeDropIncludesOwnedToastObjects verifies that a
real TOAST table exists and is removed by a successful contained drop.
##########
catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlSchemaOperations.java:
##########
@@ -144,6 +168,67 @@ public String generateDropDatabaseSql(String schema,
boolean cascade) {
return sqlBuilder.toString();
}
+ @Override
+ protected void dropDatabase(String schema, boolean cascade) {
+ if (!cascade) {
+ super.dropDatabase(schema, false);
+ return;
+ }
+ try (Connection connection = getConnection()) {
+ connection.setAutoCommit(false);
+ try {
+ // Keep existing relations stable while checking dependencies and
executing the drop.
+ try (PreparedStatement relations =
+ connection.prepareStatement(
+ "SELECT c.relname FROM pg_catalog.pg_class c JOIN
pg_catalog.pg_namespace n "
+ + "ON c.relnamespace = n.oid WHERE n.nspname = ? AND
c.relkind IN ('r', 'p') ORDER BY c.oid")) {
+ relations.setString(1, schema);
+ try (ResultSet result = relations.executeQuery()) {
+ while (result.next()) {
+ JdbcConnectorUtils.executeUpdate(
+ connection,
+ "LOCK TABLE ONLY "
+ + quoteName(schema)
+ + "."
+ + quoteName(result.getString(1))
+ + " IN ACCESS EXCLUSIVE MODE");
Review Comment:
Fixed on the original branch in
[f7622e853d](https://github.com/yuqi1129/gravitino/commit/f7622e853d41c02fff1237ac809ce976b81aa149)
by removing explicit LOCK TABLE.
testSchemaOwnerCanDropAnotherRolesTableWithOneSlotPool runs as a non-superuser
schema owner without SELECT privilege on another role's table and succeeds,
including with a one-slot pool. Restoring the old locking code makes the test
fail with permission denied.
--
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]
