Skip to content

Conversation

@KodaiD
Copy link
Contributor

@KodaiD KodaiD commented Jun 17, 2025

Description

This PR fixes the table creation logic in the DynamoAdmin to create indexes when the table metadata includes them.

Currently, the table creation process with ifNotExists does not create indexes even if the table metadata specifies them. It can lead to inconsistencies between the actual table structure and its metadata.

To resolve this issue, this PR modifies the table creation logic to include index creation when the metadata specifies indexes.

Related issues and/or PRs

N/A

Changes made

  • Update createTableInternal method in DynamoAdmin to include index creation when the table metadata specifies indexes.
  • Extract the following methods to make it reusable:
    • createIndex
    • indexExists

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 KodaiD requested a review from Copilot June 17, 2025 03:49
@KodaiD KodaiD self-assigned this Jun 17, 2025
@KodaiD KodaiD added the bugfix label Jun 17, 2025
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 ensures that secondary indexes are created when a DynamoDB table already exists and ifNotExists is true in DynamoAdmin, and refactors index creation into reusable methods.

  • Adds logic in createTableInternal to loop through and create missing secondary indexes.
  • Extracts and overloads createIndex, moving table update and index-wait logic into a private method.
  • Implements indexExists to check index presence and refactors waiting logic.
Comments suppressed due to low confidence (3)

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

  • response.table().globalSecondaryIndexes() may be null when no global indexes are defined, causing a NullPointerException. Consider null-checking or defaulting to an empty list before iterating.
for (GlobalSecondaryIndexDescription globalSecondaryIndex : response.table().globalSecondaryIndexes()) {

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

  • The new index creation path when the table exists isn't covered by existing tests. Consider adding unit or integration tests for scenarios where ifNotExists is true and secondary indexes must be created.
} else if (!metadata.getSecondaryIndexNames().isEmpty()) {

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

  • [nitpick] Consider adding JavaDoc for this new overloaded createIndex method to clarify the role of each parameter, especially the ifNotExists flag, to improve readability and maintainability.
private void createIndex(Namespace namespace, String table, String columnName, DataType dataType, Long ru, boolean ifNotExists)

@KodaiD
Copy link
Contributor Author

KodaiD commented Jun 17, 2025

It will be better to solve the issue with another way. So I close this for now.

@KodaiD KodaiD closed this Jun 17, 2025
@brfrn169 brfrn169 deleted the fix-dynamo-table-creation branch July 8, 2025 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant