roryqi commented on code in PR #14528:
URL: https://github.com/apache/gravitino/pull/14528#discussion_r4442379543
##########
catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlSchemaOperations.java:
##########
@@ -134,14 +136,52 @@ private ResultSet getSchema(Connection connection, String
schemaName) throws SQL
return metaData.getSchemas(connection.getCatalog(), schemaName);
}
+ /**
+ * Generates an atomic, restrictive schema drop even when cascade is
requested.
+ *
+ * <p>PostgreSQL cascade can remove objects outside this schema. Until
schema-local cascade is
+ * supported safely, the generated statement refuses every non-empty schema.
+ *
+ * @param schema the schema to drop
+ * @param cascade whether cascade was requested; it does not enable
PostgreSQL native cascade
+ * @return the restrictive drop statement
+ */
@Override
public String generateDropDatabaseSql(String schema, boolean cascade) {
- StringBuilder sqlBuilder =
- new StringBuilder(String.format("DROP SCHEMA %s%s%s", PG_QUOTE,
schema, PG_QUOTE));
- if (cascade) {
- sqlBuilder.append(" CASCADE");
+ // PostgreSQL CASCADE also removes objects outside the schema. A
client-side dependency
+ // preflight cannot close the race with concurrent DDL, so always let
PostgreSQL enforce
+ // RESTRICT atomically. Non-empty cascade requests are refused until a
schema-local cascade
+ // can be guaranteed for every object kind and dependency type.
+ return "DROP SCHEMA " + quoteName(schema) + " RESTRICT";
+ }
+
+ /**
+ * Drops an empty schema without requiring explicit locks on its relations.
+ *
+ * @param schema the schema to drop
+ * @param cascade whether cascade was requested
+ * @throws NonEmptySchemaException if the schema is non-empty, including
when cascade is requested
+ */
+ @Override
+ protected void dropDatabase(String schema, boolean cascade) {
+ try (Connection connection = getConnection()) {
+ JdbcConnectorUtils.executeUpdate(connection,
generateDropDatabaseSql(schema, cascade));
+ } catch (SQLException e) {
+ if ("2BP01".equals(e.getSQLState())) {
Review Comment:
Could u use a string constant instead of a string?
--
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]
