Fix to avoid deleting non ScalarDB tables when dropping namespaces

[KodaiD]

Description

This PR fixes a bug that drops non-ScalarDB-managed tables when dropping namespaces.

Suppose a user has multiple tables under the schema "ns" that are not managed by ScalarDB. If the user imports one of these tables using importTable, ScalarDB will only see one table existing within "ns". Subsequently, if the user drops that table through ScalarDB, ScalarDB will determine that "ns" is empty and internally issue a DROP SCHEMA ns query. As a result, all tables not under ScalarDB's management will be dropped.

Some databases, including MySQL, allow schema deletion to succeed even if tables still exist within the schema, and there is no way to prevent this. Therefore, it is necessary to check for the existence of tables before deleting the schema.

Since it is difficult to include table existence checking logic within RdbEngineStrategy.dropNamespaceSql, the table existence check is performed for all databases in the JdbcAdmin abstraction layer.

Related issues and/or PRs

N/A

Changes made

• Updated JdbcAdmin to check if non-ScalarDB tables exist before dropping the specified schema.
• Updated unit test cases related to dropNamespace.
• Added an integration test case that covers the bug.

Checklist

• I have commented my code, particularly in hard-to-understand areas.
• I have updated the documentation to reflect the changes.
• I have considered whether similar issues could occur in other products, components, or modules if this PR is for bug fixes.
• Any remaining open issues linked to this PR are documented and up-to-date (Jira, GitHub, etc.).
• Tests (unit, integration, etc.) have been added for the changes.
• My changes generate no new warnings.
• Any dependent changes in other PRs have been merged and published.

Additional notes (optional)

N/A

Release notes

N/A

[KodaiD]

/gemini review

[gemini-code-assist]

Code Review

This PR addresses a bug in JdbcAdmin that caused non-ScalarDB tables to be dropped when dropping namespaces. The fix involves checking for remaining tables in a namespace before dropping it and throwing an exception if non-ScalarDB tables exist. Unit and integration tests have been updated to reflect this change.

[Copilot]

Pull Request Overview

This PR fixes a critical bug where dropping a namespace in ScalarDB could inadvertently delete non-ScalarDB-managed tables. The fix adds a check to verify that no non-ScalarDB tables remain in a namespace before allowing the namespace to be dropped, throwing an IllegalStateException if any such tables are found.

Key Changes:

• Added namespace emptiness validation in JdbcAdmin.dropNamespace() before attempting to drop the schema
• Implemented getTableNamesInNamespaceSql() method across all RdbEngine implementations to query existing tables in a namespace
• Added integration tests to verify the fix prevents deletion of non-ScalarDB tables

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
JdbcAdmin.java	Added table existence check before dropping namespace
RdbEngineStrategy.java	Added interface method for querying table names in a namespace
RdbEngineMysql.java	Implemented SQL query to get table names for MySQL
RdbEnginePostgresql.java	Implemented SQL query to get table names for PostgreSQL
RdbEngineSqlServer.java	Implemented SQL query to get table names for SQL Server
RdbEngineOracle.java	Implemented SQL query to get table names for Oracle
RdbEngineDb2.java	Implemented SQL query to get table names for DB2
RdbEngineSqlite.java	Implemented SQL query to get table names for SQLite
JdbcAdminTest.java	Updated unit tests to mock the new namespace emptiness check
DistributedStorageAdminImportTableIntegrationTestBase.java	Added integration test and updated teardown logic
DistributedTransactionAdminImportTableIntegrationTestBase.java	Added integration test for transaction admin
JdbcAdminImportTableIntegrationTest.java	Implemented test utility method and disabled SQLite test
ConsensusCommitAdminImportTableIntegrationTestWithJdbcDatabase.java	Implemented test utility method and disabled SQLite test

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[komamitsu]

LGTM, thank you!

[feeblefakie]

Overall, looking good. Thank you for the prompt work.
But, should we also check other databases, such as Cassandra, Cosmos, and Dynamo?
Cassandra will probably drop a keyspace even if a table exists in the keyspace.

[KodaiD]

Overall, looking good. Thank you for the prompt work. But, should we also check other databases, such as Cassandra, Cosmos, and Dynamo? Cassandra will probably drop a keyspace even if a table exists in the keyspace.

@feeblefakie Ah, I see... I had assumed that the case where ScalarDB managed tables and unmanaged tables coexist only occurs when importTable is executed. Therefore, I only implemented the bug fix for JDBC databases. However, even in the case of non-JDBC databases, it's possible for a user to create a ScalarDB table and then create a non-ScalarDB managed table using the same namespace name. Let me check.

[KodaiD]

In Cassandra adapter, table metadata of both ScalarDB-managed tables and non-ScalarDB-managed tables is stored in the Cassandra metadata table. Therefore, Cassandra can detect remaining tables when dropping a key space, even if they're non-ScalarDB-managed tables.

For DynamoDB, since it doesn't have a concept of namespaces (a ScalarDB namespace is represented as a table name prefix), we don't need to care about the bug. SQLite is as well.

Therefore, a check for the existence of non-ScalarDB-managed tables has been added to CosmosAdmin and JdbcAdmin (except for SQLite).

[KodaiD]

@feeblefakie Now the PR is updated. PTAL!

[brfrn169]

Left several comments. PTAL!

[brfrn169]

Left a couple of comments. Other than that, LGTM! Thank you!

[brfrn169]

LGTM! Thank you!

[feeblefakie]

I might miss something, but Cassandra drops a keyspace even if there exists a table.
So, should it check the table existence for Cassandra as well to meet the following requirement?

dropNamespace won’t drop a given namespace if there is at least one table that is either ScalarDB-managed or non-ScalarDB-managed.

[KodaiD]

I might miss something, but Cassandra drops a keyspace even if there exists a table. So, should it check the table existence for Cassandra as well to meet the following requirement?

dropNamespace won’t drop a given namespace if there is at least one table that is either ScalarDB-managed or non-ScalarDB-managed.

@feeblefakie I appreciate the confirmation. Unlike other adapters, Cassandra adapter does not create its own table metadata table. Instead, it uses Cassandra's table metadata table. Cassandra's table metadata table stores information about both ScalarDB tables and non-ScalarDB tables. Therefore, before CassandraAdmin.dropNamespace is called, the following part of CommonDistributedStorageAdmin.dropNamespace can detect the existence of non-ScalarDB tables and throw an exception.

if (!getNamespaceTableNames(namespace).isEmpty()) {
  throw new IllegalArgumentException(
      CoreError.NAMESPACE_NOT_EMPTY.buildMessage(namespace, getNamespaceTableNames(namespace)));
}

The integration test case added in this PR also succeeds with Cassandra adapter, confirming that dropNamespace does not delete non-ScalarDB tables.

[Torch3333]

LGTM, thank you!

[feeblefakie]

@KodaiD Ah, OK. Thanks for the clarification!

[feeblefakie]

LGTM! Thank you!