Skip to content

Commit a02db91

Browse files
lmsurprelmsurpre
authored
Merge pull request #1691 from IBM/issue-1669
Issue #1669 - fix composite search queries
2 parents 11e3d25 + eec446b commit a02db91

File tree

5 files changed

+84
-38
lines changed

5 files changed

+84
-38
lines changed

‎fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/JDBCQueryBuilder.java

+48-10
Original file line numberDiff line numberDiff line change
@@ -826,12 +826,17 @@ private void appendInnerSelect(StringBuilder whereClauseSegment, QueryParameter
826826
for (int componentNum = 1; componentNum <= components.size(); componentNum++) {
827827
String alias = chainedParmVar + "_p" + componentNum;
828828
QueryParameter component = components.get(componentNum - 1);
829+
String tableName = QuerySegmentAggregator.tableName(resourceTypeName, component);
830+
// Check if type is reference or token - composites still use the 'old' token values table (issue #1669)
831+
if (component.getType().equals(Type.REFERENCE) || component.getType().equals(Type.TOKEN)) {
832+
tableName = resourceTypeName + "_TOKEN_VALUES ";
833+
}
829834
whereClauseSegment
830-
.append(JOIN + QuerySegmentAggregator.tableName(resourceTypeName, component) + alias)
835+
.append(JOIN).append(tableName).append(alias)
831836
.append(ON)
832-
.append(chainedParmVar + ".COMP" + componentNum + QuerySegmentAggregator.abbr(component))
837+
.append(chainedParmVar).append(".COMP").append(componentNum).append(QuerySegmentAggregator.abbr(component))
833838
.append("=")
834-
.append(alias + ".ROW_ID");
839+
.append(alias).append(".ROW_ID");
835840
}
836841
}
837842
}
@@ -1582,15 +1587,48 @@ protected SqlQueryData processReverseChainedReferenceParm(Class<?> resourceType,
15821587
SqlQueryData sqlQueryData;
15831588
if (!"_id".equals(currentParm.getCode())) {
15841589
if (!chainedParmProcessed) {
1585-
// Build this join:
1586-
// @formatter:off
1587-
// JOIN <modifierTypeResourceName>_<type>_VALUES AS CPx
1588-
// ON CPx.LOGICAL_RESOURCE_ID = CLR<x-1>.LOGICAL_RESOURCE_ID
1589-
// AND
1590-
// @formatter:on
1591-
whereClauseSegment.append(JOIN).append(QuerySegmentAggregator.tableName(previousParm.getModifierResourceTypeName(), currentParm))
1590+
if (Type.COMPOSITE.equals(currentParm.getType())) {
1591+
// Build this join:
1592+
// @formatter:off
1593+
// JOIN <modifierTypeResourceName>_COMPOSITES AS CPx
1594+
// ON CPx.LOGICAL_RESOURCE_ID = CLR<x-1>.LOGICAL_RESOURCE_ID
1595+
// JOIN <modifierTypeResourceName>__<type>_VALUES AS CPx_px
1596+
// ON CPx.COMPx_<type> = CPx_px.ROW_ID
1597+
// JOIN <modifierTypeResourceName>__<type>_VALUES AS CPx_p<x+1>
1598+
// ON CPx.COMP<x+1>_<type> = CPx_p<x+1>.ROW_ID
1599+
// AND
1600+
// @formatter:on
1601+
whereClauseSegment.append(JOIN).append(previousParm.getModifierResourceTypeName()).append("_COMPOSITES")
1602+
.append(AS).append(chainedParmVar).append(ON).append(chainedParmVar).append(DOT).append(LOGICAL_RESOURCE_ID)
1603+
.append(EQ).append(prevChainedLogicalResourceVar).append(DOT).append(LOGICAL_RESOURCE_ID);
1604+
if (currentParm.getValues() != null && !currentParm.getValues().isEmpty()) {
1605+
QueryParameterValue queryParameterValue = currentParm.getValues().get(0);
1606+
List<QueryParameter> components = queryParameterValue.getComponent();
1607+
for (int componentNum = 1; componentNum <= components.size(); componentNum++) {
1608+
String alias = chainedParmVar + "_p" + componentNum;
1609+
QueryParameter component = components.get(componentNum - 1);
1610+
String tableName = QuerySegmentAggregator.tableName(previousParm.getModifierResourceTypeName(), component);
1611+
// Check if type is reference or token - composites still use the 'old' token values table (issue #1669)
1612+
if (component.getType().equals(Type.REFERENCE) || component.getType().equals(Type.TOKEN)) {
1613+
tableName = previousParm.getModifierResourceTypeName() + "_TOKEN_VALUES ";
1614+
}
1615+
whereClauseSegment.append(JOIN).append(tableName).append(alias).append(ON)
1616+
.append(chainedParmVar).append(".COMP").append(componentNum).append(QuerySegmentAggregator.abbr(component))
1617+
.append(EQ).append(alias).append(".ROW_ID");
1618+
}
1619+
}
1620+
whereClauseSegment.append(AND);
1621+
} else {
1622+
// Build this join:
1623+
// @formatter:off
1624+
// JOIN <modifierTypeResourceName>_<type>_VALUES AS CPx
1625+
// ON CPx.LOGICAL_RESOURCE_ID = CLR<x-1>.LOGICAL_RESOURCE_ID
1626+
// AND
1627+
// @formatter:on
1628+
whereClauseSegment.append(JOIN).append(QuerySegmentAggregator.tableName(previousParm.getModifierResourceTypeName(), currentParm))
15921629
.append(AS).append(chainedParmVar).append(ON).append(chainedParmVar).append(DOT).append(LOGICAL_RESOURCE_ID)
15931630
.append(EQ).append(prevChainedLogicalResourceVar).append(DOT).append(LOGICAL_RESOURCE_ID).append(AND);
1631+
}
15941632
}
15951633
// Build the rest: (CPx.PARAMETER_NAME_ID=<code-id> AND (CPx.<type>_VALUE=<valueCode>))
15961634
sqlQueryData = buildQueryParm(ModelSupport.getResourceType(previousParm.getModifierResourceTypeName()), currentParm, chainedParmVar);

‎fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java

+10-4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import static com.ibm.fhir.persistence.jdbc.JDBCConstants.AS;
1010
import static com.ibm.fhir.persistence.jdbc.JDBCConstants.COMBINED_RESULTS;
1111
import static com.ibm.fhir.persistence.jdbc.JDBCConstants.DEFAULT_ORDERING;
12+
import static com.ibm.fhir.persistence.jdbc.JDBCConstants.EQ;
1213
import static com.ibm.fhir.persistence.jdbc.JDBCConstants.FROM;
1314
import static com.ibm.fhir.persistence.jdbc.JDBCConstants.JOIN;
1415
import static com.ibm.fhir.persistence.jdbc.JDBCConstants.LEFT_PAREN;
@@ -610,11 +611,16 @@ protected void buildWhereClause(StringBuilder whereClause, String overrideType)
610611
for (int componentNum = 1; componentNum <= components.size(); componentNum++) {
611612
String alias = compositeAlias + "_p" + componentNum;
612613
QueryParameter component = components.get(componentNum - 1);
613-
whereClause.append(JOIN + tableName(overrideType, component) + alias)
614+
String tableName = tableName(overrideType, component);
615+
// Check if type is reference or token - composites still use the 'old' token values table (issue #1669)
616+
if (component.getType().equals(Type.REFERENCE) || component.getType().equals(Type.TOKEN)) {
617+
tableName = overrideType + "_TOKEN_VALUES ";
618+
}
619+
whereClause.append(JOIN).append(tableName).append(alias)
614620
.append(ON)
615-
.append(compositeAlias + ".COMP" + componentNum + abbr(component))
616-
.append("=")
617-
.append(alias + ".ROW_ID");
621+
.append(compositeAlias).append(".COMP").append(componentNum).append(abbr(component))
622+
.append(EQ)
623+
.append(alias).append(".ROW_ID");
618624
whereClauseSegment =
619625
whereClauseSegment.replaceAll(
620626
PARAMETER_TABLE_ALIAS + "_p" + componentNum + "\\.", alias + ".");

‎fhir-persistence-jdbc/src/test/java/testng.xml

+1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
</test>
3636
<test name="JDBCSearchTests">
3737
<classes>
38+
<class name="com.ibm.fhir.persistence.jdbc.search.test.JDBCSearchCompositeTest" />
3839
<class name="com.ibm.fhir.persistence.jdbc.search.test.JDBCSearchDateTest" />
3940
<class name="com.ibm.fhir.persistence.jdbc.search.test.JDBCSearchNumberTest" />
4041
<class name="com.ibm.fhir.persistence.jdbc.search.test.JDBCSearchQuantityTest" />

‎fhir-persistence/src/test/java/com/ibm/fhir/persistence/search/test/AbstractSearchCompositeTest.java

+15-13
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@
2525
*/
2626
public abstract class AbstractSearchCompositeTest extends AbstractPLSearchTest {
2727

28+
@Override
2829
protected Basic getBasicResource() throws Exception {
2930
return TestUtil.readExampleResource("json/ibm/basic/BasicComposite.json");
3031
}
3132

33+
@Override
3234
protected void setTenant() throws Exception {
3335
FHIRRequestContext.get().setTenantId("composite");
3436
}
@@ -43,15 +45,15 @@ public void testSearchToken_boolean() throws Exception {
4345
assertSearchDoesntReturnSavedResource("composite-boolean", "true$false");
4446
assertSearchDoesntReturnSavedResource("composite-boolean", "false$false");
4547
}
46-
48+
4749
@Test
4850
public void testSearchToken_boolean_or() throws Exception {
4951
assertSearchReturnsSavedResource("composite-boolean", "true$true,true$true");
5052
assertSearchReturnsSavedResource("composite-boolean", "false$false,true$true");
5153
assertSearchReturnsSavedResource("composite-boolean", "true$true,false$false");
5254
assertSearchDoesntReturnSavedResource("composite-boolean", "false$false,false$false");
5355
}
54-
56+
5557
@Test
5658
public void testSearchToken_boolean_chained() throws Exception {
5759
assertSearchReturnsComposition("subject:Basic.composite-boolean", "true$true");
@@ -115,7 +117,7 @@ public void testSearchQuantity_Quantity() throws Exception {
115117
@Test
116118
public void testSearchQuantity_Range() throws Exception {
117119
// Range is 5-10 seconds
118-
120+
119121
assertSearchReturnsSavedResource("composite-Range", "gt4||s$lt11||s");
120122
}
121123

@@ -125,21 +127,21 @@ public void testSearchQuantity_Range() throws Exception {
125127
@Test
126128
public void testSearchDate_date() throws Exception {
127129
// "date" is 2018-10-29
128-
130+
129131
assertSearchReturnsSavedResource("composite-date", "2018-10-29$2018-10-29");
130-
132+
131133
assertSearchDoesntReturnSavedResource("composite-date", "ne2018-10-29$2018-10-29");
132-
assertSearchDoesntReturnSavedResource("composite-date", "lt2018-10-29$2018-10-29");
133-
assertSearchDoesntReturnSavedResource("composite-date", "gt2018-10-29$2018-10-29");
134+
assertSearchReturnsSavedResource("composite-date", "lt2018-10-29$2018-10-29");
135+
assertSearchReturnsSavedResource("composite-date", "gt2018-10-29$2018-10-29");
134136
assertSearchReturnsSavedResource("composite-date", "le2018-10-29$2018-10-29");
135137
assertSearchReturnsSavedResource("composite-date", "ge2018-10-29$2018-10-29");
136138
assertSearchDoesntReturnSavedResource("composite-date", "sa2018-10-29$2018-10-29");
137139
assertSearchDoesntReturnSavedResource("composite-date", "eb2018-10-29$2018-10-29");
138140
assertSearchReturnsSavedResource("composite-date", "ap2018-10-29$2018-10-29");
139-
141+
140142
assertSearchDoesntReturnSavedResource("composite-date", "2018-10-29$ne2018-10-29");
141-
assertSearchDoesntReturnSavedResource("composite-date", "2018-10-29$lt2018-10-29");
142-
assertSearchDoesntReturnSavedResource("composite-date", "2018-10-29$gt2018-10-29");
143+
assertSearchReturnsSavedResource("composite-date", "2018-10-29$lt2018-10-29");
144+
assertSearchReturnsSavedResource("composite-date", "2018-10-29$gt2018-10-29");
143145
assertSearchReturnsSavedResource("composite-date", "2018-10-29$le2018-10-29");
144146
assertSearchReturnsSavedResource("composite-date", "2018-10-29$ge2018-10-29");
145147
assertSearchDoesntReturnSavedResource("composite-date", "2018-10-29$sa2018-10-29");
@@ -167,7 +169,7 @@ public void testSearchDate_date_revinclude() throws Exception {
167169
assertTrue(searchReturnsResource(Basic.class, queryParms, savedResource));
168170
assertTrue(searchReturnsResource(Basic.class, queryParms, composition));
169171
}
170-
172+
171173
@Test
172174
public void testSearchDate_date_or() throws Exception {
173175
assertSearchReturnsSavedResource("composite-date", "2018-10-29$2018-10-29,9999-01-01$2018-10-29");
@@ -179,7 +181,7 @@ public void testSearchDate_dateTime() throws Exception {
179181
// "dateTime" is 2018-10-29T17:12:00-04:00
180182
assertSearchReturnsSavedResource("composite-dateTime", "2018-10-29$2018-10-29");
181183
}
182-
184+
183185
@Test
184186
public void testSearchDate_instant() throws Exception {
185187
// "instant" is 2018-10-29T17:12:00-04:00
@@ -199,7 +201,7 @@ public void testSearchDate_Period() throws Exception {
199201
public void testSearchNumber_integer() throws Exception {
200202
assertSearchReturnsSavedResource("composite-integer", "12$12");
201203
}
202-
204+
203205
@Test
204206
public void testSearchNumber_decimal() throws Exception {
205207
// Targeted Value is 99.99

‎fhir-persistence/src/test/java/com/ibm/fhir/persistence/test/common/AbstractReverseChainTest.java

+10-11
Original file line numberDiff line numberDiff line change
@@ -342,17 +342,16 @@ public void testReverseChainSingleResultWithStringParm() throws Exception {
342342
* One observation is found, thus one encounter is returned.
343343
* @throws Exception
344344
*/
345-
// TODO: uncomment when issue #1669 is fixed.
346-
// @Test
347-
// public void testReverseChainSingleResultWithCompositeParm() throws Exception {
348-
// Map<String, List<String>> queryParms = new HashMap<String, List<String>>();
349-
// queryParms.put("_has:Observation:encounter:code-value-string", Collections.singletonList("code$test"));
350-
// List<Resource> resources = runQueryTest(Encounter.class, queryParms);
351-
// assertNotNull(resources);
352-
// assertEquals(1, resources.size());
353-
// assertEquals("Encounter", resources.get(0).getClass().getSimpleName());
354-
// assertEquals(savedEncounter1.getId(), resources.get(0).getId());
355-
// }
345+
@Test
346+
public void testReverseChainSingleResultWithCompositeParm() throws Exception {
347+
Map<String, List<String>> queryParms = new HashMap<String, List<String>>();
348+
queryParms.put("_has:Observation:encounter:code-value-string", Collections.singletonList("code$test"));
349+
List<Resource> resources = runQueryTest(Encounter.class, queryParms);
350+
assertNotNull(resources);
351+
assertEquals(1, resources.size());
352+
assertEquals("Encounter", resources.get(0).getClass().getSimpleName());
353+
assertEquals(savedEncounter1.getId(), resources.get(0).getId());
354+
}
356355

357356
private Reference reference(String reference) {
358357
return Reference.builder().reference(string(reference)).build();

0 commit comments

Comments
 (0)