Skip to content

[RW-514] Joining to tables after LIMIT where possible in cohort materialization. #744

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Apr 19, 2018
Merged

[RW-514] Joining to tables after LIMIT where possible in cohort materialization. #744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
41a9dfa
Start
Apr 17, 2018
d35006e
Fixes
Apr 18, 2018
80a1c44
Removing imports
Apr 18, 2018
ff959b6
PR comments
Apr 19, 2018
e4563cd
Removing log
Apr 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
164 changes: 163 additions & 1 deletion api/src/bigquerytest/java/org/pmiops/workbench/cohorts/CohortMaterializationServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,9 @@ public void testMaterializeCohortObservationFilterObservationDatetimeLessThanOrE

@Test
public void testMaterializeCohortPersonConceptSelectColumns() {
// Here person is in the inner query; gender_concept, gender_concept.vocabulary,
// and gender_concept.vocabulary.vocabulary_concept are only referenced in the select clause
// and are in the outer query.
TableQuery tableQuery = new TableQuery();
tableQuery.setTableName("person");
tableQuery.setColumns(ImmutableList.of("person_id", "gender_concept.concept_name",
Expand Down Expand Up @@ -1136,6 +1139,166 @@ public void testMaterializeCohortPersonConceptSelectColumns() {
assertThat(response.getNextPageToken()).isNull();
}

@Test
public void testMaterializeCohortWhereInner() {
// person and gender_concept are in the inner query (since gender_concept is in the where clause.)
TableQuery tableQuery = new TableQuery();
tableQuery.setTableName("person");
tableQuery.setColumns(ImmutableList.of("person_id", "gender_concept.concept_name",
"gender_concept.vocabulary_id", "gender_concept.vocabulary.vocabulary_name",
"gender_concept.vocabulary.vocabulary_reference",
"gender_concept.vocabulary.vocabulary_concept.concept_name"));
ColumnFilter columnFilter1 = new ColumnFilter();
columnFilter1.setColumnName("person_id");
columnFilter1.setOperator(Operator.NOT_EQUAL);
columnFilter1.setValueNumber(new BigDecimal(2L));
ColumnFilter columnFilter2 = new ColumnFilter();
columnFilter2.setColumnName("gender_concept.vocabulary_id");
columnFilter2.setValue("Gender");
tableQuery.setFilters(makeAllOf(columnFilter1, columnFilter2));
FieldSet fieldSet = new FieldSet();
fieldSet.setTableQuery(tableQuery);
MaterializeCohortResponse response = cohortMaterializationService.materializeCohort(null,
SearchRequests.allGenders(), makeRequest(fieldSet, 1000));
ImmutableMap<String, Object> p1Map = ImmutableMap.<String, Object>builder()
.put("person_id", 1L)
.put("gender_concept.concept_name", "MALE")
.put("gender_concept.vocabulary_id", "Gender")
.put("gender_concept.vocabulary.vocabulary_name", "Gender vocabulary")
.put("gender_concept.vocabulary.vocabulary_reference", "Gender reference")
.put("gender_concept.vocabulary.vocabulary_concept.concept_name", "Gender vocabulary concept")
.build();
ImmutableMap<String, Object> p2Map = ImmutableMap.<String, Object>builder()
.put("person_id", 102246L)
.put("gender_concept.concept_name", "FEMALE")
.put("gender_concept.vocabulary_id", "Gender")
.put("gender_concept.vocabulary.vocabulary_name", "Gender vocabulary")
.put("gender_concept.vocabulary.vocabulary_reference", "Gender reference")
.put("gender_concept.vocabulary.vocabulary_concept.concept_name", "Gender vocabulary concept")
.build();
assertResults(response, p1Map, p2Map);
assertThat(response.getNextPageToken()).isNull();
}

@Test
public void testMaterializeCohortWhereNestedInner() {
// person and gender_concept.vocabulary are in the inner query, and thus so is gender_concept.
TableQuery tableQuery = new TableQuery();
tableQuery.setTableName("person");
tableQuery.setColumns(ImmutableList.of("person_id", "gender_concept.concept_name",
"gender_concept.vocabulary_id", "gender_concept.vocabulary.vocabulary_name",
"gender_concept.vocabulary.vocabulary_reference",
"gender_concept.vocabulary.vocabulary_concept.concept_name"));
ColumnFilter columnFilter1 = new ColumnFilter();
columnFilter1.setColumnName("person_id");
columnFilter1.setOperator(Operator.NOT_EQUAL);
columnFilter1.setValueNumber(new BigDecimal(2L));
ColumnFilter columnFilter2 = new ColumnFilter();
columnFilter2.setColumnName("gender_concept.vocabulary.vocabulary_name");
columnFilter2.setValue("Gender vocabulary");
tableQuery.setFilters(makeAllOf(columnFilter1, columnFilter2));
FieldSet fieldSet = new FieldSet();
fieldSet.setTableQuery(tableQuery);
MaterializeCohortResponse response = cohortMaterializationService.materializeCohort(null,
SearchRequests.allGenders(), makeRequest(fieldSet, 1000));
ImmutableMap<String, Object> p1Map = ImmutableMap.<String, Object>builder()
.put("person_id", 1L)
.put("gender_concept.concept_name", "MALE")
.put("gender_concept.vocabulary_id", "Gender")
.put("gender_concept.vocabulary.vocabulary_name", "Gender vocabulary")
.put("gender_concept.vocabulary.vocabulary_reference", "Gender reference")
.put("gender_concept.vocabulary.vocabulary_concept.concept_name", "Gender vocabulary concept")
.build();
ImmutableMap<String, Object> p2Map = ImmutableMap.<String, Object>builder()
.put("person_id", 102246L)
.put("gender_concept.concept_name", "FEMALE")
.put("gender_concept.vocabulary_id", "Gender")
.put("gender_concept.vocabulary.vocabulary_name", "Gender vocabulary")
.put("gender_concept.vocabulary.vocabulary_reference", "Gender reference")
.put("gender_concept.vocabulary.vocabulary_concept.concept_name", "Gender vocabulary concept")
.build();
assertResults(response, p1Map, p2Map);
assertThat(response.getNextPageToken()).isNull();
}

@Test
public void testMaterializeCohortOrderByInner() {
// person and gender_concept are in the inner query (since gender_concept is in order by).
TableQuery tableQuery = new TableQuery();
tableQuery.setTableName("person");
tableQuery.setColumns(ImmutableList.of("person_id", "gender_concept.concept_name",
"gender_concept.vocabulary_id", "gender_concept.vocabulary.vocabulary_name",
"gender_concept.vocabulary.vocabulary_reference",
"gender_concept.vocabulary.vocabulary_concept.concept_name"));
ColumnFilter columnFilter1 = new ColumnFilter();
columnFilter1.setColumnName("person_id");
columnFilter1.setOperator(Operator.NOT_EQUAL);
columnFilter1.setValueNumber(new BigDecimal(2L));
tableQuery.setFilters(makeResultFilters(columnFilter1));
tableQuery.setOrderBy(ImmutableList.of("gender_concept.concept_name"));
FieldSet fieldSet = new FieldSet();
fieldSet.setTableQuery(tableQuery);
MaterializeCohortResponse response = cohortMaterializationService.materializeCohort(null,
SearchRequests.allGenders(), makeRequest(fieldSet, 1000));
ImmutableMap<String, Object> p1Map = ImmutableMap.<String, Object>builder()
.put("person_id", 1L)
.put("gender_concept.concept_name", "MALE")
.put("gender_concept.vocabulary_id", "Gender")
.put("gender_concept.vocabulary.vocabulary_name", "Gender vocabulary")
.put("gender_concept.vocabulary.vocabulary_reference", "Gender reference")
.put("gender_concept.vocabulary.vocabulary_concept.concept_name", "Gender vocabulary concept")
.build();
ImmutableMap<String, Object> p2Map = ImmutableMap.<String, Object>builder()
.put("person_id", 102246L)
.put("gender_concept.concept_name", "FEMALE")
.put("gender_concept.vocabulary_id", "Gender")
.put("gender_concept.vocabulary.vocabulary_name", "Gender vocabulary")
.put("gender_concept.vocabulary.vocabulary_reference", "Gender reference")
.put("gender_concept.vocabulary.vocabulary_concept.concept_name", "Gender vocabulary concept")
.build();
assertResults(response, p2Map, p1Map);
assertThat(response.getNextPageToken()).isNull();
}

@Test
public void testMaterializeCohortOrderByNestedInner() {
// person and gender_concept.vocabulary are in the inner query, and thus so is gender_concept.
TableQuery tableQuery = new TableQuery();
tableQuery.setTableName("person");
tableQuery.setColumns(ImmutableList.of("person_id", "gender_concept.concept_name",
"gender_concept.vocabulary_id", "gender_concept.vocabulary.vocabulary_name",
"gender_concept.vocabulary.vocabulary_reference",
"gender_concept.vocabulary.vocabulary_concept.concept_name"));
ColumnFilter columnFilter1 = new ColumnFilter();
columnFilter1.setColumnName("person_id");
columnFilter1.setOperator(Operator.NOT_EQUAL);
columnFilter1.setValueNumber(new BigDecimal(2L));
tableQuery.setFilters(makeResultFilters(columnFilter1));
tableQuery.setOrderBy(ImmutableList.of("DESCENDING(gender_concept.vocabulary.vocabulary_name)", "person_id"));
FieldSet fieldSet = new FieldSet();
fieldSet.setTableQuery(tableQuery);
MaterializeCohortResponse response = cohortMaterializationService.materializeCohort(null,
SearchRequests.allGenders(), makeRequest(fieldSet, 1000));
ImmutableMap<String, Object> p1Map = ImmutableMap.<String, Object>builder()
.put("person_id", 1L)
.put("gender_concept.concept_name", "MALE")
.put("gender_concept.vocabulary_id", "Gender")
.put("gender_concept.vocabulary.vocabulary_name", "Gender vocabulary")
.put("gender_concept.vocabulary.vocabulary_reference", "Gender reference")
.put("gender_concept.vocabulary.vocabulary_concept.concept_name", "Gender vocabulary concept")
.build();
ImmutableMap<String, Object> p2Map = ImmutableMap.<String, Object>builder()
.put("person_id", 102246L)
.put("gender_concept.concept_name", "FEMALE")
.put("gender_concept.vocabulary_id", "Gender")
.put("gender_concept.vocabulary.vocabulary_name", "Gender vocabulary")
.put("gender_concept.vocabulary.vocabulary_reference", "Gender reference")
.put("gender_concept.vocabulary.vocabulary_concept.concept_name", "Gender vocabulary concept")
.build();
assertResults(response, p1Map, p2Map);
assertThat(response.getNextPageToken()).isNull();
}

@Test
public void testMaterializeCohortPersonConceptFilter() {
TableQuery tableQuery = new TableQuery();
Expand Down Expand Up @@ -1199,7 +1362,6 @@ public void testMaterializeAnnotationQueryWithPagination() {
assertThat(response2.getNextPageToken()).isNull();
}


private ResultFilters makeResultFilters(ColumnFilter columnFilter) {
ResultFilters result = new ResultFilters();
result.setColumnFilter(columnFilter);
Expand Down
5 changes: 4 additions & 1 deletion api/src/bigquerytest/resources/bigquery/cbdata/observation_data.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
"observation_source_value":"5",
"observation_source_concept_id":5,
"unit_source_value":"5",
"qualifier_source_value":"5"
"qualifier_source_value":"5",
"value_source_concept_id":6,
"value_source_value": "6",
"questionnaire_response_id":7
},
{
"observation_id":6,
Expand Down
15 changes: 15 additions & 0 deletions api/src/bigquerytest/resources/bigquery/schema/observation.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,5 +83,20 @@
"type": "string",
"name": "qualifier_source_value",
"mode": "nullable"
},
{
"type": "integer",
"name": "value_source_concept_id",
"mode": "nullable"
},
{
"type": "string",
"name": "value_source_value",
"mode": "nullable"
},
{
"type": "integer",
"name": "questionnaire_response_id",
"mode": "nullable"
}
]
20 changes: 12 additions & 8 deletions api/src/main/java/org/pmiops/workbench/cohortbuilder/CohortQueryBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,20 @@ public CohortQueryBuilder(DomainLookupService domainLookupService) {
public QueryJobConfiguration buildQuery(ParticipantCriteria participantCriteria,
String sqlTemplate, String endSql, String mainTable,
Map<String, QueryParameterValue> params) {
SearchRequest request = participantCriteria.getSearchRequest();
StringBuilder queryBuilder = new StringBuilder(sqlTemplate.replace("${mainTable}", mainTable));
addWhereClause(participantCriteria, mainTable, queryBuilder, params);
queryBuilder.append(endSql.replace("${mainTable}", mainTable));

return QueryJobConfiguration
.newBuilder(queryBuilder.toString())
.setNamedParameters(params)
.setUseLegacySql(false)
.build();
}

public void addWhereClause(ParticipantCriteria participantCriteria, String mainTable,
StringBuilder queryBuilder, Map<String, QueryParameterValue> params) {
SearchRequest request = participantCriteria.getSearchRequest();
if (request == null) {
queryBuilder.append(PERSON_ID_WHITELIST_TEMPLATE.replace("${mainTable}", mainTable));
params.put(PERSON_ID_WHITELIST_PARAM, QueryParameterValue.array(
Expand Down Expand Up @@ -81,13 +92,6 @@ public QueryJobConfiguration buildQuery(ParticipantCriteria participantCriteria,
}
queryBuilder.append(joiner.toString());
}
queryBuilder.append(endSql.replace("${mainTable}", mainTable));

return QueryJobConfiguration
.newBuilder(queryBuilder.toString())
.setNamedParameters(params)
.setUseLegacySql(false)
.build();
}

private StringJoiner buildQuery(List<SearchGroup> groups, String mainTable,
Expand Down
Loading