Issue #1734 - support :in and :not-in modifiers for Token search

Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
LinuxForHealth · Mar 30, 2021 · c08e434 · c08e434
1 parent 11e4c50
commit c08e434
Show file tree

Hide file tree

Showing 15 changed files with 306 additions and 71 deletions.
diff --git a/docs/src/pages/Conformance.md b/docs/src/pages/Conformance.md
@@ -2,7 +2,7 @@
 layout: post
 title:  Conformance
 description: Notes on the Conformance of the IBM FHIR Server
-date:   2021-03-26 12:00:00 -0400
+date:   2021-03-30 12:00:00 -0400
 permalink: /conformance/
 ---
 
@@ -163,7 +163,7 @@ FHIR search modifiers are described at https://www.hl7.org/fhir/R4/search.html#m
 |String                    |`:exact`,`:contains`,`:missing`    |"starts with" search that is case-insensitive and accent-insensitive|
 |Reference                 |`:[type]`,`:missing`,`:identifier` |exact match search and targets are implicitly added|
 |URI                       |`:below`,`:above`,`:missing`       |exact match search|
-|Token                     |`:missing`,`:not`,`:of-type`       |exact match search|
+|Token                     |`:missing`,`:not`,`:of-type`,`:in`,`:not-in`       |exact match search|
 |Number                    |`:missing`                         |implicit range search (see http://hl7.org/fhir/R4/search.html#number)|
 |Date                      |`:missing`                         |implicit range search (see https://www.hl7.org/fhir/search.html#date)|
 |Quantity                  |`:missing`                         |implicit range search (see http://hl7.org/fhir/R4/search.html#quantity)|
@@ -172,7 +172,11 @@ FHIR search modifiers are described at https://www.hl7.org/fhir/R4/search.html#m
 
 Due to performance implications, the `:exact` modifier should be used for String search parameters when possible.
 
-The `:text` modifier, as well as the `:above`, `:below`, `:in`, and `:not-in` modifiers for Token search parameters, are not yet supported by the IBM FHIR server. Use of these modifiers will result in an HTTP 400 error with an OperationOutcome that describes the failure.
+The `:text`, `:above`, and `:below` modifiers for Token search parameters are not yet supported by the IBM FHIR server. Use of these modifiers will result in an HTTP 400 error with an OperationOutcome that describes the failure.
+
+The `:in` and `:not-in` modifiers for Token search parameters are supported, with the following restrictions:
+* The search parameter value must be an absolute URI that identifies a value set.
+* The referenced value set must exist in the FHIR registry and must be expandable.
 
 ### Search prefixes
 FHIR search prefixes are described at https://www.hl7.org/fhir/R4/search.html#prefix.

diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/JDBCConstants.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/JDBCConstants.java
@@ -114,6 +114,9 @@ public class JDBCConstants {
     // Db2 optimization hints
     public static final String SEARCH_REOPT = "search.reopt";
 
+    // Default code_system_id value
+    public static final String DEFAULT_TOKEN_SYSTEM = "default-token-system";
+
     /**
      * Calendar object to use while inserting Timestamp objects into the database.
      */
@@ -134,7 +137,7 @@ public class JDBCConstants {
         supportedModifiersMap.put(Type.STRING, Arrays.asList(Modifier.EXACT, Modifier.CONTAINS, Modifier.MISSING));
         supportedModifiersMap.put(Type.REFERENCE, Arrays.asList(Modifier.TYPE, Modifier.MISSING, Modifier.IDENTIFIER));
         supportedModifiersMap.put(Type.URI, Arrays.asList(Modifier.BELOW, Modifier.ABOVE, Modifier.MISSING));
-        supportedModifiersMap.put(Type.TOKEN, Arrays.asList(Modifier.MISSING, Modifier.NOT, Modifier.OF_TYPE));
+        supportedModifiersMap.put(Type.TOKEN, Arrays.asList(Modifier.MISSING, Modifier.NOT, Modifier.OF_TYPE, Modifier.IN, Modifier.NOT_IN));
         supportedModifiersMap.put(Type.NUMBER, Arrays.asList(Modifier.MISSING));
         supportedModifiersMap.put(Type.DATE, Arrays.asList(Modifier.MISSING));
         supportedModifiersMap.put(Type.QUANTITY, Arrays.asList(Modifier.MISSING));

diff --git a/...sistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/CodeSystemDAOImpl.java b/...sistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/CodeSystemDAOImpl.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2020
+ * (C) Copyright IBM Corp. 2020, 2021
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -28,8 +28,6 @@ public class CodeSystemDAOImpl implements CodeSystemDAO {
     private static final Logger log = Logger.getLogger(CodeSystemDAOImpl.class.getName());
     private static final String CLASSNAME = CodeSystemDAOImpl.class.getName();
 
-    public static final String DEFAULT_TOKEN_SYSTEM = "default-token-system";
-
     private static final String SQL_CALL_ADD_CODE_SYSTEM_ID = "CALL %s.add_code_system(?, ?)";
 
     private static final String SQL_SELECT_ALL_CODE_SYSTEMS = "SELECT CODE_SYSTEM_ID, CODE_SYSTEM_NAME FROM CODE_SYSTEMS";

diff --git a/...rsistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/ParameterDAOImpl.java b/...rsistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/ParameterDAOImpl.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2017,2020
+ * (C) Copyright IBM Corp. 2017,2021
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -15,6 +15,7 @@
 import javax.transaction.TransactionSynchronizationRegistry;
 
 import com.ibm.fhir.persistence.exception.FHIRPersistenceException;
+import com.ibm.fhir.persistence.jdbc.JDBCConstants;
 import com.ibm.fhir.persistence.jdbc.connection.FHIRDbFlavor;
 import com.ibm.fhir.persistence.jdbc.dao.api.CodeSystemDAO;
 import com.ibm.fhir.persistence.jdbc.dao.api.ParameterDAO;
@@ -39,8 +40,6 @@ public class ParameterDAOImpl extends FHIRDbDAOImpl implements ParameterDAO {
     private static final Logger log = Logger.getLogger(ParameterDAOImpl.class.getName());
     private static final String CLASSNAME = ParameterDAOImpl.class.getName();
 
-    public static final String DEFAULT_TOKEN_SYSTEM = "default-token-system";
-
     private Map<String, Integer> newParameterNameIds = new HashMap<>();
     private Map<String, Integer> newCodeSystemIds = new HashMap<>();
 
@@ -319,7 +318,7 @@ public int acquireCodeSystemId(String codeSystemName) throws FHIRPersistenceExce
 
         try {
             if (myCodeSystemName == null || myCodeSystemName.isEmpty()) {
-                myCodeSystemName = DEFAULT_TOKEN_SYSTEM;
+                myCodeSystemName = JDBCConstants.DEFAULT_TOKEN_SYSTEM;
             }
             codeSystemId = getCodeSystemIdFromCaches(myCodeSystemName);
 

diff --git a/...tence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/ParameterNameDAOImpl.java b/...tence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/ParameterNameDAOImpl.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2020
+ * (C) Copyright IBM Corp. 2019, 2021
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -29,8 +29,6 @@ public class ParameterNameDAOImpl implements ParameterNameDAO {
     private static final Logger log = Logger.getLogger(ParameterNameDAOImpl.class.getName());
     private static final String CLASSNAME = ParameterNameDAOImpl.class.getName();
 
-    public static final String DEFAULT_TOKEN_SYSTEM = "default-token-system";
-
     private static final String SQL_SELECT_ALL_SEARCH_PARAMETER_NAMES = "SELECT PARAMETER_NAME_ID, PARAMETER_NAME FROM PARAMETER_NAMES";
 
     private static final String SQL_SELECT_PARAMETER_NAME_ID = "SELECT PARAMETER_NAME_ID FROM PARAMETER_NAMES WHERE PARAMETER_NAME = ?";

diff --git a/...e-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/ParameterVisitorBatchDAO.java b/...e-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/ParameterVisitorBatchDAO.java
@@ -20,6 +20,7 @@
 import java.util.logging.Logger;
 
 import com.ibm.fhir.persistence.exception.FHIRPersistenceException;
+import com.ibm.fhir.persistence.jdbc.JDBCConstants;
 import com.ibm.fhir.persistence.jdbc.dao.api.IResourceReferenceDAO;
 import com.ibm.fhir.persistence.jdbc.dao.api.JDBCIdentityCache;
 import com.ibm.fhir.persistence.jdbc.dto.CompositeParmVal;
@@ -602,7 +603,7 @@ public void visit(ReferenceParmVal rpv) throws FHIRPersistenceException {
             rec = new ResourceTokenValueRec(parameterNameId, resourceType, resourceTypeId, logicalResourceId, refResourceType, refLogicalId, refVersion, this.currentCompositeId, isSystemParam);
         } else {
             // stored as a token with the default system
-            rec = new ResourceTokenValueRec(parameterNameId, resourceType, resourceTypeId, logicalResourceId, TokenParmVal.DEFAULT_TOKEN_SYSTEM, refLogicalId, this.currentCompositeId, isSystemParam);
+            rec = new ResourceTokenValueRec(parameterNameId, resourceType, resourceTypeId, logicalResourceId, JDBCConstants.DEFAULT_TOKEN_SYSTEM, refLogicalId, this.currentCompositeId, isSystemParam);
         }
 
         if (this.transactionData != null) {

diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dto/QuantityParmVal.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dto/QuantityParmVal.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2017,2019
+ * (C) Copyright IBM Corp. 2017,2021
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -9,6 +9,7 @@
 import java.math.BigDecimal;
 
 import com.ibm.fhir.persistence.exception.FHIRPersistenceException;
+import com.ibm.fhir.persistence.jdbc.JDBCConstants;
 
 /**
  * This class defines the Data Transfer Object representing a row in the X_QUANTITY_VALUES tables.
@@ -26,9 +27,6 @@ public class QuantityParmVal implements ExtractedParameterValue {
     // The SearchParameter base type. If "Resource", then this is a Resource-level attribute
     private String base;
 
-    // A default value for the token-system as the schema column is not null (simplifying queries)
-    public static final String DEFAULT_TOKEN_SYSTEM = "default-token-system";
-
     public QuantityParmVal() {
         super();
     }
@@ -51,7 +49,7 @@ public void setValueNumber(BigDecimal valueNumber) {
 
     public String getValueSystem() {
         if (valueSystem == null) {
-            return DEFAULT_TOKEN_SYSTEM;
+            return JDBCConstants.DEFAULT_TOKEN_SYSTEM;
         }
         return valueSystem;
     }

diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dto/TokenParmVal.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dto/TokenParmVal.java
@@ -1,12 +1,13 @@
 /*
- * (C) Copyright IBM Corp. 2017,2020
+ * (C) Copyright IBM Corp. 2017,2021
  *
  * SPDX-License-Identifier: Apache-2.0
  */
 
 package com.ibm.fhir.persistence.jdbc.dto;
 
 import com.ibm.fhir.persistence.exception.FHIRPersistenceException;
+import com.ibm.fhir.persistence.jdbc.JDBCConstants;
 
 /**
  * This class defines the Data Transfer Object representing a row in the X_TOKEN_VALUES tables.
@@ -21,9 +22,6 @@ public class TokenParmVal implements ExtractedParameterValue {
     // The SearchParameter base type. If "Resource", then this is a Resource-level attribute
     private String base;
 
-    // A default value for the token-system as the schema column is not null (simplifying queries)
-    public static final String DEFAULT_TOKEN_SYSTEM = "default-token-system";
-
     public TokenParmVal() {
         super();
     }
@@ -44,7 +42,7 @@ public String getName() {
 
     public String getValueSystem() {
         if (valueSystem == null) {
-            return DEFAULT_TOKEN_SYSTEM;
+            return JDBCConstants.DEFAULT_TOKEN_SYSTEM;
         }
         return valueSystem;
     }

diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/JDBCQueryBuilder.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/JDBCQueryBuilder.java
@@ -66,6 +66,7 @@
 import com.ibm.fhir.model.util.ModelSupport;
 import com.ibm.fhir.persistence.exception.FHIRPersistenceException;
 import com.ibm.fhir.persistence.exception.FHIRPersistenceNotSupportedException;
+import com.ibm.fhir.persistence.jdbc.JDBCConstants;
 import com.ibm.fhir.persistence.jdbc.connection.QueryHints;
 import com.ibm.fhir.persistence.jdbc.dao.api.JDBCIdentityCache;
 import com.ibm.fhir.persistence.jdbc.dao.api.ParameterDAO;
@@ -90,6 +91,7 @@
 import com.ibm.fhir.search.parameters.QueryParameter;
 import com.ibm.fhir.search.parameters.QueryParameterValue;
 import com.ibm.fhir.search.util.SearchUtil;
+import com.ibm.fhir.term.util.ValueSetSupport;
 
 /**
  * This is the JDBC implementation of a query builder for the IBM FHIR Server
@@ -1224,36 +1226,42 @@ private SqlQueryData processTokenParm(Class<?> resourceType, QueryParameter quer
                 }
 
                 whereClauseSegment.append(LEFT_PAREN);
-                // Include code
-                whereClauseSegment.append(tableAlias + DOT).append(TOKEN_VALUE).append(operator).append(BIND_VAR);
-                bindVariables.add(SqlParameterEncoder.encode(value.getValueCode()));
-
-                // Include system if present.
-                if (value.getValueSystem() != null && !value.getValueSystem().isEmpty()) {
-                    Long commonTokenValueId = null;
-                    if (NE.equals(operator)) {
-                        whereClauseSegment.append(OR);
-                    } else {
-                        whereClauseSegment.append(AND);
-
-                        // use #1929 optimization if we can
-                        commonTokenValueId = getCommonTokenValueId(value.getValueSystem(), value.getValueCode());
-                    }
+
+                if (Modifier.IN.equals(queryParm.getModifier()) || Modifier.NOT_IN.equals(queryParm.getModifier())) {
+                    populateValueSetCodesSubSegment(whereClauseSegment, value.getValueCode(), tableAlias);
+                } else {
+                    // Include code
+                    whereClauseSegment.append(tableAlias + DOT).append(TOKEN_VALUE).append(operator).append(BIND_VAR);
+                    bindVariables.add(SqlParameterEncoder.encode(value.getValueCode()));
+
+                    // Include system if present.
+                    if (value.getValueSystem() != null && !value.getValueSystem().isEmpty()) {
+                        Long commonTokenValueId = null;
+                        if (NE.equals(operator)) {
+                            whereClauseSegment.append(OR);
+                        } else {
+                            whereClauseSegment.append(AND);
+
+                            // use #1929 optimization if we can
+                            commonTokenValueId = getCommonTokenValueId(value.getValueSystem(), value.getValueCode());
+                        }
 
-                    if (commonTokenValueId != null) {
-                        // #1929 improves cardinality estimation
-                        // resulting in far better execution plans for many search queries. Because COMMON_TOKEN_VALUE_ID
-                        // is the primary key for the common_token_values table, we don't need the CODE_SYSTEM_ID = ? predicate.
-                        whereClauseSegment.append(tableAlias).append(DOT).append(COMMON_TOKEN_VALUE_ID).append(EQ)
+                        if (commonTokenValueId != null) {
+                            // #1929 improves cardinality estimation
+                            // resulting in far better execution plans for many search queries. Because COMMON_TOKEN_VALUE_ID
+                            // is the primary key for the common_token_values table, we don't need the CODE_SYSTEM_ID = ? predicate.
+                            whereClauseSegment.append(tableAlias).append(DOT).append(COMMON_TOKEN_VALUE_ID).append(EQ)
                             .append(commonTokenValueId);
-                    } else {
-                        // common token value not found so we can't use the optimization. Filter the code-system-id
-                        // instead, which ends up being the logical equivalent.
-                        Integer codeSystemId = identityCache.getCodeSystemId(value.getValueSystem());
-                        whereClauseSegment.append(tableAlias).append(DOT).append(CODE_SYSTEM_ID).append(operator)
-                        .append(nullCheck(codeSystemId));
+                        } else {
+                            // common token value not found so we can't use the optimization. Filter the code-system-id
+                            // instead, which ends up being the logical equivalent.
+                            Integer codeSystemId = identityCache.getCodeSystemId(value.getValueSystem());
+                            whereClauseSegment.append(tableAlias).append(DOT).append(CODE_SYSTEM_ID).append(operator)
+                            .append(nullCheck(codeSystemId));
+                        }
                     }
                 }
+
                 whereClauseSegment.append(RIGHT_PAREN);
                 parmValueProcessed = true;
             }
@@ -1913,15 +1921,16 @@ private SqlQueryData populateCompositeSelectSubSegment(QueryParameter queryParm,
     /**
      * Builds a query that returns included resources.
      *
-     * @param resourceType
-     *                      - The type of resource being searched for.
-     * @param searchContext
-     *                      - The search context containing the search parameters.
-     * @return String - A count query SQL string
+     * @param resourceType  - the type of resource being searched for.
+     * @param searchContext - the search context containing the search parameters.
+     * @param inclusionParm - the inclusion parameter for which the query is being built.
+     * @param ids           - the set of logical resource IDs the query will run against.
+     * @param inclusionType - either INCLUDE or REVINCLUDE.
+     * @return SqlQueryData the populated inclusion query
      * @throws Exception
      */
     public SqlQueryData buildIncludeQuery(Class<?> resourceType, FHIRSearchContext searchContext,
-        InclusionParameter inclusionParm, Set<String> ids, String inclusionType) throws Exception {
+            InclusionParameter inclusionParm, Set<String> ids, String inclusionType) throws Exception {
         final String METHODNAME = "buildIncludeQuery";
         log.entering(CLASSNAME, METHODNAME);
 
@@ -1938,4 +1947,58 @@ public SqlQueryData buildIncludeQuery(Class<?> resourceType, FHIRSearchContext s
         return query;
     }
 
+    /**
+     * Populates an IN clause with value set codes for a token search parameter specifying the
+     * :in or :not-in modifier.
+     *
+     * @param whereClauseSegment  - the segment to which the sub-segment will be added
+     * @param parameterValue      - the search parameter value - a ValueSet URL
+     * @param parameterTableAlias - the alias for the parameter table e.g. CPx
+     * @throws FHIRPersistenceException
+     */
+    private void populateValueSetCodesSubSegment(StringBuilder whereClauseSegment, String parameterValue,
+            String parameterTableAlias) throws FHIRPersistenceException {
+        final String METHODNAME = "populateValueSetCodesSubSegment";
+        log.entering(CLASSNAME, METHODNAME, parameterValue);
+
+        String tokenValuePredicateString = parameterTableAlias + DOT + TOKEN_VALUE + IN + LEFT_PAREN;
+        String codeSystemIdPredicateString = parameterTableAlias + DOT + CODE_SYSTEM_ID + IN + LEFT_PAREN;
+        String defaultCodeSystemId = nullCheck(identityCache.getCodeSystemId(JDBCConstants.DEFAULT_TOKEN_SYSTEM));
+        boolean codeSystemProcessed = false;
+
+        // Note: validation that the value set exists and is expandable was done when the
+        // search parameter was parsed, so does not need to be done here.
+        Map<String, Set<String>> codeSetMap = ValueSetSupport.getCodeSetMap(ValueSetSupport.getValueSet(parameterValue));
+        for (String codeSetUrl : codeSetMap.keySet()) {
+            Set<String> codes = codeSetMap.get(codeSetUrl);
+            if (codes != null) {
+                // Strip version from canonical codeSet URL. We don't store version in TOKEN_VALUES
+                // table so will just ignore it.
+                int index = codeSetUrl.lastIndexOf("|");
+                if (index != -1) {
+                    codeSetUrl = codeSetUrl.substring(0, index);
+                }
+
+                if (codeSystemProcessed) {
+                    whereClauseSegment.append(OR);
+                }
+
+                // <parameterTableAlias>.TOKEN_VALUE IN (...)
+                whereClauseSegment.append(tokenValuePredicateString)
+                    .append("'").append(String.join("','", codes)).append("'")
+                    .append(RIGHT_PAREN);
+
+                // AND <parameterTableAlias>.CODE_SYSTEM_ID IN ({n}, {default-code-system-id})
+                whereClauseSegment.append(AND).append(codeSystemIdPredicateString)
+                    .append(nullCheck(identityCache.getCodeSystemId(codeSetUrl)))
+                    .append(COMMA).append(defaultCodeSystemId)
+                    .append(RIGHT_PAREN);
+
+                codeSystemProcessed = true;
+            }
+        }
+
+        log.exiting(CLASSNAME, METHODNAME);
+    }
+
 }
diff --git a/...istence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java b/...istence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java
@@ -601,7 +601,7 @@ protected void buildWhereClause(StringBuilder whereClause, String overrideType)
                                     .replaceAll(AS + PARAMETER_TABLE_ALIAS, AS + paramTableAlias)
                                     .replaceAll(PARAMETER_TABLE_NAME_PLACEHOLDER, valuesTable);
 
-                            if (Modifier.NOT.equals(param.getModifier())) {
+                            if (Modifier.NOT.equals(param.getModifier()) || Modifier.NOT_IN.equals(param.getModifier())) {
                                 // Not exists against a standard parameter table
                                 //   NOT EXISTS (SELECT 1 FROM Observation_TOKEN_VALUES AS param0
                                 //                     WHERE param0.PARAMETER_NAME_ID=1191 AND param0.TOKEN_VALUE = :p1