Skip to content

Commit

Permalink
issue #2528 - change interpretation of lt and gt
Browse files Browse the repository at this point in the history 
For both date and number searches.

We use the range interpretation of the search prefixes. The spec says
`lt` means "the range below the search value intersects (i.e. overlaps)
with the range of the target value" and `gt` means "the range above the
search value intersects (i.e. overlaps) with the range of the target
value"

Previously, we interpreted that to mean that a search like `gt2016`
should return any value above `2016-01-01 00:00:00` in the server's
timezone.

However, this interpretation leads to confusing and unintuitive results.
Instead, we will now interpret the "search value" of 2016 to be the
range `[2016-01-01 00:00:00, 2017-01-01 00:00:00)` and therefor it will
only find target values above 2017-01-01 00:00:00 (inclusive).

And similarly for `lt`.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
  • Loading branch information
@lmsurpre
lmsurpre committed Jun 19, 2021
1 parent 6ed332f commit ce7f76d
Show file tree
Hide file tree
Showing 12 changed files with 454 additions and 470 deletions.
4 changes: 2 additions & 2 deletions ...ence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/type/DateParmBehaviorUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public void buildPredicates(StringBuilder whereClauseSegment, List<Timestamp> bi
case GT:
// GT - Greater Than
// the range above the search value intersects (i.e. overlaps) with the range of the target value
buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_END, GT, lowerBound);
buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_END, GT, upperBound);
break;
case LE:
// LE - Less Than Equal
Expand All @@ -121,7 +121,7 @@ public void buildPredicates(StringBuilder whereClauseSegment, List<Timestamp> bi
case LT:
// LT - Less Than
// the range below the search value intersects (i.e. overlaps) with the range of the target value
buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_START, LT, upperBound);
buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_START, LT, lowerBound);
break;
case AP:
// AP - Approximate - Relative
Expand Down
4 changes: 2 additions & 2 deletions ...e-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/type/NewDateParmBehaviorUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public void buildPredicates(WhereFragment whereClauseSegment, String tableAlias,
case GT:
// GT - Greater Than
// the range above the search value intersects (i.e. overlaps) with the range of the target value
buildCommonClause(whereClauseSegment, tableAlias, DATE_END, GT, lowerBound);
buildCommonClause(whereClauseSegment, tableAlias, DATE_END, GT, upperBound);
break;
case LE:
// LE - Less Than Equal
Expand All @@ -113,7 +113,7 @@ public void buildPredicates(WhereFragment whereClauseSegment, String tableAlias,
case LT:
// LT - Less Than
// the range below the search value intersects (i.e. overlaps) with the range of the target value
buildCommonClause(whereClauseSegment, tableAlias, DATE_START, LT, upperBound);
buildCommonClause(whereClauseSegment, tableAlias, DATE_START, LT, lowerBound);
break;
case AP:
// AP - Approximate - Relative
Expand Down
10 changes: 5 additions & 5 deletions ...jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/type/NewNumberParmBehaviorUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public static void addValue(WhereFragment whereClauseSegment, String tableAlias,
// GT - Greater Than
// the range above the search value intersects (i.e. overlaps) with the range of the target value
buildCommonClause(whereClauseSegment, tableAlias, columnBase, columnBase + _LOW,
columnBase + _HIGH, GT, value, lowerBound);
columnBase + _HIGH, GT, value, upperBound);
break;
case LE:
// LE - Less Than Equal
Expand All @@ -137,7 +137,7 @@ public static void addValue(WhereFragment whereClauseSegment, String tableAlias,
// LT - Less Than
// the range below the search value intersects (i.e. overlaps) with the range of the target value
buildCommonClause(whereClauseSegment, tableAlias, columnBase, columnBase + _LOW,
columnBase + _HIGH, LT, value, upperBound);
columnBase + _HIGH, LT, value, lowerBound);
break;
case AP:
// AP - Approximate - Relative
Expand Down Expand Up @@ -256,15 +256,15 @@ public static void buildApproxRangeClause(WhereFragment whereClauseSegment, Stri
BigDecimal approximateUpperBound = upperBound.add(factor);

whereClauseSegment.leftParen();

// The following clauses test for overlap when both xx_VALUE_HIGH and xx_VALUE_LOW are non-null.
// Example:
// P2.QUANTITY_VALUE_HIGH >= 8.5
// AND P2.QUANTITY_VALUE_LOW <= 11.5
whereClauseSegment.col(tableAlias, columnBase + _HIGH).gte().bind(approximateLowerBound);
whereClauseSegment.and();
whereClauseSegment.col(tableAlias, columnBase + _LOW).lte().bind(approximateUpperBound);

// The following clauses test for overlap when the target is a Range data type and either
// xx_VALUE_HIGH or xx_VALUE_LOW is NULL (unknown bound).
// Example:
Expand Down Expand Up @@ -306,7 +306,7 @@ public static void buildApproxRangeClause(WhereFragment whereClauseSegment, Stri
whereClauseSegment.and();
whereClauseSegment.col(tableAlias, columnBase + _HIGH).isNull();
whereClauseSegment.rightParen();

whereClauseSegment.rightParen();
}

Expand Down
34 changes: 17 additions & 17 deletions ...ce-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/type/NumberParmBehaviorUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ private NumberParmBehaviorUtil() {
public static void executeBehavior(StringBuilder whereClauseSegment, QueryParameter queryParm,
List<Object> bindVariables, Class<?> resourceType, String tableAlias, JDBCQueryBuilder queryBuilder)
throws FHIRPersistenceException {
// Start the Clause
// Start the Clause
// Query: AND (
whereClauseSegment.append(AND).append(LEFT_PAREN);

Expand All @@ -73,8 +73,8 @@ public static void executeBehavior(StringBuilder whereClauseSegment, QueryParame
}
BigDecimal originalNumber = value.getValueNumber();

// seen is used to optimize against a repeated value passed in.
// the hash must use the prefix and original number.
// seen is used to optimize against a repeated value passed in.
// the hash must use the prefix and original number.
String hash = prefix.value() + originalNumber.toPlainString();
if (!seen.contains(hash)) {
seen.add(hash);
Expand All @@ -84,24 +84,24 @@ public static void executeBehavior(StringBuilder whereClauseSegment, QueryParame
// ) OR (
whereClauseSegment.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;
}

addValue(whereClauseSegment, bindVariables, tableAlias, NUMBER_VALUE, prefix, value.getValueNumber());
}
}

// End the Clause started above, and closes the parameter expression.
// Query: ))
// End the Clause started above, and closes the parameter expression.
// Query: ))
whereClauseSegment.append(RIGHT_PAREN).append(RIGHT_PAREN);
}

/**
* Append the condition and bind the variables according to the semantics of the
* passed prefix
* adds the value to the whereClause.
*
*
* @param whereClauseSegment
* @param bindVariables
* @param tableAlias
Expand Down Expand Up @@ -139,7 +139,7 @@ public static void addValue(StringBuilder whereClauseSegment, List<Object> bindV
case GT:
// GT - Greater Than
// the range above the search value intersects (i.e. overlaps) with the range of the target value
buildCommonClause(whereClauseSegment, bindVariables, tableAlias, columnBase, GT, value, lowerBound);
buildCommonClause(whereClauseSegment, bindVariables, tableAlias, columnBase, GT, value, upperBound);
break;
case LE:
// LE - Less Than Equal
Expand All @@ -150,7 +150,7 @@ public static void addValue(StringBuilder whereClauseSegment, List<Object> bindV
case LT:
// LT - Less Than
// the range below the search value intersects (i.e. overlaps) with the range of the target value
buildCommonClause(whereClauseSegment, bindVariables, tableAlias, columnBase, LT, value, upperBound);
buildCommonClause(whereClauseSegment, bindVariables, tableAlias, columnBase, LT, value, lowerBound);
break;
case AP:
// AP - Approximate - Relative
Expand Down Expand Up @@ -179,7 +179,7 @@ public static void addValue(StringBuilder whereClauseSegment, List<Object> bindV
* circuits double matches.
* If one exists, great, we'll return it, else we'll peek at the other column.
* <br>
*
*
* @param whereClauseSegment
* @param bindVariables
* @param tableAlias
Expand All @@ -190,7 +190,7 @@ public static void addValue(StringBuilder whereClauseSegment, List<Object> bindV
*/
public static void buildCommonClause(StringBuilder whereClauseSegment, List<Object> bindVariables, String tableAlias,
String columnBase, String operator, BigDecimal value, BigDecimal bound) {

String orEqualsOperator = null;
if (GTE.equals(operator)) {
operator = GT;
Expand All @@ -200,7 +200,7 @@ public static void buildCommonClause(StringBuilder whereClauseSegment, List<Obje
orEqualsOperator = LTE;
}
boolean gtOp = GT.equals(operator);

whereClauseSegment
.append(LEFT_PAREN)
.append(tableAlias).append(DOT).append(gtOp ? columnBase + _HIGH : columnBase + _LOW).append(operator).append(BIND_VAR)
Expand All @@ -216,10 +216,10 @@ public static void buildCommonClause(StringBuilder whereClauseSegment, List<Obje
bindVariables.add(value);
bindVariables.add(orEqualsOperator != null ? bound : value);
}

/**
* the build eb or sa clause considers only _VALUE_LOW and _VALUE_HIGH
*
*
* @param whereClauseSegment
* @param bindVariables
* @param tableAlias
Expand All @@ -232,7 +232,7 @@ public static void buildEbOrSaClause(StringBuilder whereClauseSegment, List<Obje
whereClauseSegment.append(tableAlias).append(DOT).append(columnNameLowOrHigh).append(operator).append(BIND_VAR);
bindVariables.add(bound);
}

/**
* Add the equals range clause to the given whereClauseSegment
* @param whereClauseSegment
Expand All @@ -254,7 +254,7 @@ public static void buildEqualsRangeClause(StringBuilder whereClauseSegment, List
bindVariables.add(lowerBound);
bindVariables.add(upperBound);
}

/**
* Add the approx range clause to the given whereClauseSegment
* @param whereClauseSegment
Expand Down Expand Up @@ -311,7 +311,7 @@ public static void buildApproxRangeClause(StringBuilder whereClauseSegment, List
bindVariables.add(approximateLowerBound);
bindVariables.add(approximateUpperBound);
}

/**
* Add the not-equals range clause to the given whereClauseSegment
* @param whereClauseSegment
Expand Down
36 changes: 12 additions & 24 deletions ...-jdbc/src/test/java/com/ibm/fhir/persistence/jdbc/test/util/DateParmBehaviorUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

public class DateParmBehaviorUtilTest {
private static final Logger log = java.util.logging.Logger.getLogger(DateParmBehaviorUtilTest.class.getName());
private static final Level LOG_LEVEL = Level.FINE;
private static final Level LOG_LEVEL = Level.INFO;

//---------------------------------------------------------------------------------------------------------
// Supporting Methods:
Expand Down Expand Up @@ -109,20 +109,18 @@ private void runTest(QueryParameter queryParm, List<Timestamp> expectedBindVaria

}

@SuppressWarnings("unused")
@Test
public void testHandleDateRangeComparisonWithExact() throws Exception {
String vTime = "2019-12-11T00:00:00+00:00";

TemporalAccessor v = DateTimeHandler.parse(vTime);
Timestamp value = Timestamp.from(DateTimeHandler.generateValue(v, vTime));
Timestamp lower = Timestamp.from(DateTimeHandler.generateLowerBound(Prefix.EQ, v, vTime));
Timestamp upper = Timestamp.from(DateTimeHandler.generateUpperBound(Prefix.EQ, v, vTime));

// gt - Greater Than
QueryParameter queryParm = generateQueryParameter(SearchConstants.Prefix.GT, null, vTime);
List<Timestamp> expectedBindVariables = new ArrayList<>();
expectedBindVariables.add(value);
expectedBindVariables.add(upper);
String expectedSql =
" AND ((Date.DATE_END > ?)))";
runTest(queryParm,
Expand All @@ -132,7 +130,7 @@ public void testHandleDateRangeComparisonWithExact() throws Exception {
// lt - Less Than
queryParm = generateQueryParameter(SearchConstants.Prefix.LT, null, vTime);
expectedBindVariables = new ArrayList<>();
expectedBindVariables.add(upper);
expectedBindVariables.add(lower);
expectedSql =
" AND ((Date.DATE_START < ?)))";
runTest(queryParm,
Expand All @@ -142,7 +140,7 @@ public void testHandleDateRangeComparisonWithExact() throws Exception {
// ge - Greater than Equal
queryParm = generateQueryParameter(SearchConstants.Prefix.GE, null, vTime);
expectedBindVariables = new ArrayList<>();
expectedBindVariables.add(value);
expectedBindVariables.add(lower);
expectedSql =
" AND ((Date.DATE_END >= ?)))";
runTest(queryParm,
Expand Down Expand Up @@ -172,7 +170,7 @@ public void testHandleDateRangeComparisonWithExact() throws Exception {
// eb - Ends before
queryParm = generateQueryParameter(SearchConstants.Prefix.EB, null, vTime);
expectedBindVariables = new ArrayList<>();
expectedBindVariables.add(value);
expectedBindVariables.add(lower);
expectedSql =
" AND ((Date.DATE_END < ?)))";
runTest(queryParm,
Expand All @@ -184,15 +182,13 @@ public void testHandleDateRangeComparisonWithExact() throws Exception {
public void testPrecisionWithEqual() throws Exception {
String vTime = "2019-12-11T00:00:00+00:00";
TemporalAccessor v = DateTimeHandler.parse(vTime);
Timestamp value = Timestamp.from(DateTimeHandler.generateValue(v, vTime));
Timestamp lower = Timestamp.from(DateTimeHandler.generateLowerBound(Prefix.EQ, v, vTime));
Timestamp upper = Timestamp.from(DateTimeHandler.generateUpperBound(Prefix.EQ, v, vTime));

QueryParameter queryParm = generateQueryParameter(SearchConstants.Prefix.EQ, null, vTime);
List<Timestamp> expectedBindVariables = new ArrayList<>();
expectedBindVariables.add(lower);
expectedBindVariables.add(upper);
expectedBindVariables.add(value);
expectedBindVariables.add(value);
expectedBindVariables.add(value);
String expectedSql =
" AND (((Date.DATE_START >= ? AND Date.DATE_END <= ?))))";
runTest(queryParm,
Expand All @@ -204,19 +200,15 @@ public void testPrecisionWithEqual() throws Exception {
public void testPrecisionWithMultipleValuesForEqual() throws Exception {
String vTime = "2019-12-11T00:00:00+00:00";
TemporalAccessor v = DateTimeHandler.parse(vTime);
Timestamp value = Timestamp.from(DateTimeHandler.generateValue(v, vTime));
Timestamp lower = Timestamp.from(DateTimeHandler.generateLowerBound(Prefix.EQ, v, vTime));
Timestamp upper = Timestamp.from(DateTimeHandler.generateUpperBound(Prefix.EQ, v, vTime));

QueryParameter queryParm = generateQueryParameter(SearchConstants.Prefix.EQ, null, vTime, vTime);
List<Timestamp> expectedBindVariables = new ArrayList<>();
expectedBindVariables.add(upper);
expectedBindVariables.add(value);
expectedBindVariables.add(value);
expectedBindVariables.add(value);
expectedBindVariables.add(upper);
expectedBindVariables.add(value);
expectedBindVariables.add(value);
expectedBindVariables.add(value);
expectedBindVariables.add(lower);
expectedBindVariables.add(lower);
expectedBindVariables.add(lower);

String expectedSql =
" AND (((Date.DATE_START >= ? AND Date.DATE_END <= ?)) OR ((Date.DATE_START >= ? AND Date.DATE_END <= ?))))";
Expand All @@ -225,21 +217,17 @@ public void testPrecisionWithMultipleValuesForEqual() throws Exception {
expectedSql);
}

@SuppressWarnings("unused")
@Test
public void testPrecisionWithNotEqual() throws Exception {
String vTime = "2019-12-11T00:00:00+00:00";
TemporalAccessor v = DateTimeHandler.parse(vTime);
Timestamp value = Timestamp.from(DateTimeHandler.generateValue(v, vTime));
Timestamp lower = Timestamp.from(DateTimeHandler.generateLowerBound(Prefix.EQ, v, vTime));
Timestamp upper = Timestamp.from(DateTimeHandler.generateUpperBound(Prefix.EQ, v, vTime));

QueryParameter queryParm = generateQueryParameter(SearchConstants.Prefix.NE, null, vTime);
List<Timestamp> expectedBindVariables = new ArrayList<>();
expectedBindVariables.add(lower);
expectedBindVariables.add(upper);
expectedBindVariables.add(upper);
expectedBindVariables.add(value);
expectedBindVariables.add(value);

String expectedSql =
" AND (((Date.DATE_START < ? OR Date.DATE_END > ?))))";
Expand Down
Loading

0 comments on commit ce7f76d

Please sign in to comment.