Revisit catalog usage override impl #2057
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…TargetTierDefinition Previous implementation relied on sql `concat` method which seem to prevent sql engine from using an index. The new implementation simplifies the logic by looking at the first block only (with the assumption) and all blocks within a tier should provide the matching tier (if this were not the case previous impl would anyway fail to return the entry).
…TargetUsageDefinition Previous implementation relied on sql `concat` method which seem to prevent sql engine from using an index. This is similar to what we did in 5eaef9b Note however that previous CatalogOverrideUsageTierSqlDao#getTargetUsageDefinition would return a list of `Long` indicating that we could have different tiers across different usage sections. I can't seem to understand the use case and how the code was handling this anyways, so after much thinking and additional tests, i removed this (what to seem unused)complexity.
…tTargetPhaseDefinition Previous implementation relied on sql `concat` method which seem to prevent sql engine from using an index. This is exactlyvsimilar to what we did in 6f3e16a incl the change of signature from a list of Long to a Long.
…TargetPlanDefinition Previous implementation relied on sql `concat` method which seem to prevent sql engine from using an index. This is exactlyvsimilar to what we did in 6f3e16a incl the change of signature from a list of Long to a Long.
pierre
approved these changes
Sep 18, 2024
| final CatalogOverridePlanPhaseSqlDao sqlDao = inTransactionHandle.attach(CatalogOverridePlanPhaseSqlDao.class); | ||
|
|
||
| final List<String> keys = new ArrayList<String>(); | ||
| if (overridePhaseDefinitionModelDaos.length == 0) { |
Member
There was a problem hiding this comment.
Nit: not required (loop wouldn't have any element to go through).
| final CatalogOverridePhaseUsageSqlDao sqlDao = inTransactionHandle.attach(CatalogOverridePhaseUsageSqlDao.class); | ||
|
|
||
| final List<String> keys = new ArrayList<String>(); | ||
| if (overrideUsageDefinitionModelDaos.length == 0) { |
| final CatalogOverrideUsageTierSqlDao sqlDao = inTransactionHandle.attach(CatalogOverrideUsageTierSqlDao.class); | ||
|
|
||
| final List<String> keys = new ArrayList<String>(); | ||
| if (overrideTierDefinitionModelDaos.length == 0) { |
| final CatalogOverrideTierBlockSqlDao sqlDao = inTransactionHandle.attach(CatalogOverrideTierBlockSqlDao.class); | ||
|
|
||
| final List<String> keys = new ArrayList<String>(); | ||
| if (overrideBlockDefinitionModelDaos.length == 0) { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR attempts to address some performance issues that were reported when a large number of entries have been inserted in the catalog tables to override some usage sections.
Additional tests have been written to minimize potential effect for the change on existing data but certainly not bullet proof. Details is in each commits.