Repository navigation
Conversation
getBatchConceptRecordCount.sql selects and filters on cc.concept_id but
never declares the cc alias, so warming the CDM cache with
cache.achilles.usePersonCount=false fails on every dialect ("missing
FROM-clause entry for table cc" on PostgreSQL, "The multi-part identifier
could not be bound" on SQL Server). The person count variant already
declares the alias.
With cache.achilles.usePersonCount=false the person counts of cache entries are null, and unboxing them into a long[] threw a NullPointerException whenever an already cached concept was warmed again.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes #2561
Follow-up to #2517. With
cache.achilles.usePersonCount=false, CDM cache warming still fails after the placeholder rename in a9720e7. This PR fixes two problems on that path:getBatchConceptRecordCount.sqlreferencescc.concept_idwithout declaring theccalias. Warming fails withmissing FROM-clause entry for table "cc"(PostgreSQL) orThe multi-part identifier "cc.concept_id" could not be bound(SQL Server).CDMCacheService.warmcatches and logs the error, so nothing gets cached. The fix adds the alias, which makes the file matchgetBatchConceptRecordPersonCount.sql.CDMCacheBatchService.savecompares the new and cached counts by unboxing them into along[]. WithusePersonCount=falsethe person counts arenull, so re-warming a concept already incdm_cachethrows aNullPointerException. The fix compares the boxedLongvalues withArrays.equals(Object[], Object[]), which handles nulls.The default
usePersonCount=truepath is unchanged.Tests
CDMCacheServiceWarmTestrunsCDMCacheService.warmagainst the embedded PostgreSQL instance, with and without person counts. Before the fix, theusePersonCount=falsecase cached nothing because of the SQL error. It passes now.CDMCacheBatchServiceTestcovers saving entries without person counts over existing cache entries, both unchanged and changed. Both cases threw a NullPointerException before the fix.mvn -B test: 144 unit tests, 0 failures, 5 skipped. The integration tests that run in the same build all pass except 3 inVocabularyServiceIT(canGetConcept,canGetVocabularies,canGetDomainsreturn 404). Those 3 fail the same way on unmodified master.