Skip to content

Conversation

@feeblefakie
Copy link
Contributor

This is an automated request for a manual backport of the following:

  1. Resolve any conflicts that occur during the cherry-picking process.
git fetch origin &&
git checkout 3-pull-2990 &&
git cherry-pick --no-rerere-autoupdate -m1 2e7cd60ebea64f069d58ccc3891a64509e4d1e21
  1. Push the changes.
  2. Merge this PR after all checks have passed.

Thank you!

@KodaiD KodaiD marked this pull request as ready for review September 11, 2025 14:56
@brfrn169
Copy link
Collaborator

/gemini review

@brfrn169 brfrn169 requested a review from Copilot September 11, 2025 15:14
Copy link
Contributor

Copilot AI left a 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 PR backports the "Add rename column" feature from a previous pull request. The purpose is to add a new renameColumn method to the admin interfaces and implementations across all storage backends in ScalarDB.

Key Changes

  • Added renameColumn method to Admin interfaces and all implementations
  • Updated TableMetadata.Builder to support column renaming operations
  • Added comprehensive test coverage for the new functionality

Reviewed Changes

Copilot reviewed 53 out of 53 changed files in this pull request and generated no comments.

Show a summary per file
File Description
integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java Added comprehensive integration tests for renaming columns in various scenarios
integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminPermissionIntegrationTestBase.java Added permission test and exposed helper methods as protected
integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java Added integration tests and exposed admin field as protected
core/src/test/java/* Added unit tests for renameColumn across all admin implementations
core/src/main/java/com/scalar/db/transaction/* Implemented renameColumn delegation in transaction admin classes
core/src/main/java/com/scalar/db/storage/* Implemented renameColumn functionality for each storage backend
core/src/main/java/com/scalar/db/api/* Added renameColumn method to Admin interface and TableMetadata.Builder
core/src/integration-test/java/* Added integration tests with proper database-specific limitations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 renameColumn functionality across various storage backends. The changes are extensive, touching the core Admin API, implementing the feature for Cassandra and JDBC, and disabling it for unsupported databases like CosmosDB and DynamoDB. The integration and unit tests have been updated accordingly. My review focuses on ensuring the atomicity of database schema modifications and improving test code maintainability. I've identified a critical issue in the JDBC implementation where schema and metadata updates are not performed atomically, which could lead to an inconsistent state. I've also noted an opportunity to reduce code duplication in the Cassandra integration tests.

@brfrn169 brfrn169 merged commit 4687f97 into 3 Sep 12, 2025
56 checks passed
@brfrn169 brfrn169 deleted the 3-pull-2990 branch September 12, 2025 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants