Create index if not exists in repairTable of CosmosAdmin and DynamoAdmin #2800

KodaiD · 2025-06-20T01:33:58Z

Description

This PR fixes an issue where the repairTable operation in DynamoAdmin and CosmosAdmin did not create indexes when the table already existed. Also, to ensure the repairTable creates indexes when the table metadata specifies them, this PR adds tests for the repairTable.

Related issues and/or PRs

N/A

Changes made

Updated the repairTable method in DynamoAdmin and CosmosAdmin to create indexes when the table metadata specifies them.
Added unit tests for the repairTable method to ensure indexes are created when specified in the metadata.
Added an integration test for the repairTable method to ensure indexes are created when specified in the metadata.

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

Copilot

Pull Request Overview

This PR ensures that existing tables are updated with secondary indexes during repairTable operations across different storage backends and adds corresponding unit tests.

Added calls to create indexes when a table already exists in DynamoAdmin and CosmosAdmin
Exposed createIndex in JdbcAdmin for testing and added a parameterized JDBC test
Introduced new unit tests for index creation in JDBC, DynamoDB, Cosmos DB, and Cassandra

Reviewed Changes

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

Show a summary per file

File	Description
core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java	Made `createIndex` visible for testing
core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java	Added loop to create secondary indexes in `repairTable`
core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java	Added `updateIndexingPolicy` call in `repairTable`
core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java	(No production code change—needs index creation logic)
core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java	Added parameterized test for index creation
core/src/test/java/com/scalar/db/storage/dynamo/DynamoAdminTestBase.java	Added test for existing table index creation
core/src/test/java/com/scalar/db/storage/cosmos/CosmosAdminTest.java	Added test for existing container index creation
core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java	Added test for existing table index creation

Comments suppressed due to low confidence (3)

core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java:1344

The repairTable Javadoc should be updated to mention that it now also creates secondary indexes when the table already exists, reflecting the new loop over metadata.getSecondaryIndexNames().

      for (String indexColumnName : metadata.getSecondaryIndexNames()) {

core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java:591

Please update the method-level Javadoc for repairTable to note that it now calls updateIndexingPolicy to apply secondary indexes on existing containers.

      updateIndexingPolicy(namespace, table, metadata);

core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java:892

Ensure that the @VisibleForTesting annotation is imported (e.g., com.google.common.annotations.VisibleForTesting) or use your project’s standard testing-visibility annotation to avoid compilation issues.

  @VisibleForTesting

KodaiD · 2025-06-22T23:39:56Z

core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java

+        if (!indexExistsInternal(nonPrefixedNamespace, table, indexColumnName)) {
+          createIndex(nonPrefixedNamespace, table, indexColumnName, options);
+        }


Extending createIndex to createIndexIfNotExists did not seem straightforward, as an "Index already exists" exception is not defined in DynamoDB. Therefore, indexExistsInternal was prepared to execute createIndex only after confirming that the index does not exist.

KodaiD · 2025-06-22T23:43:00Z

core/src/test/java/com/scalar/db/storage/cosmos/CosmosAdminTest.java

+    // Existing container properties
+    CosmosContainerResponse response = mock(CosmosContainerResponse.class);
+    when(database.createContainerIfNotExists(table, "/concatenatedPartitionKey"))
+        .thenReturn(response);
+    CosmosContainerProperties properties = mock(CosmosContainerProperties.class);
+    when(response.getProperties()).thenReturn(properties);
+


These lines are prepared to handle the addition of updateIndexingPolicy.

KodaiD · 2025-06-22T23:43:27Z

core/src/test/java/com/scalar/db/storage/cosmos/CosmosAdminTest.java

+    // Existing container properties
+    CosmosContainerResponse response = mock(CosmosContainerResponse.class);
+    when(database.createContainerIfNotExists(table, "/concatenatedPartitionKey"))
+        .thenReturn(response);
+    CosmosContainerProperties properties = mock(CosmosContainerProperties.class);
+    when(response.getProperties()).thenReturn(properties);
+


KodaiD · 2025-06-22T23:43:37Z

core/src/test/java/com/scalar/db/storage/cosmos/CosmosAdminTest.java

+    // Existing container properties
+    CosmosContainerResponse response = mock(CosmosContainerResponse.class);
+    when(database.createContainerIfNotExists(table, "/concatenatedPartitionKey"))
+        .thenReturn(response);
+    CosmosContainerProperties properties = mock(CosmosContainerProperties.class);
+    when(response.getProperties()).thenReturn(properties);
+


KodaiD · 2025-06-22T23:46:23Z

...n-test/src/main/java/com/scalar/db/api/DistributedStorageAdminRepairIntegrationTestBase.java

+    // Arrange
+    admin.dropIndex(getNamespace(), getTable(), COL_NAME5);
+
+    // Act
+    admin.repairTable(getNamespace(), getTable(), getTableMetadata(), getCreationOptions());


Prepare a situation where the index does not exist in the actual table, but the index is specified in the table metadata given when repairTable is executed.

core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java

core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java

komamitsu

LGTM! 👍

Torch3333

LGTM, thank you!

feeblefakie

Although I left one suggestion for naming, LGTM! Thank you!

feeblefakie · 2025-06-25T01:10:38Z

core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java

        TableMetadata.newBuilder(tableMetadata).addSecondaryIndex(columnName).build());
  }

+  private boolean rawIndexExists(String nonPrefixedNamespace, String table, String indexName) {


Suggested change

private boolean rawIndexExists(String nonPrefixedNamespace, String table, String indexName) {

private boolean nativeIndexExists(String nonPrefixedNamespace, String table, String indexName) {

I felt raw sounds misleading. How about native?

Thank you for your comment! It seems that the prefix "Raw" is used to refer to things not managed by ScalarDB metadata, such as addRawColumnToTable. Therefore, the name rawIndexExists is being used for now.

Thanks! OK, let's keep it as is for this PR.

@brfrn169 It's very minor, but I think raw sounds misleading , so let's revisit the naming at some point.

brfrn169 · 2025-06-25T05:04:31Z

...n-test/src/main/java/com/scalar/db/api/DistributedStorageAdminRepairIntegrationTestBase.java

+    assertThatCode(() -> admin.dropIndex(getNamespace(), getTable(), COL_NAME5))
+        .doesNotThrowAnyException();


Does the fact that admin.dropIndex() doesn’t throw an exception really confirm that the index has been created?

@brfrn169 admin.dropIndex drops an index in the underlying storage and updates the metadata. Therefore, if the method succeeds, my thinking is that the index can be considered to have been deleted. What do you think?

brfrn169

LGTM! Thank you!

Create index if not exists in repairTable of CosmosAdmin and DynamoAdmin

948822d

KodaiD requested a review from Copilot June 20, 2025 01:33

KodaiD self-assigned this Jun 20, 2025

KodaiD added the bugfix label Jun 20, 2025

Copilot AI reviewed Jun 20, 2025

View reviewed changes

KodaiD added 5 commits June 20, 2025 14:24

Merge branch 'master' into fix-nosql-admin-repair

5fec84d

Replace createIndex with createIndexIfNotExists

8f3d453

Update unit test to handle createIndex with ifNotExists

0d11898

Merge branch 'master' into fix-nosql-admin-repair

da032bb

Add integration test

82354b9

KodaiD commented Jun 22, 2025

View reviewed changes

KodaiD marked this pull request as ready for review June 23, 2025 00:14

KodaiD requested review from Torch3333, brfrn169, feeblefakie and komamitsu June 23, 2025 00:15

komamitsu reviewed Jun 23, 2025

View reviewed changes

core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java Show resolved Hide resolved

core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java Outdated Show resolved Hide resolved

core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java Outdated Show resolved Hide resolved

Apply suggestions

3700df9

KodaiD requested a review from komamitsu June 23, 2025 05:21

komamitsu approved these changes Jun 23, 2025

View reviewed changes

Torch3333 approved these changes Jun 23, 2025

View reviewed changes

feeblefakie approved these changes Jun 25, 2025

View reviewed changes

brfrn169 reviewed Jun 25, 2025

View reviewed changes

KodaiD requested a review from brfrn169 June 25, 2025 10:31

brfrn169 approved these changes Jun 25, 2025

View reviewed changes

Merge branch 'master' into fix-nosql-admin-repair

7b700b6

brfrn169 merged commit 54ab4e6 into master Jun 26, 2025
57 checks passed

brfrn169 deleted the fix-nosql-admin-repair branch June 26, 2025 01:50

	private boolean rawIndexExists(String nonPrefixedNamespace, String table, String indexName) {
	private boolean nativeIndexExists(String nonPrefixedNamespace, String table, String indexName) {

		assertThatCode(() -> admin.dropIndex(getNamespace(), getTable(), COL_NAME5))
		.doesNotThrowAnyException();

Create index if not exists in repairTable of CosmosAdmin and DynamoAdmin #2800

Create index if not exists in repairTable of CosmosAdmin and DynamoAdmin #2800

Uh oh!

Conversation

KodaiD commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues and/or PRs

Changes made

Checklist

Additional notes (optional)

Release notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

komamitsu left a comment

Choose a reason for hiding this comment

Uh oh!

Torch3333 left a comment

Choose a reason for hiding this comment

Uh oh!

feeblefakie left a comment

Choose a reason for hiding this comment

Uh oh!

feeblefakie Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brfrn169 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

KodaiD commented Jun 20, 2025 •

edited

Loading

feeblefakie Jun 25, 2025 •

edited

Loading