Skip to content

Renaming performance indexes on upgrade#342

Merged
gilleain merged 24 commits intomainfrom
feature/renamePerformanceIndexes
Oct 2, 2025
Merged

Renaming performance indexes on upgrade#342
gilleain merged 24 commits intomainfrom
feature/renamePerformanceIndexes

Conversation

@BrunoSMonteiro74
Copy link
Copy Markdown
Contributor

No description provided.

…r, InlineTableUpgrader, OracleMetaDataProvider. #2

Refactor duplicated code.
@BrunoSMonteiro74 BrunoSMonteiro74 requested a review from jsimlo May 2, 2025 17:38
Changes that apply to more than one file:
- Remove ignoredIndexesMap
- Remove schemaResources from constructor
- Use Table.ignoredIndexes instead of a map for all tables
Files:
- AbstractSchemaChangeVisitor
- DatabaseMetaDataProvider - also add support for ignored indexes at table loading level
- GraphBasedUpgradeBuilder
- GraphBasedUpgradeSchemaChangeVisitor
- InlineTableUpgrader
- OracleMetaDataProvider
- Upgrade
- UpgradeTestHelper

Add ignoredIndexes() property:
- SchemaUtils.TableBuilder / Impl
- Table
- TableBean

Update tests to pass.
- Re-add schemaResources for constructor of InlineTableUpgrader and GraphBasedUpgradeSchemaChangeVisitor
Files:
- AbstractSchemaChangeVisitor
- GraphBasedUpgradeBuilder
- GraphBasedUpgradeSchemaChangeVisitor
- InlineTableUpgrader
- OracleMetaDataProvider
- Upgrade
- UpgradeTestHelper

Update tests to pass.
Comment thread morf-core/src/main/java/org/alfasoftware/morf/jdbc/DatabaseMetaDataProvider.java Outdated
Add ignoredIndexes to AdditionalMetadata.
Add getIgnoredIndexes/set and getIgnoredIndexesForTable to UpdateConfigAndContext.
Replace Set<String> exclusiveExecutionSteps parameter on GraphBasedUpgradeBuilder by UpdateConfigAndContext.
Read ignoredIndexes from AdditionalMetadata on section for healing where we have schemaResources open to avoid opening more than once.
Adapt code on GraphBasedUpgradeSchemaChangeVisitor and InlineTableUpgrader to use the UpdateConfigAndContext.getIgnoredIndexesForTable instead.
Adapt OracleMetaDataProvider to read ignored indexes into the owned map and expose ignoredIndexes()
Adapt PostgreSQLMetaDataProvider to read ignored indexes into the owned map and expose ignoredIndexes()
Adapt tests to changes.
@sonarqubecloud
Copy link
Copy Markdown

Comment thread morf-core/src/main/java/org/alfasoftware/morf/metadata/SchemaUtils.java Outdated
Comment thread morf-core/src/main/java/org/alfasoftware/morf/metadata/TableBean.java Outdated
PostgreSQLMetaDataProvider - way of calculating ignored indexes was changed. the indexes map table name is stored in lower case for consistency.
UpgradeConfigAndContext - the setIgnoredIndexes ensures the table name is lowercase in the map stored.
Remove unnecessary line endings on TableBean and SchemaUtils.

private final Supplier<Map<AName, RealName>> allIndexNames = Suppliers.memoize(this::loadAllIndexNames);
private final Supplier<Map<String, List<Index>>> ignoredIndexes = Suppliers.memoize(this::loadAllIgnoredIndexes);
private final Map<String, List<Index>> allIgnoredIndexes = Maps.newHashMap();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should definitely use Suppliers.memoize(this::loadAllIgnoredIndexes) for this.
That loadAllIgnoredIndexes() method can call allIndexNames.get() to populate allIgnoredIndexesTables.
I am not sure I like this side-effect allIgnoredIndexesTables population, but at least it is private to this class.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that change wont work. I had to change it to load in separate ways. the side effect here loads the ignored indexes on need only after allIndexNames.get() is read (only once) - that method loads the tables and sets allIgnoredIndexesTables.

...
private final Supplier<Map<AName, RealName>> allIndexNames = Suppliers.memoize(this::loadAllIndexNames);
...

public Map<String, List<Index>> ignoredIndexes() {
    // make sure allIgnoredIndexesTables is loaded.
    allIndexNames.get();
    if (allIgnoredIndexes.isEmpty() && !allIgnoredIndexesTables.isEmpty()) {
      loadAllIgnoredIndexes();
    }
    return allIgnoredIndexes;
  }

// make sure allIgnoredIndexesTables is loaded.
allIndexNames.get();
if (allIgnoredIndexes.isEmpty() && !allIgnoredIndexesTables.isEmpty()) {
loadAllIgnoredIndexes();
Copy link
Copy Markdown
Contributor

@jsimlo jsimlo Oct 1, 2025

Choose a reason for hiding this comment

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

There could be schemas with no ignored indexes, re-loading this again and again.
That is currently fairly little extra work; but it does create a potential tech pickle for us in the future.
Suppliers.memoize would solve this problem for you :)

Copy link
Copy Markdown
Contributor Author

@BrunoSMonteiro74 BrunoSMonteiro74 Oct 1, 2025

Choose a reason for hiding this comment

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

I don't seem to understand. The condition doesn't load it again and again.
allIndexNames.get() sets allIgnoredIndexTables to be not empty only and only if there are ignored indexes - only once.
On second passes it will never call loadAllIgnoredIndexes() because allIgnoredIndexes now is not empty.
It is a bit obscure, but when you think of it, it works efficiently.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Oct 2, 2025

@gilleain gilleain merged commit fd3cc16 into main Oct 2, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants