-
Notifications
You must be signed in to change notification settings - Fork 40
Backport to branch(3) : Add rename table #3025
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces the renameTable
functionality to the Admin
API. The implementation is provided for JDBC-based storages, while for Cassandra, Cosmos DB, and DynamoDB, it correctly throws UnsupportedOperationException
as they do not support table renaming. The changes are well-spread across the codebase, including interface definitions, common admin logic, specific storage implementations, and transaction admin decorators. Integration tests have been added to cover the new functionality, including permission checks and scenarios with indexes.
My review focuses on ensuring consistency and correctness. I have one suggestion in JdbcAdmin.java
to use the newly added CoreError
enum for exception messages to maintain consistency with the rest of the codebase. Overall, the changes are solid and well-tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This is a backport PR that adds rename table functionality to the ScalarDB Admin API. The changes introduce a new renameTable
method to the Admin interface and implement it across all storage adapters and transaction managers.
- Added
renameTable
method to the Admin interface and all implementing classes - Implemented table renaming for JDBC-based storage adapters with database-specific SQL generation
- Added comprehensive test coverage for the new functionality across all storage types
- Properly handled unsupported operations for non-JDBC storage types (Cassandra, Cosmos DB, DynamoDB)
Reviewed Changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
core/src/main/java/com/scalar/db/api/Admin.java | Added renameTable method to the core Admin interface |
core/src/main/java/com/scalar/db/storage/jdbc/* | Implemented JDBC-specific table renaming with database engine strategies |
core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java | Added unsupported operation implementation for Cassandra |
core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java | Added unsupported operation implementation for Cosmos DB |
core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java | Added unsupported operation implementation for DynamoDB |
core/src/main/java/com/scalar/db/transaction/* | Added renameTable delegation in transaction admin classes |
integration-test/src/main/java/* | Added comprehensive integration tests for table renaming functionality |
core/src/test/java/* | Added unit tests covering all storage adapters and transaction managers |
core/src/integration-test/java/* | Disabled unsupported operations tests for non-JDBC storage types |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This is an automated request for a manual backport of the following:
Thank you!