Skip to content
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

IGNITE-24247 Indexes created with table must be created in AVAILABLE state #5113

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

xtern
Copy link
Contributor

@xtern xtern commented Jan 23, 2025

https://issues.apache.org/jira/browse/IGNITE-24247

  • Added UpdateContext that holds a set of table ids.
  • When processing CreateTableCommand, we add the table ID to the context.
  • When processing the CreateIndexCommand, we set the status of the index depending on whether there was a create table command for the corresponding table in the same batch.
  • Most of the files have been changed because the UpdateProducer interface has changed.

@xtern xtern force-pushed the ignite-24247 branch 3 times, most recently from cc914e9 to 440313a Compare January 24, 2025 13:57
@xtern xtern changed the title IGNITE-24247 IGNITE-24247 Indexes created with table must be created in AVAILABLE state Jan 24, 2025
@xtern xtern marked this pull request as ready for review January 24, 2025 13:58
for (UpdateEntry updates : command.get(catalog)) {
catalog = updates.applyUpdate(catalog, INITIAL_CAUSALITY_TOKEN);
for (UpdateEntry updates : command.get(updateContext)) {
updateContext = new UpdateContext(updates.applyUpdate(updateContext.catalog(), INITIAL_CAUSALITY_TOKEN));
Copy link
Member

@AMashenkov AMashenkov Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't reuse updateContext instance and call UpdateContext.updateCatalog() instead of creating a new objectt.

Comment on lines +488 to +489
int versionAfter = catalogManager.latestCatalogVersion();
assertThat(versionAfter, equalTo(versionBefore + 1));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, here we want to be sure index is not available for SQL in registered/building state.
So, we table and index should be created using separate catalog command commands.

However, your case fix make sense. Let's have both tests.

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.

2 participants