issue #1385 - remove xx_RESOURCES join from the default FROM clause

Its not super clean, but I wanted to see if it was possible to get some
isolated benefits from #1385 without needing to adopt the new query
builder.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>

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

@@ -244,9 +244,8 @@ public int compare(QueryParameter leftParameter, QueryParameter rightParameter)
         QuerySegmentAggregator helper;
         boolean isValidQuery = true;
 
-        helper =
-                QuerySegmentAggregatorFactory.buildQuerySegmentAggregator(resourceType, offset, pageSize,
-                        this.parameterDao, this.resourceDao, searchContext, false, this.queryHints, this.identityCache);
+        helper = QuerySegmentAggregatorFactory.buildQuerySegmentAggregator(resourceType, offset, pageSize,
+                this.parameterDao, this.resourceDao, searchContext, false, this.queryHints, this.identityCache);
 
         // Special logic for handling LocationPosition queries. These queries have interdependencies between
         // a couple of related input query parameters
@@ -1225,7 +1224,7 @@ private SqlQueryData processTokenParm(Class<?> resourceType, QueryParameter quer
                 }
 
                 whereClauseSegment.append(LEFT_PAREN);
-                
+
                 if (Modifier.IN.equals(queryParm.getModifier()) || Modifier.NOT_IN.equals(queryParm.getModifier())) {
                     populateValueSetCodesSubSegment(whereClauseSegment, value.getValueCode(), tableAlias);
                 } else {
@@ -1260,7 +1259,7 @@ private SqlQueryData processTokenParm(Class<?> resourceType, QueryParameter quer
                         }
                     }
                 }
-                
+
                 whereClauseSegment.append(RIGHT_PAREN);
                 parmValueProcessed = true;
             }
@@ -1382,7 +1381,7 @@ private SqlQueryData processCompositeParm(Class<?> resourceType, QueryParameter
             if (queryParm.getValues().size() > 1) {
                 // Build as WHERE clause using EXISTS
                 SqlQueryData compositeQueryData = populateCompositeSelectSubSegment(queryParm, resourceType, paramTableAlias, logicalResourceTableAlias);
-                joinSegment.append(" WHERE (EXISTS ").append(compositeQueryData.getQueryString()).append(RIGHT_PAREN);
+                joinSegment.append(WHERE).append("(EXISTS ").append(compositeQueryData.getQueryString()).append(RIGHT_PAREN);
                 bindVariables.addAll(compositeQueryData.getBindVariables());
             } else {
                 // Build as JOINS
@@ -1980,9 +1979,9 @@ private void populateValueSetCodesSubSegment(StringBuilder whereClauseSegment, S
                 if (codeSystemProcessed) {
                     whereClauseSegment.append(OR);
                 }
-                
+
                 // TODO: investigate if we can use COMMON_TOKEN_VALUES support
-                
+
                 // <parameterTableAlias>.TOKEN_VALUE IN (...)
                 whereClauseSegment.append(tokenValuePredicateString)
                     .append("'").append(String.join("','", codes)).append("'")
@@ -1991,7 +1990,7 @@ private void populateValueSetCodesSubSegment(StringBuilder whereClauseSegment, S
                 // AND <parameterTableAlias>.CODE_SYSTEM_ID = {n}
                 whereClauseSegment.append(AND).append(codeSystemIdPredicateString)
                     .append(nullCheck(identityCache.getCodeSystemId(codeSetUrl)));
-                
+
                 codeSystemProcessed = true;
             }
         }

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

@@ -67,10 +67,10 @@ public class QuerySegmentAggregator {
     protected static final String SYSTEM_LEVEL_SELECT_ROOT =
             "SELECT RESOURCE_ID, LOGICAL_RESOURCE_ID, VERSION_ID, LAST_UPDATED, IS_DELETED, DATA, LOGICAL_ID ";
     protected static final String SYSTEM_LEVEL_SUBSELECT_ROOT = SELECT_ROOT;
-    protected static final String SELECT_COUNT_ROOT = "SELECT COUNT(DISTINCT R.LOGICAL_RESOURCE_ID) ";
+    protected static final String SELECT_COUNT_ROOT = "SELECT COUNT(DISTINCT LR.LOGICAL_RESOURCE_ID) ";
     protected static final String SYSTEM_LEVEL_SELECT_COUNT_ROOT = "SELECT SUM(CNT) ";
     protected static final String SYSTEM_LEVEL_SUBSELECT_COUNT_ROOT = " SELECT COUNT(DISTINCT LR.LOGICAL_RESOURCE_ID) AS CNT ";
-    protected static final String WHERE_CLAUSE_ROOT = "WHERE R.IS_DELETED = 'N'";
+    protected static final String NOT_DELETED = " LR.IS_DELETED = 'N'";
 
     // Enables the SKIP_WHERE of WHERE clauses.
     public static final String ID = "_id";
@@ -362,6 +362,7 @@ protected SqlQueryData buildSystemLevelQuery(String selectRoot, String subSelect
                 buildWhereClause(queryString, resourceTypeName); // technically the JOIN clause
                 queryString.append(RIGHT_PAREN).append(" AS LR ");
 
+                // TODO: join to logical resources instead
                 // JOIN to RESOURCES R
                 queryString.append(JOIN);
                 processFromClauseForLastUpdated(queryString, resourceTypeName);
@@ -416,10 +417,12 @@ protected void buildFromClause(StringBuilder fromClause, String simpleName) {
         fromClause.append(" LR ");
 
         // This is requires for the count queries here
-        fromClause.append(JOIN);
-        processFromClauseForLastUpdated(fromClause, simpleName);
-        fromClause.append(" R ON R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID ")
-                .append(" AND R.IS_DELETED = 'N' ");
+        if (!queryParmLastUpdateds.isEmpty()) {
+            fromClause.append(JOIN);
+            processFromClauseForLastUpdated(fromClause, simpleName);
+            fromClause.append(" R ON R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID ")
+                    .append(" AND R.IS_DELETED = 'N' ");
+        }
 
         log.exiting(CLASSNAME, METHODNAME);
     }
@@ -470,7 +473,7 @@ public void processFromClauseForId(StringBuilder fromClause, String target) {
 
         if (!queryParamIds.isEmpty()) {
             // ID, then special handling.
-            fromClause.append("( SELECT LOGICAL_ID, LOGICAL_RESOURCE_ID, CURRENT_RESOURCE_ID FROM ");
+            fromClause.append("( SELECT LOGICAL_ID, LOGICAL_RESOURCE_ID, CURRENT_RESOURCE_ID, IS_DELETED FROM ");
             fromClause.append(target);
             fromClause.append("_LOGICAL_RESOURCES");
             fromClause.append(" ILR WHERE ILR.LOGICAL_ID IN ( ");
@@ -513,6 +516,7 @@ public void processFromClauseForLastUpdated(StringBuilder fromClause, String tar
             behaviorUtil.buildLastUpdatedDerivedTable(fromClause, target, queryParmLastUpdateds);
             lastUpdatedObjects.addAll(behaviorUtil.getBindVariables());
         } else {
+            // TODO: go to logical resources instead
             // Not _lastUpdated, then go to the default.
             fromClause.append(target);
             fromClause.append("_RESOURCES");
@@ -541,6 +545,8 @@ protected void buildWhereClause(StringBuilder whereClause, String overrideType)
        final String METHODNAME = "buildWhereClause";
         log.entering(CLASSNAME, METHODNAME);
 
+        boolean whereWordAdded = false;
+
         // Override the Type is null, then use the default type here.
         if (overrideType == null) {
             overrideType = this.resourceType.getSimpleName();
@@ -573,6 +579,7 @@ protected void buildWhereClause(StringBuilder whereClause, String overrideType)
                     if (missingOrNotModifierWhereClause.length() == 0) {
                         missingOrNotModifierWhereClause.append(querySegmentString);
                     } else {
+                        whereWordAdded = true;
                         // If not the first param with a :missing or :not modifier, replace the WHERE with an AND
                         missingOrNotModifierWhereClause.append(querySegmentString.replaceFirst(WHERE, AND));
                     }
@@ -633,19 +640,25 @@ protected void buildWhereClause(StringBuilder whereClause, String overrideType)
                                             .append(" AND LR.LOGICAL_RESOURCE_ID = ")
                                             .append(paramTableAlias)
                                             .append(".LOGICAL_RESOURCE_ID");
-                            }
-                        }
+                            } // end if/else for NOT / NOT-IN
+                        } // end if/else for isReverseChained
                     } else {
                         whereClause.append(querySegment.getQueryString()
                             .replaceAll(PARAMETER_TABLE_ALIAS + "_p", paramTableAlias + "_p"));
-                    }
-                }
+                        whereWordAdded = true;
+                    } // end if/else for COMPOSITE
+                } // end if/else for MISSING
             } // end if SKIP_WHERE
         } // end for
 
         // If there were any query parameters with :missing, :not, or :not-in modifier, append the missingOrNotModifierWhereClause
         if (missingOrNotModifierWhereClause.length() > 0) {
             whereClause.append(missingOrNotModifierWhereClause.toString());
+            whereWordAdded = true;
+        }
+
+        if (!SortedQuerySegmentAggregator.class.equals(this.getClass())) {
+            whereClause.append(whereWordAdded ? AND : WHERE).append(NOT_DELETED);
         }
 
         log.exiting(CLASSNAME, METHODNAME);

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

@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2017, 2020
+ * (C) Copyright IBM Corp. 2017, 2021
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -31,6 +31,7 @@
 
 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.ParameterDAO;
 import com.ibm.fhir.persistence.jdbc.dao.api.ResourceDAO;
@@ -48,7 +49,7 @@ public class SortedQuerySegmentAggregator extends QuerySegmentAggregator {
     private static final String CLASSNAME = SortedQuerySegmentAggregator.class.getName();
     private static final Logger log = java.util.logging.Logger.getLogger(CLASSNAME);
 
-    public static final String GROUP_BY = " GROUP BY R.RESOURCE_ID ";
+    public static final String GROUP_BY = " GROUP BY LR.CURRENT_RESOURCE_ID ";
     private static final String SORT_PARAMETER_ALIAS = "S";
 
     private List<SortParameter> sortParameters;
@@ -138,6 +139,9 @@ public SqlQueryData buildQuery() throws Exception {
             // Build LEFT OUTER JOIN clause
             sqlSortQuery.append(this.buildSortJoinClause());
 
+            // hacky!  what if there's a where clause added earlier!?
+            sqlSortQuery.append(JDBCConstants.WHERE).append(NOT_DELETED);
+
             // Build GROUP BY clause
             sqlSortQuery.append(GROUP_BY);
 
@@ -171,7 +175,7 @@ private String buildSelectClause() throws FHIRPersistenceException {
         log.entering(CLASSNAME, METHODNAME);
 
         StringBuilder selectBuffer = new StringBuilder();
-        selectBuffer.append("SELECT R.RESOURCE_ID");
+        selectBuffer.append("SELECT LR.CURRENT_RESOURCE_ID");
 
         // Build MIN and/or MAX clauses
         for (int i = 0; i < this.sortParameters.size(); i++) {
@@ -219,7 +223,7 @@ private String buildAggregateExpression(SortParameter sortParm, int sortParmInde
             expression.append(LEFT_PAREN);
 
             if (SearchConstants.LAST_UPDATED.equals(sortParm.getCode())) {
-                expression.append("R.LAST_UPDATED");
+                expression.append("LR.LAST_UPDATED");
             } else {
                 expression.append(SORT_PARAMETER_ALIAS).append(sortParmIndex).append(DOT_CHAR);
                 expression.append(attributeName);
@@ -327,7 +331,7 @@ private String buildSortJoinClause() throws FHIRPersistenceException {
                         .append(sortParameterNameId)
                         .append(AND)
                         .append(SORT_PARAMETER_ALIAS).append(sortParmIndex)
-                        .append(".LOGICAL_RESOURCE_ID = R.LOGICAL_RESOURCE_ID")
+                        .append(".LOGICAL_RESOURCE_ID = LR.LOGICAL_RESOURCE_ID")
                         .append(RIGHT_PAREN).append(SPACE);
 
                 sortParmIndex++;

fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/type/LastUpdatedParmBehaviorUtil.java

@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2020
+ * (C) Copyright IBM Corp. 2019, 2021
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -34,7 +34,7 @@
  * This utility encapsulates the logic specific to fhir-search related to date.
  * <br>
  * The derived table looks similar to the following SQL:
- * 
+ *
  * <pre>
  * (
  *       SELECT *
@@ -62,7 +62,7 @@ public LastUpdatedParmBehaviorUtil() {
     /**
      * builds the query parameters for the last updated
      * <br>
-     * 
+     *
      * @param fromClause
      * @param target
      * @param parameters
@@ -72,15 +72,15 @@ public void buildLastUpdatedDerivedTable(StringBuilder fromClause, String target
         fromClause.append(LEFT_PAREN);
         fromClause.append(" SELECT * FROM ");
         fromClause.append(target);
-        fromClause.append("_RESOURCES IR WHERE ");
+        fromClause.append("_LOGICAL_RESOURCES IR WHERE ");
 
         boolean parmProcessed = false;
         for (QueryParameter queryParm : parameters) {
             // If multiple values are present, we need to AND them together.
             if (parmProcessed) {
                 fromClause.append(AND);
             } else {
-                // Signal to the downstream to treat any subsequent value as an AND condition 
+                // Signal to the downstream to treat any subsequent value as an AND condition
                 parmProcessed = true;
             }
             executeBehavior(fromClause, queryParm);
@@ -92,28 +92,28 @@ public void buildLastUpdatedDerivedTable(StringBuilder fromClause, String target
 
     /**
      * generate for each
-     * 
+     *
      * @param fromClause
      * @param queryParm
      */
     public void executeBehavior(StringBuilder whereClause, QueryParameter queryParm) {
-        // Start the Clause 
+        // Start the Clause
         // Query: (
         whereClause.append(LEFT_PAREN);
 
-        // Initially we don't want to treat this as the second value. 
+        // Initially we don't want to treat this as the second value.
         boolean parmValueProcessed = false;
         for (QueryParameterValue value : queryParm.getValues()) {
             // If multiple values are present, we need to OR them together.
             if (parmValueProcessed) {
                 // OR
                 whereClause.append(RIGHT_PAREN).append(OR).append(LEFT_PAREN);
             } else {
-                // Signal to the downstream to treat any subsequent value as an OR condition 
+                // Signal to the downstream to treat any subsequent value as an OR condition
                 parmValueProcessed = true;
             }
 
-            // Let's get the prefix. 
+            // Let's get the prefix.
             Prefix prefix = value.getPrefix();
             if (prefix == null) {
                 // Default to EQ
@@ -125,14 +125,14 @@ public void executeBehavior(StringBuilder whereClause, QueryParameter queryParm)
             buildPredicates(whereClause, prefix, v, upperBound);
         }
 
-        // End the Clause started above, and closes the parameter expression. 
+        // End the Clause started above, and closes the parameter expression.
         // Query: )
         whereClause.append(RIGHT_PAREN);
     }
 
     /**
      * builds query elements based on prefix type.
-     * 
+     *
      * @param whereClauseSegment
      * @param prefix
      * @param value
@@ -184,20 +184,20 @@ public void buildPredicates(StringBuilder whereClauseSegment, Prefix prefix, Ins
 
     /**
      * builds the common clause
-     * 
+     *
      * @param whereClauseSegment
      * @param operator
      * @param value
      */
     public void buildCommonClause(StringBuilder whereClauseSegment, String operator, Instant value) {
-        // Example: <code>LAST_UPDATED <= ?</code> 
+        // Example: <code>LAST_UPDATED <= ?</code>
         whereClauseSegment.append(LAST_UPDATED_COLUMN_NAME).append(operator).append(BIND_VAR);
         bindVariables.add(generateTimestamp(value));
     }
 
     /**
      * builds equals range
-     * 
+     *
      * @param whereClauseSegment
      * @param lowerBound
      * @param upperBound
@@ -226,7 +226,7 @@ public void buildEqualsRangeClause(StringBuilder whereClauseSegment, Instant low
 
     /**
      * build not equals range clause
-     * 
+     *
      * @param whereClauseSegment
      * @param lowerBound
      * @param upperBound