From 1d78a431a91e23df349f6f860ccbd500e702840c Mon Sep 17 00:00:00 2001 From: alex-odysseus Date: Fri, 26 Apr 2024 14:38:08 +0200 Subject: [PATCH 1/7] Retaining Cohort Covariates feature # Conflicts: # pom.xml # src/main/java/org/ohdsi/webapi/feasibility/FeasibilityStudyQueryBuilder.java # src/main/java/org/ohdsi/webapi/ircalc/IRAnalysisQueryBuilder.java --- src/main/java/org/ohdsi/webapi/Constants.java | 1 + .../CohortGenerationRequest.java | 17 +++++++++++++ .../CohortGenerationRequestBuilder.java | 24 ++++++++++++++++++- .../CohortGenerationUtils.java | 16 +++++++++---- .../GenerateCohortTasklet.java | 4 +++- .../FeasibilityStudyQueryBuilder.java | 2 +- .../GenerationCacheHelper.java | 2 +- .../webapi/ircalc/IRAnalysisQueryBuilder.java | 2 +- .../service/CohortDefinitionService.java | 6 ++--- .../service/CohortGenerationService.java | 16 +++++++------ 10 files changed, 70 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/Constants.java b/src/main/java/org/ohdsi/webapi/Constants.java index 2069ed108d..9da5d7af13 100644 --- a/src/main/java/org/ohdsi/webapi/Constants.java +++ b/src/main/java/org/ohdsi/webapi/Constants.java @@ -82,6 +82,7 @@ interface Params { String EXECUTABLE_FILE_NAME = "executableFilename"; String GENERATION_ID = "generation_id"; String DESIGN_HASH = "design_hash"; + String RETAIN_COHORT_COVARIATES = "retains_cohort_covariates"; } interface Variables { diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequest.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequest.java index 647fb9251e..2a2d4f7b81 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequest.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequest.java @@ -10,6 +10,8 @@ public class CohortGenerationRequest { private String sessionId; private String targetSchema; private Integer targetId; + private Boolean retainCohortCovariates; + public CohortGenerationRequest(CohortExpression expression, Source source, String sessionId, Integer targetId, String targetSchema) { @@ -19,6 +21,16 @@ public CohortGenerationRequest(CohortExpression expression, Source source, Strin this.targetId = targetId; this.targetSchema = targetSchema; } + + public CohortGenerationRequest(CohortExpression expression, Source source, String sessionId, Integer targetId, String targetSchema, Boolean retainCohortCovariates) { + + this.expression = expression; + this.source = source; + this.sessionId = sessionId; + this.targetId = targetId; + this.targetSchema = targetSchema; + this.retainCohortCovariates = retainCohortCovariates; + } public CohortExpression getExpression() { @@ -44,4 +56,9 @@ public Integer getTargetId() { return targetId; } + + public Boolean getRetainCohortCovariates() { + + return retainCohortCovariates; + } } diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java index d23e894812..526fad705b 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java @@ -10,12 +10,25 @@ public class CohortGenerationRequestBuilder { private String sessionId; private String targetSchema; private Integer targetId; - + private Boolean retainCohortCovariates; + public CohortGenerationRequestBuilder(String sessionId, String targetSchema) { this.sessionId = sessionId; this.targetSchema = targetSchema; } + + public CohortGenerationRequestBuilder(String sessionId, String targetSchema, Boolean retainCohortCovariates) { + + this.sessionId = sessionId; + this.targetSchema = targetSchema; + this.retainCohortCovariates = retainCohortCovariates; + } + + public Boolean getRetainCohortCovariates() { + + return retainCohortCovariates; + } public CohortGenerationRequestBuilder withSource(Source source) { @@ -43,4 +56,13 @@ public CohortGenerationRequest build() { return new CohortGenerationRequest(expression, source, sessionId, targetId, targetSchema); } + + public CohortGenerationRequest buildWithRetainCohortCovariates() { + + if (this.source == null || this.expression == null || this.targetId == null || this.retainCohortCovariates == null) { + throw new RuntimeException("CohortGenerationRequest should contain non-null expression, source and targetId"); + } + + return new CohortGenerationRequest(expression, source, sessionId, targetId, targetSchema, retainCohortCovariates); + } } diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java index 232466c464..91e6e4ddd9 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java @@ -1,7 +1,7 @@ package org.ohdsi.webapi.cohortdefinition; import org.apache.commons.lang3.StringUtils; - +import org.ohdsi.circe.cohortdefinition.CohortExpression; import org.ohdsi.circe.cohortdefinition.CohortExpressionQueryBuilder; import org.ohdsi.circe.cohortdefinition.InclusionRule; import org.ohdsi.sql.SqlRender; @@ -10,6 +10,7 @@ import org.ohdsi.webapi.source.Source; import org.ohdsi.webapi.util.SourceUtils; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.util.ObjectUtils; import java.util.Arrays; @@ -17,7 +18,7 @@ import static org.ohdsi.webapi.Constants.Params.TARGET_DATABASE_SCHEMA; import static org.ohdsi.webapi.Constants.Params.DESIGN_HASH; - +import static org.ohdsi.webapi.Constants.Params.RESULTS_DATABASE_SCHEMA; import static org.ohdsi.webapi.Constants.Tables.COHORT_CACHE; import static org.ohdsi.webapi.Constants.Tables.COHORT_CENSOR_STATS_CACHE; import static org.ohdsi.webapi.Constants.Tables.COHORT_INCLUSION_RESULT_CACHE; @@ -46,9 +47,11 @@ public static void insertInclusionRules(CohortDefinition cohortDef, Source sourc public static String[] buildGenerationSql(CohortGenerationRequest request) { Source source = request.getSource(); + CohortExpression expression = request.getExpression(); String cdmSchema = SourceUtils.getCdmQualifier(source); String vocabSchema = SourceUtils.getVocabQualifierOrNull(source); + String resultSchema = SourceUtils.getResultsQualifier(source); CohortExpressionQueryBuilder expressionQueryBuilder = new CohortExpressionQueryBuilder(); StringBuilder sqlBuilder = new StringBuilder(); @@ -59,10 +62,12 @@ public static String[] buildGenerationSql(CohortGenerationRequest request) { options.cdmSchema = cdmSchema; options.vocabularySchema = vocabSchema; options.generateStats = true; // always generate with stats + options.resultSchema = resultSchema; + options.retainCohortCovariates = !ObjectUtils.isEmpty(request.getRetainCohortCovariates()) && request.getRetainCohortCovariates(); // this field decides whether to retain cohort covariates final String oracleTempSchema = SourceUtils.getTempQualifier(source); - String expressionSql = expressionQueryBuilder.buildExpressionQuery(request.getExpression(), options); + String expressionSql = expressionQueryBuilder.buildExpressionQuery(expression, options); expressionSql = SqlRender.renderSql( expressionSql, new String[] {"target_cohort_table", @@ -82,11 +87,12 @@ public static String[] buildGenerationSql(CohortGenerationRequest request) { } ); sqlBuilder.append(expressionSql); +// expressionSql = expressionSql.replaceAll("@results_database_schema", request.getTargetSchema()); String renderedSql = SqlRender.renderSql( sqlBuilder.toString(), - new String[] {TARGET_DATABASE_SCHEMA}, - new String[]{request.getTargetSchema()} + new String[] {TARGET_DATABASE_SCHEMA, RESULTS_DATABASE_SCHEMA}, + new String[]{request.getTargetSchema(), resultSchema} ); String translatedSql = SqlTranslate.translateSql(renderedSql, source.getSourceDialect(), request.getSessionId(), oracleTempSchema); return SqlSplit.splitSql(translatedSql); diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/GenerateCohortTasklet.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/GenerateCohortTasklet.java index 47173e8b45..0343e1bc9c 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/GenerateCohortTasklet.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/GenerateCohortTasklet.java @@ -67,13 +67,15 @@ protected String[] prepareQueries(ChunkContext chunkContext, CancelableJdbcTempl Integer sourceId = Integer.parseInt(jobParams.get(SOURCE_ID).toString()); String targetSchema = jobParams.get(TARGET_DATABASE_SCHEMA).toString(); String sessionId = jobParams.getOrDefault(SESSION_ID, SessionUtils.sessionId()).toString(); + Boolean retainCohortCovariates = Boolean.valueOf(jobParams.get(RETAIN_COHORT_COVARIATES).toString()); CohortDefinition cohortDefinition = cohortDefinitionRepository.findOneWithDetail(cohortDefinitionId); Source source = sourceService.findBySourceId(sourceId); CohortGenerationRequestBuilder generationRequestBuilder = new CohortGenerationRequestBuilder( sessionId, - targetSchema + targetSchema, + retainCohortCovariates ); int designHash = this.generationCacheHelper.computeHash(cohortDefinition.getDetails().getExpression()); diff --git a/src/main/java/org/ohdsi/webapi/feasibility/FeasibilityStudyQueryBuilder.java b/src/main/java/org/ohdsi/webapi/feasibility/FeasibilityStudyQueryBuilder.java index e940c3706b..01ce4bcce1 100644 --- a/src/main/java/org/ohdsi/webapi/feasibility/FeasibilityStudyQueryBuilder.java +++ b/src/main/java/org/ohdsi/webapi/feasibility/FeasibilityStudyQueryBuilder.java @@ -76,7 +76,7 @@ private String getInclusionRuleInserts(FeasibilityStudy study) private String getInclusionRuleQuery(CriteriaGroup inclusionRule) { String resultSql = INCLUSION_RULE_QUERY_TEMPLATE; - String additionalCriteriaQuery = "\nJOIN (\n" + cohortExpressionQueryBuilder.getCriteriaGroupQuery(inclusionRule, "#primary_events") + ") AC on AC.event_id = pe.event_id"; + String additionalCriteriaQuery = "\nJOIN (\n" + cohortExpressionQueryBuilder.getCriteriaGroupQuery(inclusionRule, "#primary_events", null, false) + ") AC on AC.event_id = pe.event_id"; additionalCriteriaQuery = StringUtils.replace(additionalCriteriaQuery,"@indexId", "" + 0); resultSql = StringUtils.replace(resultSql, "@additionalCriteriaQuery", additionalCriteriaQuery); return resultSql; diff --git a/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java b/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java index 94a6ff1ac2..1bf8c53a3e 100644 --- a/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java +++ b/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java @@ -59,7 +59,7 @@ public CacheResult computeCacheIfAbsent(CohortDefinition cohortDefinition, Sourc .withExpression(cohortDefinition.getDetails().getExpressionObject()) .withSource(source) .withTargetId(designHash) - .build(); + .buildWithRetainCohortCovariates(); String[] sqls = CohortGenerationUtils.buildGenerationSql(cohortGenerationRequest); sqlExecutor.accept(designHash, sqls); cache = generationCacheService.cacheResults(CacheableGenerationType.COHORT, designHash, source.getSourceId()); diff --git a/src/main/java/org/ohdsi/webapi/ircalc/IRAnalysisQueryBuilder.java b/src/main/java/org/ohdsi/webapi/ircalc/IRAnalysisQueryBuilder.java index 9410d97d39..2ea3dcfc07 100644 --- a/src/main/java/org/ohdsi/webapi/ircalc/IRAnalysisQueryBuilder.java +++ b/src/main/java/org/ohdsi/webapi/ircalc/IRAnalysisQueryBuilder.java @@ -66,7 +66,7 @@ public IRAnalysisQueryBuilder(ObjectMapper objectMapper) { private String getStrataQuery(CriteriaGroup strataCriteria) { String resultSql = STRATA_QUERY_TEMPLATE; - String additionalCriteriaQuery = "\nJOIN (\n" + cohortExpressionQueryBuilder.getCriteriaGroupQuery(strataCriteria, "#analysis_events") + ") AC on AC.person_id = pe.person_id AND AC.event_id = pe.event_id"; + String additionalCriteriaQuery = "\nJOIN (\n" + cohortExpressionQueryBuilder.getCriteriaGroupQuery(strataCriteria, "#analysis_events", null, false) + ") AC on AC.person_id = pe.person_id AND AC.event_id = pe.event_id"; additionalCriteriaQuery = StringUtils.replace(additionalCriteriaQuery,"@indexId", "" + 0); resultSql = StringUtils.replace(resultSql, "@additionalCriteriaQuery", additionalCriteriaQuery); return resultSql; diff --git a/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java b/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java index 4de4e872e6..4a316f3b09 100644 --- a/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java +++ b/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java @@ -570,14 +570,14 @@ public CohortDTO saveCohortDefinition(@PathParam("id") final int id, CohortDTO d @Produces(MediaType.APPLICATION_JSON) @Path("/{id}/generate/{sourceKey}") @Transactional - public JobExecutionResource generateCohort(@PathParam("id") final int id, @PathParam("sourceKey") final String sourceKey) { + public JobExecutionResource generateCohort(@PathParam("id") final int id, @PathParam("sourceKey") final String sourceKey, @QueryParam("retainCohortCovariates") String retainCohortCovariates) { Source source = getSourceRepository().findBySourceKey(sourceKey); CohortDefinition currentDefinition = this.cohortDefinitionRepository.findOne(id); UserEntity user = userRepository.findByLogin(security.getSubject()); - return cohortGenerationService.generateCohortViaJob(user, currentDefinition, source); + return cohortGenerationService.generateCohortViaJob(user, currentDefinition, source, Boolean.parseBoolean(retainCohortCovariates)); } - + /** * Cancel a cohort generation task * diff --git a/src/main/java/org/ohdsi/webapi/service/CohortGenerationService.java b/src/main/java/org/ohdsi/webapi/service/CohortGenerationService.java index 89ec980407..bf84f1aa7e 100644 --- a/src/main/java/org/ohdsi/webapi/service/CohortGenerationService.java +++ b/src/main/java/org/ohdsi/webapi/service/CohortGenerationService.java @@ -41,6 +41,7 @@ import static org.ohdsi.webapi.Constants.Params.SESSION_ID; import static org.ohdsi.webapi.Constants.Params.SOURCE_ID; import static org.ohdsi.webapi.Constants.Params.TARGET_DATABASE_SCHEMA; +import static org.ohdsi.webapi.Constants.Params.RETAIN_COHORT_COVARIATES; @Component @DependsOn("flyway") @@ -71,7 +72,7 @@ public CohortGenerationService(CohortDefinitionRepository cohortDefinitionReposi this.generationCacheHelper = generationCacheHelper; } - public JobExecutionResource generateCohortViaJob(UserEntity userEntity, CohortDefinition cohortDefinition, Source source) { + public JobExecutionResource generateCohortViaJob(UserEntity userEntity, CohortDefinition cohortDefinition, Source source, Boolean retainCohortCovariates) { CohortGenerationInfo info = cohortDefinition.getGenerationInfoList().stream() .filter(val -> Objects.equals(val.getId().getSourceId(), source.getSourceId())).findFirst() @@ -86,10 +87,10 @@ public JobExecutionResource generateCohortViaJob(UserEntity userEntity, CohortDe cohortDefinitionRepository.save(cohortDefinition); - return runGenerateCohortJob(cohortDefinition, source); + return runGenerateCohortJob(cohortDefinition, source, retainCohortCovariates); } - private Job buildGenerateCohortJob(CohortDefinition cohortDefinition, Source source, JobParameters jobParameters) { + private Job buildGenerateCohortJob(CohortDefinition cohortDefinition, Source source, JobParameters jobParameters, Boolean retainCohortCovariates) { log.info("Beginning generate cohort for cohort definition id: {}", cohortDefinition.getId()); @@ -121,13 +122,13 @@ private Job buildGenerateCohortJob(CohortDefinition cohortDefinition, Source sou return generateJobBuilder.build(); } - private JobExecutionResource runGenerateCohortJob(CohortDefinition cohortDefinition, Source source) { - final JobParametersBuilder jobParametersBuilder = getJobParametersBuilder(source, cohortDefinition); - Job job = buildGenerateCohortJob(cohortDefinition, source, jobParametersBuilder.toJobParameters()); + private JobExecutionResource runGenerateCohortJob(CohortDefinition cohortDefinition, Source source, Boolean retainCohortCovariates) { + final JobParametersBuilder jobParametersBuilder = getJobParametersBuilder(source, cohortDefinition, retainCohortCovariates); + Job job = buildGenerateCohortJob(cohortDefinition, source, jobParametersBuilder.toJobParameters(), retainCohortCovariates); return jobService.runJob(job, jobParametersBuilder.toJobParameters()); } - private JobParametersBuilder getJobParametersBuilder(Source source, CohortDefinition cohortDefinition) { + private JobParametersBuilder getJobParametersBuilder(Source source, CohortDefinition cohortDefinition, Boolean retainCohortCovariates) { JobParametersBuilder builder = new JobParametersBuilder(); builder.addString(JOB_NAME, String.format("Generating cohort %d : %s (%s)", cohortDefinition.getId(), source.getSourceName(), source.getSourceKey())); @@ -136,6 +137,7 @@ private JobParametersBuilder getJobParametersBuilder(Source source, CohortDefini builder.addString(COHORT_DEFINITION_ID, String.valueOf(cohortDefinition.getId())); builder.addString(SOURCE_ID, String.valueOf(source.getSourceId())); builder.addString(GENERATE_STATS, Boolean.TRUE.toString()); + builder.addString(RETAIN_COHORT_COVARIATES, String.valueOf(retainCohortCovariates)); return builder; } From 886e6813f337d1a20ba899bb2eee76535f51ce0d Mon Sep 17 00:00:00 2001 From: alex-odysseus Date: Mon, 20 May 2024 14:39:30 +0200 Subject: [PATCH 2/7] Fixing an SQL generation issue where selecting from incorrect tables has been performed which don't have design_hash columns The cohort_details_xyz results schema table has cohort ID in its name for easier identification --- .../cohortdefinition/CohortGenerationRequest.java | 11 +++++++++-- .../CohortGenerationRequestBuilder.java | 9 ++++++++- .../cohortdefinition/CohortGenerationUtils.java | 9 ++++----- .../webapi/generationcache/GenerationCacheHelper.java | 1 + 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequest.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequest.java index 2a2d4f7b81..600f3db739 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequest.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequest.java @@ -11,7 +11,7 @@ public class CohortGenerationRequest { private String targetSchema; private Integer targetId; private Boolean retainCohortCovariates; - + private Integer cohortId; public CohortGenerationRequest(CohortExpression expression, Source source, String sessionId, Integer targetId, String targetSchema) { @@ -22,7 +22,8 @@ public CohortGenerationRequest(CohortExpression expression, Source source, Strin this.targetSchema = targetSchema; } - public CohortGenerationRequest(CohortExpression expression, Source source, String sessionId, Integer targetId, String targetSchema, Boolean retainCohortCovariates) { + public CohortGenerationRequest(CohortExpression expression, Source source, String sessionId, Integer targetId, + String targetSchema, Boolean retainCohortCovariates, Integer cohortId) { this.expression = expression; this.source = source; @@ -30,6 +31,7 @@ public CohortGenerationRequest(CohortExpression expression, Source source, Strin this.targetId = targetId; this.targetSchema = targetSchema; this.retainCohortCovariates = retainCohortCovariates; + this.cohortId = cohortId; } public CohortExpression getExpression() { @@ -61,4 +63,9 @@ public Boolean getRetainCohortCovariates() { return retainCohortCovariates; } + + public Integer getCohortId() { + + return cohortId; + } } diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java index 526fad705b..0cff180a01 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java @@ -10,6 +10,7 @@ public class CohortGenerationRequestBuilder { private String sessionId; private String targetSchema; private Integer targetId; + private Integer cohortId; private Boolean retainCohortCovariates; public CohortGenerationRequestBuilder(String sessionId, String targetSchema) { @@ -47,6 +48,11 @@ public CohortGenerationRequestBuilder withTargetId(Integer targetId) { this.targetId = targetId; return this; } + + public CohortGenerationRequestBuilder withCohortId(Integer cohortId) { + this.cohortId = cohortId; + return this; + } public CohortGenerationRequest build() { @@ -63,6 +69,7 @@ public CohortGenerationRequest buildWithRetainCohortCovariates() { throw new RuntimeException("CohortGenerationRequest should contain non-null expression, source and targetId"); } - return new CohortGenerationRequest(expression, source, sessionId, targetId, targetSchema, retainCohortCovariates); + return new CohortGenerationRequest(expression, source, sessionId, targetId, targetSchema, + retainCohortCovariates, cohortId); } } diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java index 91e6e4ddd9..544a2e7c2e 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java @@ -51,7 +51,6 @@ public static String[] buildGenerationSql(CohortGenerationRequest request) { String cdmSchema = SourceUtils.getCdmQualifier(source); String vocabSchema = SourceUtils.getVocabQualifierOrNull(source); - String resultSchema = SourceUtils.getResultsQualifier(source); CohortExpressionQueryBuilder expressionQueryBuilder = new CohortExpressionQueryBuilder(); StringBuilder sqlBuilder = new StringBuilder(); @@ -59,10 +58,10 @@ public static String[] buildGenerationSql(CohortGenerationRequest request) { CohortExpressionQueryBuilder.BuildExpressionQueryOptions options = new CohortExpressionQueryBuilder.BuildExpressionQueryOptions(); options.cohortIdFieldName = DESIGN_HASH; options.cohortId = request.getTargetId(); + options.resultCohortId = request.getCohortId(); options.cdmSchema = cdmSchema; options.vocabularySchema = vocabSchema; options.generateStats = true; // always generate with stats - options.resultSchema = resultSchema; options.retainCohortCovariates = !ObjectUtils.isEmpty(request.getRetainCohortCovariates()) && request.getRetainCohortCovariates(); // this field decides whether to retain cohort covariates final String oracleTempSchema = SourceUtils.getTempQualifier(source); @@ -86,13 +85,13 @@ public static String[] buildGenerationSql(CohortGenerationRequest request) { "@target_database_schema.cohort_inclusion" } ); + expressionSql = expressionSql.replaceAll("@results_database_schema", request.getTargetSchema()); sqlBuilder.append(expressionSql); -// expressionSql = expressionSql.replaceAll("@results_database_schema", request.getTargetSchema()); String renderedSql = SqlRender.renderSql( sqlBuilder.toString(), - new String[] {TARGET_DATABASE_SCHEMA, RESULTS_DATABASE_SCHEMA}, - new String[]{request.getTargetSchema(), resultSchema} + new String[] {TARGET_DATABASE_SCHEMA}, + new String[]{request.getTargetSchema()} ); String translatedSql = SqlTranslate.translateSql(renderedSql, source.getSourceDialect(), request.getSessionId(), oracleTempSchema); return SqlSplit.splitSql(translatedSql); diff --git a/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java b/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java index 1bf8c53a3e..e671fb8cae 100644 --- a/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java +++ b/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java @@ -59,6 +59,7 @@ public CacheResult computeCacheIfAbsent(CohortDefinition cohortDefinition, Sourc .withExpression(cohortDefinition.getDetails().getExpressionObject()) .withSource(source) .withTargetId(designHash) + .withCohortId(cohortDefinition.getId()) .buildWithRetainCohortCovariates(); String[] sqls = CohortGenerationUtils.buildGenerationSql(cohortGenerationRequest); sqlExecutor.accept(designHash, sqls); From 8170bd5c03a443532f8f8c011e78fc65a9cc1460 Mon Sep 17 00:00:00 2001 From: alex-odysseus Date: Thu, 25 Jul 2024 20:02:22 +0200 Subject: [PATCH 3/7] Addressing circe-be changes from Boolean to boolean # Conflicts: # src/main/java/org/ohdsi/webapi/feasibility/FeasibilityStudyQueryBuilder.java # src/main/java/org/ohdsi/webapi/ircalc/IRAnalysisQueryBuilder.java --- .../webapi/feasibility/FeasibilityStudyQueryBuilder.java | 6 +++--- .../org/ohdsi/webapi/ircalc/IRAnalysisQueryBuilder.java | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/feasibility/FeasibilityStudyQueryBuilder.java b/src/main/java/org/ohdsi/webapi/feasibility/FeasibilityStudyQueryBuilder.java index 01ce4bcce1..ea1f9da9e6 100644 --- a/src/main/java/org/ohdsi/webapi/feasibility/FeasibilityStudyQueryBuilder.java +++ b/src/main/java/org/ohdsi/webapi/feasibility/FeasibilityStudyQueryBuilder.java @@ -76,9 +76,9 @@ private String getInclusionRuleInserts(FeasibilityStudy study) private String getInclusionRuleQuery(CriteriaGroup inclusionRule) { String resultSql = INCLUSION_RULE_QUERY_TEMPLATE; - String additionalCriteriaQuery = "\nJOIN (\n" + cohortExpressionQueryBuilder.getCriteriaGroupQuery(inclusionRule, "#primary_events", null, false) + ") AC on AC.event_id = pe.event_id"; - additionalCriteriaQuery = StringUtils.replace(additionalCriteriaQuery,"@indexId", "" + 0); - resultSql = StringUtils.replace(resultSql, "@additionalCriteriaQuery", additionalCriteriaQuery); + String additionalCriteriaQuery = "\nJOIN (\n" + + cohortExpressionQueryBuilder.getCriteriaGroupQuery(inclusionRule, "#primary_events", false) + + ") AC on AC.event_id = pe.event_id"; additionalCriteriaQuery = StringUtils.replace(additionalCriteriaQuery,"@indexId", "" + 0); resultSql = StringUtils.replace(resultSql, "@additionalCriteriaQuery", additionalCriteriaQuery); return resultSql; } diff --git a/src/main/java/org/ohdsi/webapi/ircalc/IRAnalysisQueryBuilder.java b/src/main/java/org/ohdsi/webapi/ircalc/IRAnalysisQueryBuilder.java index 2ea3dcfc07..1ecadb4383 100644 --- a/src/main/java/org/ohdsi/webapi/ircalc/IRAnalysisQueryBuilder.java +++ b/src/main/java/org/ohdsi/webapi/ircalc/IRAnalysisQueryBuilder.java @@ -66,7 +66,9 @@ public IRAnalysisQueryBuilder(ObjectMapper objectMapper) { private String getStrataQuery(CriteriaGroup strataCriteria) { String resultSql = STRATA_QUERY_TEMPLATE; - String additionalCriteriaQuery = "\nJOIN (\n" + cohortExpressionQueryBuilder.getCriteriaGroupQuery(strataCriteria, "#analysis_events", null, false) + ") AC on AC.person_id = pe.person_id AND AC.event_id = pe.event_id"; + String additionalCriteriaQuery = "\nJOIN (\n" + + cohortExpressionQueryBuilder.getCriteriaGroupQuery(strataCriteria, "#analysis_events", false) + + ") AC on AC.person_id = pe.person_id AND AC.event_id = pe.event_id"; additionalCriteriaQuery = StringUtils.replace(additionalCriteriaQuery,"@indexId", "" + 0); resultSql = StringUtils.replace(resultSql, "@additionalCriteriaQuery", additionalCriteriaQuery); return resultSql; From 787e805d99f6ba69aa72adc8cfad32ff2ab73530 Mon Sep 17 00:00:00 2001 From: oleg-odysseus Date: Thu, 4 Jul 2024 12:33:57 +0200 Subject: [PATCH 4/7] [ATL-11] Added saving of the covariates checkbox state --- .../webapi/cohortdefinition/CohortGenerationInfo.java | 10 ++++++++++ ...nerationInfoToCohortGenerationInfoDTOConverter.java | 1 + .../cohortdefinition/dto/CohortGenerationInfoDTO.java | 10 ++++++++++ .../ohdsi/webapi/service/CohortGenerationService.java | 4 ++-- ...00000__extend_cohort_generation_info_covariates.sql | 1 + 5 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 src/main/resources/db/migration/postgresql/V2.14.0.20240704000000__extend_cohort_generation_info_covariates.sql diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationInfo.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationInfo.java index f79d3fd1db..feb774846d 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationInfo.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationInfo.java @@ -84,6 +84,8 @@ public CohortGenerationInfo(CohortDefinition definition, Integer sourceId) @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "created_by_id") private UserEntity createdBy; + @Column(name = "is_choose_covariates") + private boolean isChooseCovariates; public CohortGenerationInfoId getId() { return id; @@ -187,4 +189,12 @@ public void setCreatedBy(UserEntity createdBy) { public UserEntity getCreatedBy() { return createdBy; } + + public boolean isChooseCovariates() { + return isChooseCovariates; + } + + public void setChooseCovariates(boolean chooseCovariates) { + isChooseCovariates = chooseCovariates; + } } diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/converter/CohortGenerationInfoToCohortGenerationInfoDTOConverter.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/converter/CohortGenerationInfoToCohortGenerationInfoDTOConverter.java index 2e8ede00b5..4436d50531 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/converter/CohortGenerationInfoToCohortGenerationInfoDTOConverter.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/converter/CohortGenerationInfoToCohortGenerationInfoDTOConverter.java @@ -22,6 +22,7 @@ public CohortGenerationInfoDTO convert(CohortGenerationInfo info) { dto.setStartTime(info.getStartTime()); dto.setStatus(info.getStatus()); dto.setIsValid(info.isIsValid()); + dto.setChooseCovariates(info.isChooseCovariates()); return dto; } diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/dto/CohortGenerationInfoDTO.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/dto/CohortGenerationInfoDTO.java index f3611a1d61..31d614f168 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/dto/CohortGenerationInfoDTO.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/dto/CohortGenerationInfoDTO.java @@ -45,6 +45,8 @@ public class CohortGenerationInfoDTO { private UserDTO createdBy; + private boolean isChooseCovariates; + public CohortGenerationInfoId getId() { return id; } @@ -124,4 +126,12 @@ public UserDTO getCreatedBy() { public void setCreatedBy(UserDTO createdBy) { this.createdBy = createdBy; } + + public boolean isChooseCovariates() { + return isChooseCovariates; + } + + public void setChooseCovariates(boolean chooseCovariates) { + isChooseCovariates = chooseCovariates; + } } diff --git a/src/main/java/org/ohdsi/webapi/service/CohortGenerationService.java b/src/main/java/org/ohdsi/webapi/service/CohortGenerationService.java index bf84f1aa7e..df5abfd3c3 100644 --- a/src/main/java/org/ohdsi/webapi/service/CohortGenerationService.java +++ b/src/main/java/org/ohdsi/webapi/service/CohortGenerationService.java @@ -11,7 +11,6 @@ import org.ohdsi.webapi.job.GeneratesNotification; import org.ohdsi.webapi.job.JobExecutionResource; import org.ohdsi.webapi.shiro.Entities.UserEntity; -import org.ohdsi.webapi.shiro.Entities.UserRepository; import org.ohdsi.webapi.source.Source; import org.ohdsi.webapi.source.SourceService; import org.ohdsi.webapi.util.SessionUtils; @@ -38,10 +37,10 @@ import static org.ohdsi.webapi.Constants.Params.COHORT_DEFINITION_ID; import static org.ohdsi.webapi.Constants.Params.GENERATE_STATS; import static org.ohdsi.webapi.Constants.Params.JOB_NAME; +import static org.ohdsi.webapi.Constants.Params.RETAIN_COHORT_COVARIATES; import static org.ohdsi.webapi.Constants.Params.SESSION_ID; import static org.ohdsi.webapi.Constants.Params.SOURCE_ID; import static org.ohdsi.webapi.Constants.Params.TARGET_DATABASE_SCHEMA; -import static org.ohdsi.webapi.Constants.Params.RETAIN_COHORT_COVARIATES; @Component @DependsOn("flyway") @@ -79,6 +78,7 @@ public JobExecutionResource generateCohortViaJob(UserEntity userEntity, CohortDe .orElse(new CohortGenerationInfo(cohortDefinition, source.getSourceId())); info.setCreatedBy(userEntity); + info.setChooseCovariates(retainCohortCovariates); cohortDefinition.getGenerationInfoList().add(info); diff --git a/src/main/resources/db/migration/postgresql/V2.14.0.20240704000000__extend_cohort_generation_info_covariates.sql b/src/main/resources/db/migration/postgresql/V2.14.0.20240704000000__extend_cohort_generation_info_covariates.sql new file mode 100644 index 0000000000..3cd9d924a7 --- /dev/null +++ b/src/main/resources/db/migration/postgresql/V2.14.0.20240704000000__extend_cohort_generation_info_covariates.sql @@ -0,0 +1 @@ +ALTER TABLE ${ohdsiSchema}.cohort_generation_info ADD is_choose_covariates BOOLEAN NOT NULL DEFAULT FALSE; \ No newline at end of file From 3f312772fd1f0aac158bb21e71cec81de0d42a9d Mon Sep 17 00:00:00 2001 From: alex-odysseus Date: Thu, 24 Oct 2024 15:17:37 +0200 Subject: [PATCH 5/7] Don't use the Cohorts cache when the retain Cohort covariates option is switched on # Conflicts: # src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java --- .../cohortdefinition/CohortGenerationRequestBuilder.java | 3 +++ .../ohdsi/webapi/generationcache/GenerationCacheHelper.java | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java index 0cff180a01..b88a661485 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java @@ -71,5 +71,8 @@ public CohortGenerationRequest buildWithRetainCohortCovariates() { return new CohortGenerationRequest(expression, source, sessionId, targetId, targetSchema, retainCohortCovariates, cohortId); + + public boolean hasRetainCohortCovariates() { + return retainCohortCovariates != null ? retainCohortCovariates.booleanValue() : false; } } diff --git a/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java b/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java index e671fb8cae..32cca80acf 100644 --- a/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java +++ b/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java @@ -51,8 +51,9 @@ public CacheResult computeCacheIfAbsent(CohortDefinition cohortDefinition, Sourc return transactionTemplateRequiresNew.execute(s -> { log.info("Retrieves or invalidates cache for cohort id = {}", cohortDefinition.getId()); GenerationCache cache = generationCacheService.getCacheOrEraseInvalid(type, designHash, source.getSourceId()); - if (cache == null) { - log.info("Cache is absent for cohort id = {}. Calculating with design hash = {}", cohortDefinition.getId(), designHash); + if (cache == null || requestBuilder.hasRetainCohortCovariates()) { + String messagePrefix = (cache == null ? "Cache is absent" : "Cache will not be used because the retain cohort covariates option is switched on"); + log.info(messagePrefix + " for cohort id = {}. Calculating with design hash = {}", cohortDefinition.getId(), designHash); // Ensure that there are no records in results schema with which we could mess up generationCacheService.removeCache(type, source, designHash); CohortGenerationRequest cohortGenerationRequest = requestBuilder From 1637ad9769ab7e8ad7121e0bb065cd9d9bb3fd2d Mon Sep 17 00:00:00 2001 From: alex-odysseus Date: Fri, 25 Oct 2024 20:47:04 +0200 Subject: [PATCH 6/7] Forcing cache entry invalidation by a getter after deletion --- .../ohdsi/webapi/generationcache/GenerationCacheHelper.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java b/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java index 32cca80acf..e45495e149 100644 --- a/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java +++ b/src/main/java/org/ohdsi/webapi/generationcache/GenerationCacheHelper.java @@ -56,6 +56,9 @@ public CacheResult computeCacheIfAbsent(CohortDefinition cohortDefinition, Sourc log.info(messagePrefix + " for cohort id = {}. Calculating with design hash = {}", cohortDefinition.getId(), designHash); // Ensure that there are no records in results schema with which we could mess up generationCacheService.removeCache(type, source, designHash); + // the line below forces a cached entry to be really deleted and it is a bit unclear why this line was even present as the cache had to be null anyway + // without it there is a constraint violation exception when there was a cache entry present and the retain covariates option is on + GenerationCache cachedResultsStillPresent = generationCacheService.getCacheOrEraseInvalid(type, designHash, source.getSourceId()); CohortGenerationRequest cohortGenerationRequest = requestBuilder .withExpression(cohortDefinition.getDetails().getExpressionObject()) .withSource(source) From 20e240a201dcbced87934f2d36a48b0cfa400fe7 Mon Sep 17 00:00:00 2001 From: alex-odysseus Date: Thu, 21 Nov 2024 19:44:25 +0100 Subject: [PATCH 7/7] Fixing issues after merging --- pom.xml | 2 +- .../cohortdefinition/CohortGenerationRequestBuilder.java | 1 + .../ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java | 1 - .../org/ohdsi/webapi/service/CohortGenerationService.java | 4 ++-- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index f82a63ccb3..be957db832 100644 --- a/pom.xml +++ b/pom.xml @@ -28,7 +28,7 @@ 1.5 - 1.11.2 + 1.11.4-SNAPSHOT 2.14 1.16.1 3.1.2 diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java index b88a661485..a958d49ee9 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationRequestBuilder.java @@ -71,6 +71,7 @@ public CohortGenerationRequest buildWithRetainCohortCovariates() { return new CohortGenerationRequest(expression, source, sessionId, targetId, targetSchema, retainCohortCovariates, cohortId); + } public boolean hasRetainCohortCovariates() { return retainCohortCovariates != null ? retainCohortCovariates.booleanValue() : false; diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java index 544a2e7c2e..2bdfbf0f94 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortGenerationUtils.java @@ -12,7 +12,6 @@ import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.util.ObjectUtils; - import java.util.Arrays; import java.util.List; diff --git a/src/main/java/org/ohdsi/webapi/service/CohortGenerationService.java b/src/main/java/org/ohdsi/webapi/service/CohortGenerationService.java index df5abfd3c3..8f9964fc27 100644 --- a/src/main/java/org/ohdsi/webapi/service/CohortGenerationService.java +++ b/src/main/java/org/ohdsi/webapi/service/CohortGenerationService.java @@ -90,7 +90,7 @@ public JobExecutionResource generateCohortViaJob(UserEntity userEntity, CohortDe return runGenerateCohortJob(cohortDefinition, source, retainCohortCovariates); } - private Job buildGenerateCohortJob(CohortDefinition cohortDefinition, Source source, JobParameters jobParameters, Boolean retainCohortCovariates) { + private Job buildGenerateCohortJob(CohortDefinition cohortDefinition, Source source, JobParameters jobParameters) { log.info("Beginning generate cohort for cohort definition id: {}", cohortDefinition.getId()); @@ -124,7 +124,7 @@ private Job buildGenerateCohortJob(CohortDefinition cohortDefinition, Source sou private JobExecutionResource runGenerateCohortJob(CohortDefinition cohortDefinition, Source source, Boolean retainCohortCovariates) { final JobParametersBuilder jobParametersBuilder = getJobParametersBuilder(source, cohortDefinition, retainCohortCovariates); - Job job = buildGenerateCohortJob(cohortDefinition, source, jobParametersBuilder.toJobParameters(), retainCohortCovariates); + Job job = buildGenerateCohortJob(cohortDefinition, source, jobParametersBuilder.toJobParameters()); return jobService.runJob(job, jobParametersBuilder.toJobParameters()); }