From 614585cc1f82bf7550aaf524ba6f078b5485116c Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Sat, 21 Sep 2019 07:42:04 -0400 Subject: [PATCH 01/27] added failing test --- .../r4/FhirResourceDaoR4SearchNoFtTest.java | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index 7c18e672068..9eee3296b54 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -3943,6 +3943,68 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { )); } + @Test + public void testDateSearchParametersShouldBeHourIndependent() { + + createObservationWithEffective("YES01", "2011-01-02T00:00:00"); + createObservationWithEffective("YES02", "2011-01-02T01:00:00"); + createObservationWithEffective("YES03", "2011-01-02T02:00:00"); + createObservationWithEffective("YES04", "2011-01-02T03:00:00"); + createObservationWithEffective("YES05", "2011-01-02T04:00:00"); + createObservationWithEffective("YES06", "2011-01-02T05:00:00"); + createObservationWithEffective("YES07", "2011-01-02T06:00:00"); + createObservationWithEffective("YES08", "2011-01-02T07:00:00"); + createObservationWithEffective("YES09", "2011-01-02T08:00:00"); + createObservationWithEffective("YES10", "2011-01-02T09:00:00"); + createObservationWithEffective("YES11", "2011-01-02T10:00:00"); + createObservationWithEffective("YES12", "2011-01-02T11:00:00"); + createObservationWithEffective("YES13", "2011-01-02T12:00:00"); + createObservationWithEffective("YES14", "2011-01-02T13:00:00"); + createObservationWithEffective("YES15", "2011-01-02T14:00:00"); + createObservationWithEffective("YES16", "2011-01-02T15:00:00"); + createObservationWithEffective("YES17", "2011-01-02T16:00:00"); + createObservationWithEffective("YES18", "2011-01-02T17:00:00"); + createObservationWithEffective("YES19", "2011-01-02T18:00:00"); + createObservationWithEffective("YES20", "2011-01-02T19:00:00"); + createObservationWithEffective("YES21", "2011-01-02T20:00:00"); + createObservationWithEffective("YES22", "2011-01-02T21:00:00"); + createObservationWithEffective("YES23", "2011-01-02T22:00:00"); + createObservationWithEffective("YES24", "2011-01-02T23:00:00"); + + SearchParameterMap map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Observation.SP_DATE, new DateParam("2011-01-02")); + IBundleProvider results = myObservationDao.search(map); + List values = toUnqualifiedVersionlessIdValues(results); + Collections.sort(values); + assertThat(values.toString(), values, contains( + "Observation/YES01", + "Observation/YES02", + "Observation/YES03", + "Observation/YES04", + "Observation/YES05", + "Observation/YES06", + "Observation/YES07", + "Observation/YES08", + "Observation/YES09", + "Observation/YES10", + "Observation/YES11", + "Observation/YES12", + "Observation/YES13", + "Observation/YES14", + "Observation/YES15", + "Observation/YES16", + "Observation/YES17", + "Observation/YES18", + "Observation/YES19", + "Observation/YES20", + "Observation/YES21", + "Observation/YES22", + "Observation/YES23", + "Observation/YES24" + )); + } + private void createObservationWithEffective(String theId, String theEffective) { Observation obs = new Observation(); obs.setId(theId); From 7f5b3394e001aa115e30cf5857cef7d4203c2280 Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Mon, 24 Feb 2020 19:34:16 -0500 Subject: [PATCH 02/27] Hacky first-pass of adding ordinal date field to ResourceIndexedSearchParamDate and using it for search --- .../uhn/fhir/rest/param/DateRangeParam.java | 22 ++++ .../dao/predicate/BasePredicateBuilder.java | 1 - .../dao/predicate/PredicateBuilderDate.java | 102 ++++++++++++------ .../r4/FhirResourceDaoR4SearchNoFtTest.java | 1 - .../src/test/resources/logback-test.xml | 2 +- .../ResourceIndexedSearchParamDate.java | 27 +++++ 6 files changed, 120 insertions(+), 35 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java index f36e4dd82f0..8b9a6245b16 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java @@ -262,6 +262,22 @@ public class DateRangeParam implements IQueryParameterAnd { validateAndSet(myLowerBound, new DateParam(ParamPrefixEnum.LESSTHAN, theUpperBound)); return this; } + public Integer getLowerBoundAsDateOrdinal() { + if (myLowerBound == null || myLowerBound.getValue() == null) { + return null; + } + Calendar cal = DateUtils.toCalendar(myLowerBound.getValue()); + String s = new StringBuilder().append(cal.get(Calendar.YEAR)).append(cal.get(Calendar.MONTH)).append(cal.get(Calendar.DAY_OF_MONTH)).toString(); + return Integer.parseInt(s); + } + public Integer getUpperBoundAsDateOrdinal() { + if (myUpperBound == null || myUpperBound.getValue() == null) { + return null; + } + Calendar cal = DateUtils.toCalendar(myUpperBound.getValue()); + String s = new StringBuilder().append(cal.get(Calendar.YEAR)).append(cal.get(Calendar.MONTH)).append(cal.get(Calendar.DAY_OF_MONTH)).toString(); + return Integer.parseInt(s); + } public Date getLowerBoundAsInstant() { if (myLowerBound == null || myLowerBound.getValue() == null) { @@ -334,6 +350,12 @@ public class DateRangeParam implements IQueryParameterAnd { Date retVal = myUpperBound.getValue(); + + //if (myUpperBound.getPrecision().ordinal() == TemporalPrecisionEnum.DAY.ordinal()) { + // DateUtils.setHours(retVal, 23); + // DateUtils.setMinutes(retVal, 59); +// DateUtils.setSeconds(retVal, 59); +// } if (myUpperBound.getPrecision().ordinal() <= TemporalPrecisionEnum.DAY.ordinal()) { Calendar cal = DateUtils.toCalendar(retVal); cal.setTimeZone(TimeZone.getTimeZone("GMT+11:30")); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/BasePredicateBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/BasePredicateBuilder.java index db189334bc7..f5c6c0480ba 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/BasePredicateBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/BasePredicateBuilder.java @@ -126,7 +126,6 @@ abstract class BasePredicateBuilder { } void addPredicateParamMissing(String theResourceName, String theParamName, boolean theMissing, Join theJoin) { - myQueryRoot.addPredicate(myBuilder.equal(theJoin.get("myResourceType"), theResourceName)); myQueryRoot.addPredicate(myBuilder.equal(theJoin.get("myParamName"), theParamName)); myQueryRoot.addPredicate(myBuilder.equal(theJoin.get("myMissing"), theMissing)); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java index 6ad65712e1d..b269d4b787f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java @@ -24,6 +24,7 @@ import ca.uhn.fhir.jpa.dao.SearchBuilder; import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamDate; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.model.api.IQueryParameterType; +import ca.uhn.fhir.model.api.TemporalPrecisionEnum; import ca.uhn.fhir.rest.param.DateParam; import ca.uhn.fhir.rest.param.DateRangeParam; import ca.uhn.fhir.rest.param.ParamPrefixEnum; @@ -127,67 +128,102 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi return combineParamIndexPredicateWithParamNamePredicate(theResourceName, theParamName, theFrom, p); } + private boolean isNullOrDayPrecision(DateParam theDateParam) { + return theDateParam == null || theDateParam.getPrecision().ordinal() == TemporalPrecisionEnum.DAY.ordinal(); + } private Predicate createPredicateDateFromRange(CriteriaBuilder theBuilder, From theFrom, DateRangeParam theRange, SearchFilterParser.CompareOperation operation) { - Date lowerBound = theRange.getLowerBoundAsInstant(); - Date upperBound = theRange.getUpperBoundAsInstant(); + Date lowerBoundInstant = theRange.getLowerBoundAsInstant(); + Date upperBoundInstant = theRange.getUpperBoundAsInstant(); + + DateParam lowerBound = theRange.getLowerBound(); + DateParam upperBound = theRange.getUpperBound(); + boolean isOrdinalComparison = isNullOrDayPrecision(lowerBound) && isNullOrDayPrecision(upperBound); Predicate lt = null; Predicate gt = null; Predicate lb = null; Predicate ub = null; if (operation == SearchFilterParser.CompareOperation.lt) { - if (lowerBound == null) { + if (lowerBoundInstant == null) { throw new InvalidRequestException("lowerBound value not correctly specified for compare operation"); } - lb = theBuilder.lessThan(theFrom.get("myValueLow"), lowerBound); + //im like 80% sure this should be ub and not lb, as it is an UPPER bound. + if (isOrdinalComparison) { + lb = theBuilder.lessThan(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateOrdinal()); + } else { + lb = theBuilder.lessThan(theFrom.get("myValueLow"), lowerBoundInstant); + } } else if (operation == SearchFilterParser.CompareOperation.le) { - if (upperBound == null) { + if (upperBoundInstant == null) { throw new InvalidRequestException("upperBound value not correctly specified for compare operation"); } - lb = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHigh"), upperBound); + //im like 80% sure this should be ub and not lb, as it is an UPPER bound. + if (isOrdinalComparison) { + lb = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateOrdinal()); + } else { + lb = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); + } } else if (operation == SearchFilterParser.CompareOperation.gt) { - if (upperBound == null) { + if (upperBoundInstant == null) { throw new InvalidRequestException("upperBound value not correctly specified for compare operation"); } - lb = theBuilder.greaterThan(theFrom.get("myValueHigh"), upperBound); - } else if (operation == SearchFilterParser.CompareOperation.ge) { - if (lowerBound == null) { + if (isOrdinalComparison) { + lb = theBuilder.greaterThan(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateOrdinal()); + } else { + lb = theBuilder.greaterThan(theFrom.get("myValueHigh"), upperBoundInstant); + } + } else if (operation == SearchFilterParser.CompareOperation.ge) { + if (lowerBoundInstant == null) { throw new InvalidRequestException("lowerBound value not correctly specified for compare operation"); } - lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLow"), lowerBound); + if (isOrdinalComparison) { + lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateOrdinal()); + } else { + lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLow"), lowerBoundInstant); + } } else if (operation == SearchFilterParser.CompareOperation.ne) { - if ((lowerBound == null) || - (upperBound == null)) { + if ((lowerBoundInstant == null) || + (upperBoundInstant == null)) { throw new InvalidRequestException("lowerBound and/or upperBound value not correctly specified for compare operation"); } - /*Predicate*/ - lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLow"), lowerBound); - /*Predicate*/ - gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHigh"), upperBound); + if (isOrdinalComparison){ + lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateOrdinal()); + gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateOrdinal()); + } else { + lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLow"), lowerBoundInstant); + gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); + } lb = theBuilder.or(lt, gt); - } else if ((operation == SearchFilterParser.CompareOperation.eq) || - (operation == null)) { - if (lowerBound != null) { - /*Predicate*/ - gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLow"), lowerBound); - /*Predicate*/ - lt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHigh"), lowerBound); - if (theRange.getLowerBound().getPrefix() == ParamPrefixEnum.STARTS_AFTER || theRange.getLowerBound().getPrefix() == ParamPrefixEnum.EQUAL) { + } else if ((operation == SearchFilterParser.CompareOperation.eq) || (operation == null)) { + if (lowerBoundInstant != null) { + if (isOrdinalComparison) { + gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateOrdinal()); + lt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getLowerBoundAsDateOrdinal()); + //also try a strict equality here. + } + else { + gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLow"), lowerBoundInstant); + lt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHigh"), lowerBoundInstant); + } + if (lowerBound.getPrefix() == ParamPrefixEnum.STARTS_AFTER || lowerBound.getPrefix() == ParamPrefixEnum.EQUAL) { lb = gt; } else { lb = theBuilder.or(gt, lt); } } - if (upperBound != null) { - /*Predicate*/ - gt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLow"), upperBound); - /*Predicate*/ - lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHigh"), upperBound); + if (upperBoundInstant != null) { + if (isOrdinalComparison) { + gt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getUpperBoundAsDateOrdinal()); + lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateOrdinal()); + } else { + gt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLow"), upperBoundInstant); + lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); + } if (theRange.getUpperBound().getPrefix() == ParamPrefixEnum.ENDS_BEFORE || theRange.getUpperBound().getPrefix() == ParamPrefixEnum.EQUAL) { ub = lt; } else { @@ -198,8 +234,10 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi throw new InvalidRequestException(String.format("Unsupported operator specified, operator=%s", operation.name())); } - - ourLog.trace("Date range is {} - {}", lowerBound, upperBound); + if (isOrdinalComparison) { + ourLog.trace("Ordinal date range is {} - {} ", theRange.getLowerBoundAsDateOrdinal(), theRange.getUpperBoundAsDateOrdinal()); + } + ourLog.trace("Date range is {} - {}", lowerBoundInstant, upperBoundInstant); if (lb != null && ub != null) { return (theBuilder.and(lb, ub)); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index 6d543952b20..66687daf9dc 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -4094,7 +4094,6 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { obs.setId(theId); obs.setEffective(new DateTimeType(theEffective)); myObservationDao.update(obs); - ourLog.info("Obs {} has time {}", theId, obs.getEffectiveDateTimeType().getValue().toString()); } diff --git a/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml b/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml index 6818eb0a0f3..c8a33e5568b 100644 --- a/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml +++ b/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml @@ -27,7 +27,7 @@ - + diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java index 7b097893c0f..e355cce822b 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java @@ -29,10 +29,12 @@ import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; +import org.apache.commons.lang3.time.DateUtils; import org.hibernate.search.annotations.Field; import org.hl7.fhir.r4.model.DateTimeType; import javax.persistence.*; +import java.util.Calendar; import java.util.Date; @Embeddable @@ -54,6 +56,12 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar @Temporal(TemporalType.TIMESTAMP) @Field public Date myValueLow; + + @Column(name="SP_VALUE_LOW_DATE_ORDINAL") + public int myValueLowDateOrdinal; + @Column(name="SP_VALUE_HIGH_DATE_ORDINAL") + public int myValueHighDateOrdinal; + @Transient private transient String myOriginalValue; @Id @@ -82,9 +90,28 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar setParamName(theParamName); setValueLow(theLow); setValueHigh(theHigh); + computeValueHighDateOrdinal(theHigh); + computeValueLowDateOrdinal(theLow); myOriginalValue = theOriginalValue; } + private void computeValueHighDateOrdinal(Date theHigh) { + this.myValueHighDateOrdinal = generateOrdinalDateInteger(theHigh); + } + private int generateOrdinalDateInteger(Date theDate) { + Calendar calendar = DateUtils.toCalendar(theDate); + String ordinalDateString = new StringBuilder() + .append(calendar.get(Calendar.YEAR)) + .append(calendar.get(Calendar.MONTH)) + .append(calendar.get(Calendar.DAY_OF_MONTH)) + .toString(); + return Integer.parseInt(ordinalDateString); + } + + private void computeValueLowDateOrdinal(Date theLow) { + this.myValueLowDateOrdinal = generateOrdinalDateInteger(theLow); + } + @Override @PrePersist public void calculateHashes() { From 34d5925393f2fae9b0edee790a24563422acbfae Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Tue, 25 Feb 2020 09:37:57 -0500 Subject: [PATCH 03/27] Move common work to DateUtils. Rename function for clarity. --- .../uhn/fhir/rest/param/DateRangeParam.java | 42 +++++++++---------- .../main/java/ca/uhn/fhir/util/DateUtils.java | 24 +++++++++++ .../dao/predicate/PredicateBuilderDate.java | 22 +++++----- 3 files changed, 54 insertions(+), 34 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java index 8b9a6245b16..d5f491a0f07 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java @@ -6,7 +6,7 @@ import ca.uhn.fhir.model.api.TemporalPrecisionEnum; import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.rest.api.QualifiedParamList; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; -import org.apache.commons.lang3.time.DateUtils; +import ca.uhn.fhir.util.DateUtils; import org.hl7.fhir.instance.model.api.IPrimitiveType; import java.util.*; @@ -262,21 +262,29 @@ public class DateRangeParam implements IQueryParameterAnd { validateAndSet(myLowerBound, new DateParam(ParamPrefixEnum.LESSTHAN, theUpperBound)); return this; } - public Integer getLowerBoundAsDateOrdinal() { + + /** + * Return the current lower bound as an integer representative of the date. + * + * e.g. 2019-02-22T04:22:00-0500 -> 20120922 + */ + public Integer getLowerBoundAsDateInteger() { if (myLowerBound == null || myLowerBound.getValue() == null) { return null; } - Calendar cal = DateUtils.toCalendar(myLowerBound.getValue()); - String s = new StringBuilder().append(cal.get(Calendar.YEAR)).append(cal.get(Calendar.MONTH)).append(cal.get(Calendar.DAY_OF_MONTH)).toString(); - return Integer.parseInt(s); + return DateUtils.convertDatetoDayInteger(myLowerBound.getValue()); } - public Integer getUpperBoundAsDateOrdinal() { + + /** + * Return the current upper bound as an integer representative of the date + * + * e.g. 2019-02-22T04:22:00-0500 -> 2019122 + */ + public Integer getUpperBoundAsDateInteger() { if (myUpperBound == null || myUpperBound.getValue() == null) { return null; } - Calendar cal = DateUtils.toCalendar(myUpperBound.getValue()); - String s = new StringBuilder().append(cal.get(Calendar.YEAR)).append(cal.get(Calendar.MONTH)).append(cal.get(Calendar.DAY_OF_MONTH)).toString(); - return Integer.parseInt(s); + return DateUtils.convertDatetoDayInteger(myUpperBound.getValue()); } public Date getLowerBoundAsInstant() { @@ -286,10 +294,7 @@ public class DateRangeParam implements IQueryParameterAnd { Date retVal = myLowerBound.getValue(); if (myLowerBound.getPrecision().ordinal() <= TemporalPrecisionEnum.DAY.ordinal()) { - Calendar cal = DateUtils.toCalendar(retVal); - cal.setTimeZone(TimeZone.getTimeZone("GMT-11:30")); - cal = DateUtils.truncate(cal, Calendar.DATE); - retVal = cal.getTime(); + retVal = DateUtils.getLowestInstantFromDate(retVal); } if (myLowerBound.getPrefix() != null) { @@ -350,17 +355,8 @@ public class DateRangeParam implements IQueryParameterAnd { Date retVal = myUpperBound.getValue(); - - //if (myUpperBound.getPrecision().ordinal() == TemporalPrecisionEnum.DAY.ordinal()) { - // DateUtils.setHours(retVal, 23); - // DateUtils.setMinutes(retVal, 59); -// DateUtils.setSeconds(retVal, 59); -// } if (myUpperBound.getPrecision().ordinal() <= TemporalPrecisionEnum.DAY.ordinal()) { - Calendar cal = DateUtils.toCalendar(retVal); - cal.setTimeZone(TimeZone.getTimeZone("GMT+11:30")); - cal = DateUtils.truncate(cal, Calendar.DATE); - retVal = cal.getTime(); + retVal = DateUtils.getHighestInstantFromDate(retVal); } if (myUpperBound.getPrefix() != null) { diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java index f99ea685652..70b742452f2 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java @@ -153,6 +153,30 @@ public final class DateUtils { return null; } + + public static Date getHighestInstantFromDate(Date theDateValue) { + return getInstantFromDateWithTimezone(theDateValue, TimeZone.getTimeZone("GMT+11:30")); + + } + public static Date getLowestInstantFromDate(Date theDateValue) { + return getInstantFromDateWithTimezone(theDateValue, TimeZone.getTimeZone("GMT-11:30")); + } + + public static Date getInstantFromDateWithTimezone(Date theDateValue, TimeZone theTimezone) { + Calendar cal = org.apache.commons.lang3.time.DateUtils.toCalendar(theDateValue); + cal.setTimeZone(theTimezone); + cal = org.apache.commons.lang3.time.DateUtils.truncate(cal, Calendar.DATE); + return cal.getTime(); + } + + + public static int convertDatetoDayInteger(final Date theDateValue) { + notNull(theDateValue, "Date value"); + Calendar cal = org.apache.commons.lang3.time.DateUtils.toCalendar(theDateValue); + String s = String.valueOf(cal.get(Calendar.YEAR)) + cal.get(Calendar.MONTH) + cal.get(Calendar.DAY_OF_MONTH); + return Integer.parseInt(s); + } + /** * Formats the given date according to the RFC 1123 pattern. * diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java index b269d4b787f..a04c8a674f5 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java @@ -152,7 +152,7 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi } //im like 80% sure this should be ub and not lb, as it is an UPPER bound. if (isOrdinalComparison) { - lb = theBuilder.lessThan(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateOrdinal()); + lb = theBuilder.lessThan(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateInteger()); } else { lb = theBuilder.lessThan(theFrom.get("myValueLow"), lowerBoundInstant); } @@ -162,7 +162,7 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi } //im like 80% sure this should be ub and not lb, as it is an UPPER bound. if (isOrdinalComparison) { - lb = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateOrdinal()); + lb = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateInteger()); } else { lb = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); } @@ -171,7 +171,7 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi throw new InvalidRequestException("upperBound value not correctly specified for compare operation"); } if (isOrdinalComparison) { - lb = theBuilder.greaterThan(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateOrdinal()); + lb = theBuilder.greaterThan(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateInteger()); } else { lb = theBuilder.greaterThan(theFrom.get("myValueHigh"), upperBoundInstant); } @@ -180,7 +180,7 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi throw new InvalidRequestException("lowerBound value not correctly specified for compare operation"); } if (isOrdinalComparison) { - lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateOrdinal()); + lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateInteger()); } else { lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLow"), lowerBoundInstant); } @@ -190,8 +190,8 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi throw new InvalidRequestException("lowerBound and/or upperBound value not correctly specified for compare operation"); } if (isOrdinalComparison){ - lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateOrdinal()); - gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateOrdinal()); + lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateInteger()); + gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateInteger()); } else { lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLow"), lowerBoundInstant); gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); @@ -201,8 +201,8 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi } else if ((operation == SearchFilterParser.CompareOperation.eq) || (operation == null)) { if (lowerBoundInstant != null) { if (isOrdinalComparison) { - gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateOrdinal()); - lt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getLowerBoundAsDateOrdinal()); + gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateInteger()); + lt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getLowerBoundAsDateInteger()); //also try a strict equality here. } else { @@ -218,8 +218,8 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi if (upperBoundInstant != null) { if (isOrdinalComparison) { - gt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getUpperBoundAsDateOrdinal()); - lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateOrdinal()); + gt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getUpperBoundAsDateInteger()); + lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateInteger()); } else { gt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLow"), upperBoundInstant); lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); @@ -235,7 +235,7 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi operation.name())); } if (isOrdinalComparison) { - ourLog.trace("Ordinal date range is {} - {} ", theRange.getLowerBoundAsDateOrdinal(), theRange.getUpperBoundAsDateOrdinal()); + ourLog.trace("Ordinal date range is {} - {} ", theRange.getLowerBoundAsDateInteger(), theRange.getUpperBoundAsDateInteger()); } ourLog.trace("Date range is {} - {}", lowerBoundInstant, upperBoundInstant); From 3b41b5b9601a69b4febbd4152f412abc435bc107 Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Tue, 25 Feb 2020 11:18:39 -0500 Subject: [PATCH 04/27] Type migration to boxed integer to avoid int defaulting to 0 --- .../dao/predicate/PredicateBuilderDate.java | 19 ++++++++++--------- .../tasks/HapiFhirJpaMigrationTasks.java | 1 + .../ResourceIndexedSearchParamDate.java | 19 ++++++++----------- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java index a04c8a674f5..eebf0f2987f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java @@ -176,15 +176,15 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi lb = theBuilder.greaterThan(theFrom.get("myValueHigh"), upperBoundInstant); } } else if (operation == SearchFilterParser.CompareOperation.ge) { - if (lowerBoundInstant == null) { - throw new InvalidRequestException("lowerBound value not correctly specified for compare operation"); - } - if (isOrdinalComparison) { - lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateInteger()); - } else { - lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLow"), lowerBoundInstant); - } - } else if (operation == SearchFilterParser.CompareOperation.ne) { + if (lowerBoundInstant == null) { + throw new InvalidRequestException("lowerBound value not correctly specified for compare operation"); + } + if (isOrdinalComparison) { + lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateInteger()); + } else { + lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLow"), lowerBoundInstant); + } + } else if (operation == SearchFilterParser.CompareOperation.ne) { if ((lowerBoundInstant == null) || (upperBoundInstant == null)) { throw new InvalidRequestException("lowerBound and/or upperBound value not correctly specified for compare operation"); @@ -224,6 +224,7 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi gt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLow"), upperBoundInstant); lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); } + if (theRange.getUpperBound().getPrefix() == ParamPrefixEnum.ENDS_BEFORE || theRange.getUpperBound().getPrefix() == ParamPrefixEnum.EQUAL) { ub = lt; } else { diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index 7731f589210..22ee5267be5 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -68,6 +68,7 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { version.onTable("HFJ_RES_VER").dropColumn("20200218.2", "FORCED_ID_PID"); version.onTable("HFJ_RES_VER").addForeignKey("20200218.3", "FK_RESOURCE_HISTORY_RESOURCE").toColumn("RES_ID").references("HFJ_RESOURCE", "RES_ID"); version.onTable("HFJ_RES_VER").modifyColumn("20200220.1", "RES_ID").nonNullable().failureAllowed().withType(BaseTableColumnTypeTask.ColumnTypeEnum.LONG); + // } protected void init420() { // 20191015 - 20200217 diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java index e355cce822b..eb5e6097c6b 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java @@ -29,12 +29,10 @@ import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; -import org.apache.commons.lang3.time.DateUtils; import org.hibernate.search.annotations.Field; import org.hl7.fhir.r4.model.DateTimeType; import javax.persistence.*; -import java.util.Calendar; import java.util.Date; @Embeddable @@ -57,10 +55,15 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar @Field public Date myValueLow; + /** + * Field which stores an integer representation of YYYYMDD as calculated by Calendar + * e.g. 2019-01-20 -> 2019020 + * (note that the month is 0 since calendar month counting starts at 0. + */ @Column(name="SP_VALUE_LOW_DATE_ORDINAL") - public int myValueLowDateOrdinal; + public Integer myValueLowDateOrdinal; @Column(name="SP_VALUE_HIGH_DATE_ORDINAL") - public int myValueHighDateOrdinal; + public Integer myValueHighDateOrdinal; @Transient private transient String myOriginalValue; @@ -99,13 +102,7 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar this.myValueHighDateOrdinal = generateOrdinalDateInteger(theHigh); } private int generateOrdinalDateInteger(Date theDate) { - Calendar calendar = DateUtils.toCalendar(theDate); - String ordinalDateString = new StringBuilder() - .append(calendar.get(Calendar.YEAR)) - .append(calendar.get(Calendar.MONTH)) - .append(calendar.get(Calendar.DAY_OF_MONTH)) - .toString(); - return Integer.parseInt(ordinalDateString); + return ca.uhn.fhir.util.DateUtils.convertDatetoDayInteger(theDate); } private void computeValueLowDateOrdinal(Date theLow) { From 13cc8b69aaaa3c825e1a2931a8b895bb4eb9dadb Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Tue, 25 Feb 2020 11:27:08 -0500 Subject: [PATCH 05/27] Add schema migration --- .../fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index 22ee5267be5..05b94668006 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -69,6 +69,11 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { version.onTable("HFJ_RES_VER").addForeignKey("20200218.3", "FK_RESOURCE_HISTORY_RESOURCE").toColumn("RES_ID").references("HFJ_RESOURCE", "RES_ID"); version.onTable("HFJ_RES_VER").modifyColumn("20200220.1", "RES_ID").nonNullable().failureAllowed().withType(BaseTableColumnTypeTask.ColumnTypeEnum.LONG); // + + // Add support for integer comparisons during day-granularity date search. + version.onTable("HFJ_SPIDX_DATE").addColumn("20200225.1", "SP_VALUE_LOW_DATE_ORDINAL").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.INT); + version.onTable("HFJ_SPIDX_DATE").addColumn("20200225.1", "SP_VALUE_HIGH_DATE_ORDINAL").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.INT); + // } protected void init420() { // 20191015 - 20200217 From 3ec772b527f89dc40ff7d632cdb906e72a0d28c8 Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Tue, 25 Feb 2020 14:06:36 -0500 Subject: [PATCH 06/27] rework ResourceIndexedSearchParamDate constructor to support incoming string values --- .../main/java/ca/uhn/fhir/util/DateUtils.java | 7 +- .../r4/FhirResourceDaoR4SearchNoFtTest.java | 2 +- .../taskdef/CalculateOrdinalDateTask.java | 264 ++++++++++++++++++ .../tasks/HapiFhirJpaMigrationTasks.java | 10 +- .../ResourceIndexedSearchParamDate.java | 16 +- .../ResourceIndexedSearchParamDateTest.java | 32 +-- .../extractor/BaseSearchParamExtractor.java | 13 +- .../InMemoryResourceMatcherR5Test.java | 2 +- 8 files changed, 315 insertions(+), 31 deletions(-) create mode 100644 hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDateTask.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java index 70b742452f2..c9697b2cf26 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java @@ -65,6 +65,8 @@ public final class DateUtils { @SuppressWarnings("WeakerAccess") public static final String PATTERN_ASCTIME = "EEE MMM d HH:mm:ss yyyy"; + private static final String PATTERN_INTEGER_DATE = "yyyyMMdd"; + private static final String[] DEFAULT_PATTERNS = new String[]{ PATTERN_RFC1123, PATTERN_RFC1036, @@ -173,8 +175,11 @@ public final class DateUtils { public static int convertDatetoDayInteger(final Date theDateValue) { notNull(theDateValue, "Date value"); Calendar cal = org.apache.commons.lang3.time.DateUtils.toCalendar(theDateValue); + SimpleDateFormat format = new SimpleDateFormat(PATTERN_INTEGER_DATE); + String theDateString = format.format(theDateValue); + String s = String.valueOf(cal.get(Calendar.YEAR)) + cal.get(Calendar.MONTH) + cal.get(Calendar.DAY_OF_MONTH); - return Integer.parseInt(s); + return Integer.parseInt(theDateString); } /** diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index 66687daf9dc..5e7e20702da 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -3960,7 +3960,6 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { assertEquals(1, results.getResources(0, 10).size()); // We expect a new one because we don't cache the search URL for very long search URLs assertEquals(2, mySearchEntityDao.count()); - } @Test @@ -4094,6 +4093,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { obs.setId(theId); obs.setEffective(new DateTimeType(theEffective)); myObservationDao.update(obs); + ourLog.info("Obs {} has time {}", theId, obs.getEffectiveDateTimeType().getValue().toString()); } diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDateTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDateTask.java new file mode 100644 index 00000000000..d77e23a9cb9 --- /dev/null +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDateTask.java @@ -0,0 +1,264 @@ +package ca.uhn.fhir.jpa.migrate.taskdef; + +/*- + * #%L + * HAPI FHIR JPA Server - Migration + * %% + * Copyright (C) 2014 - 2020 University Health Network + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + +import ca.uhn.fhir.jpa.migrate.JdbcUtils; +import ca.uhn.fhir.util.StopWatch; +import ca.uhn.fhir.util.VersionEnum; +import com.google.common.collect.ForwardingMap; +import org.apache.commons.lang3.Validate; +import org.apache.commons.lang3.concurrent.BasicThreadFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.jdbc.core.ColumnMapRowMapper; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.core.RowCallbackHandler; + +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.*; +import java.util.concurrent.*; +import java.util.function.Function; + +public class CalculateOrdinalDateTask extends BaseTableColumnTask { + + private static final Logger ourLog = LoggerFactory.getLogger(CalculateOrdinalDateTask.class); + private int myBatchSize = 10000; + private Map, Long>> myCalculators = new HashMap<>(); + private ThreadPoolExecutor myExecutor; + + public void setBatchSize(int theBatchSize) { + myBatchSize = theBatchSize; + } + + /** + * Constructor + */ + public CalculateOrdinalDateTask(VersionEnum theRelease, String theVersion) { + super(theRelease.toString(), theVersion); + setDescription("Calculate resource search parameter index hashes"); + } + + @Override + public synchronized void doExecute() throws SQLException { + if (isDryRun()) { + return; + } + + Set tableNames = JdbcUtils.getTableNames(getConnectionProperties()); + initializeExecutor(); + try { + + while(true) { + MyRowCallbackHandler rch = new MyRowCallbackHandler(); + getTxTemplate().execute(t -> { + JdbcTemplate jdbcTemplate = newJdbcTemplate(); + jdbcTemplate.setMaxRows(100000); + String sql = "SELECT * FROM " + getTableName() + " WHERE " + getColumnName() + " IS NULL"; + logInfo(ourLog, "Finding up to {} rows in {} that requires hashes", myBatchSize, getTableName()); + + jdbcTemplate.query(sql, rch); + rch.done(); + + return null; + }); + + rch.submitNext(); + List> futures = rch.getFutures(); + if (futures.isEmpty()) { + break; + } + + logInfo(ourLog, "Waiting for {} tasks to complete", futures.size()); + for (Future next : futures) { + try { + next.get(); + } catch (Exception e) { + throw new SQLException(e); + } + } + + } + + } finally { + destroyExecutor(); + } + } + + private void destroyExecutor() { + myExecutor.shutdownNow(); + } + + private void initializeExecutor() { + int maximumPoolSize = Runtime.getRuntime().availableProcessors(); + + LinkedBlockingQueue executorQueue = new LinkedBlockingQueue<>(maximumPoolSize); + BasicThreadFactory threadFactory = new BasicThreadFactory.Builder() + .namingPattern("worker-" + "-%d") + .daemon(false) + .priority(Thread.NORM_PRIORITY) + .build(); + RejectedExecutionHandler rejectedExecutionHandler = new RejectedExecutionHandler() { + @Override + public void rejectedExecution(Runnable theRunnable, ThreadPoolExecutor theExecutor) { + logInfo(ourLog, "Note: Executor queue is full ({} elements), waiting for a slot to become available!", executorQueue.size()); + StopWatch sw = new StopWatch(); + try { + executorQueue.put(theRunnable); + } catch (InterruptedException theE) { + throw new RejectedExecutionException("Task " + theRunnable.toString() + + " rejected from " + theE.toString()); + } + logInfo(ourLog, "Slot become available after {}ms", sw.getMillis()); + } + }; + myExecutor = new ThreadPoolExecutor( + 1, + maximumPoolSize, + 0L, + TimeUnit.MILLISECONDS, + executorQueue, + threadFactory, + rejectedExecutionHandler); + } + + private Future updateRows(List> theRows) { + Runnable task = () -> { + StopWatch sw = new StopWatch(); + getTxTemplate().execute(t -> { + + // Loop through rows + assert theRows != null; + for (Map nextRow : theRows) { + + Map newValues = new HashMap<>(); + MandatoryKeyMap nextRowMandatoryKeyMap = new MandatoryKeyMap<>(nextRow); + + // Apply calculators + for (Map.Entry, Long>> nextCalculatorEntry : myCalculators.entrySet()) { + String nextColumn = nextCalculatorEntry.getKey(); + Function, Long> nextCalculator = nextCalculatorEntry.getValue(); + Long value = nextCalculator.apply(nextRowMandatoryKeyMap); + newValues.put(nextColumn, value); + } + + // Generate update SQL + StringBuilder sqlBuilder = new StringBuilder(); + List arguments = new ArrayList<>(); + sqlBuilder.append("UPDATE "); + sqlBuilder.append(getTableName()); + sqlBuilder.append(" SET "); + for (Map.Entry nextNewValueEntry : newValues.entrySet()) { + if (arguments.size() > 0) { + sqlBuilder.append(", "); + } + sqlBuilder.append(nextNewValueEntry.getKey()).append(" = ?"); + arguments.add(nextNewValueEntry.getValue()); + } + sqlBuilder.append(" WHERE SP_ID = ?"); + arguments.add((Number) nextRow.get("SP_ID")); + + // Apply update SQL + newJdbcTemplate().update(sqlBuilder.toString(), arguments.toArray()); + + } + + return theRows.size(); + }); + logInfo(ourLog, "Updated {} rows on {} in {}", theRows.size(), getTableName(), sw.toString()); + }; + return myExecutor.submit(task); + } + + public CalculateOrdinalDateTask addCalculator(String theColumnName, Function, Long> theConsumer) { + Validate.isTrue(myCalculators.containsKey(theColumnName) == false); + myCalculators.put(theColumnName, theConsumer); + return this; + } + + private class MyRowCallbackHandler implements RowCallbackHandler { + + private List> myRows = new ArrayList<>(); + private List> myFutures = new ArrayList<>(); + + @Override + public void processRow(ResultSet rs) throws SQLException { + Map row = new ColumnMapRowMapper().mapRow(rs, 0); + myRows.add(row); + + if (myRows.size() >= myBatchSize) { + submitNext(); + } + } + + private void submitNext() { + if (myRows.size() > 0) { + myFutures.add(updateRows(myRows)); + myRows = new ArrayList<>(); + } + } + + public List> getFutures() { + return myFutures; + } + + public void done() { + if (myRows.size() > 0) { + submitNext(); + } + } + } + + + public static class MandatoryKeyMap extends ForwardingMap { + + private final Map myWrap; + + public MandatoryKeyMap(Map theWrap) { + myWrap = theWrap; + } + + @Override + public V get(Object theKey) { + if (!containsKey(theKey)) { + throw new IllegalArgumentException("No key: " + theKey); + } + return super.get(theKey); + } + + public String getString(String theKey) { + return (String) get(theKey); + } + + @Override + protected Map delegate() { + return myWrap; + } + + public String getResourceType() { + return getString("RES_TYPE"); + } + + public String getParamName() { + return getString("SP_NAME"); + } + } +} diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index 05b94668006..59f5720002b 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -71,8 +71,14 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { // // Add support for integer comparisons during day-granularity date search. - version.onTable("HFJ_SPIDX_DATE").addColumn("20200225.1", "SP_VALUE_LOW_DATE_ORDINAL").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.INT); - version.onTable("HFJ_SPIDX_DATE").addColumn("20200225.1", "SP_VALUE_HIGH_DATE_ORDINAL").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.INT); + Builder.BuilderWithTableName spidxDate = version.onTable("HFJ_SPIDX_DATE"); + spidxDate.addColumn("20200225.1", "SP_VALUE_LOW_DATE_ORDINAL").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.INT); + spidxDate.addColumn("20200225.2", "SP_VALUE_HIGH_DATE_ORDINAL").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.INT); + spidxDate.addTask(new CalculateHashesTask(VersionEnum.V4_3_0, "20200225.3") + .setColumnName("HASH_IDENTITY") + .addCalculator("SP_VALUE_LOW_DATE_ORDINAL", t -> BaseResourceIndexedSearchParam.calculateHashIdentity(t.getResourceType(), t.getString("SP_NAME"))) + .addCalculator("SP_VALUE_HIGH_DATE_ORDINAL", t -> BaseResourceIndexedSearchParam.calculateHashIdentity(t.getResourceType(), t.getString("SP_NAME"))) + ); // } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java index eb5e6097c6b..d4a19d9f022 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java @@ -88,24 +88,26 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar /** * Constructor */ - public ResourceIndexedSearchParamDate(String theResourceType, String theParamName, Date theLow, Date theHigh, String theOriginalValue) { + public ResourceIndexedSearchParamDate(String theResourceType, String theParamName, Date theLow, String theLowString, Date theHigh, String theHighString, String theOriginalValue) { setResourceType(theResourceType); setParamName(theParamName); setValueLow(theLow); setValueHigh(theHigh); - computeValueHighDateOrdinal(theHigh); - computeValueLowDateOrdinal(theLow); + computeValueHighDateOrdinal(theHighString); + computeValueLowDateOrdinal(theLowString); myOriginalValue = theOriginalValue; } - private void computeValueHighDateOrdinal(Date theHigh) { + private void computeValueHighDateOrdinal(String theHigh) { this.myValueHighDateOrdinal = generateOrdinalDateInteger(theHigh); } - private int generateOrdinalDateInteger(Date theDate) { - return ca.uhn.fhir.util.DateUtils.convertDatetoDayInteger(theDate); + private int generateOrdinalDateInteger(String theDateString){ + String t = theDateString.substring(0, theDateString.indexOf("T")); + t = t.replace("-", ""); + return Integer.valueOf(t); } - private void computeValueLowDateOrdinal(Date theLow) { + private void computeValueLowDateOrdinal(String theLow) { this.myValueLowDateOrdinal = generateOrdinalDateInteger(theLow); } diff --git a/hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDateTest.java b/hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDateTest.java index c45245aa3b1..407d05163f5 100644 --- a/hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDateTest.java +++ b/hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDateTest.java @@ -35,8 +35,8 @@ public class ResourceIndexedSearchParamDateTest { @Test public void equalsIsTrueForMatchingNullDates() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", null, null, "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", null, null, "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", null, null, null, null, "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", null, null, null, null, "SomeValue"); assertTrue(param.equals(param2)); assertTrue(param2.equals(param)); @@ -45,8 +45,8 @@ public class ResourceIndexedSearchParamDateTest { @Test public void equalsIsTrueForMatchingDates() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date2A, "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1B, date2B, "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date1A.toString(), date2A, date2A.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1B, date1B.toString(), date2B, date2B.toString(), "SomeValue"); assertTrue(param.equals(param2)); assertTrue(param2.equals(param)); @@ -55,8 +55,8 @@ public class ResourceIndexedSearchParamDateTest { @Test public void equalsIsTrueForMatchingTimeStampsThatMatch() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1A, timestamp2A, "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1B, timestamp2B, "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1A, timestamp1A.toString(), timestamp2A, timestamp2A.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1B, timestamp1B.toString(), timestamp2B, timestamp2B.toString(), "SomeValue"); assertTrue(param.equals(param2)); assertTrue(param2.equals(param)); @@ -67,8 +67,8 @@ public class ResourceIndexedSearchParamDateTest { // other will be equivalent but will be a java.sql.Timestamp. Equals should work in both directions. @Test public void equalsIsTrueForMixedTimestampsAndDates() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date2A, "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1A, timestamp2A, "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date1A.toString(), date2A, date2A.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1A, timestamp1A.toString(), timestamp2A, timestamp2A.toString(), "SomeValue"); assertTrue(param.equals(param2)); assertTrue(param2.equals(param)); @@ -77,8 +77,8 @@ public class ResourceIndexedSearchParamDateTest { @Test public void equalsIsFalseForNonMatchingDates() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date2A, "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date2A, date1A, "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date1A.toString(), date2A, date2A.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date2A, date2A.toString(), date1A, date1A.toString(), "SomeValue"); assertFalse(param.equals(param2)); assertFalse(param2.equals(param)); @@ -87,8 +87,8 @@ public class ResourceIndexedSearchParamDateTest { @Test public void equalsIsFalseForNonMatchingDatesNullCase() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date2A, "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", null, null, "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date1A.toString(), date2A, date2A.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", null, null, null, null, "SomeValue"); assertFalse(param.equals(param2)); assertFalse(param2.equals(param)); @@ -97,8 +97,8 @@ public class ResourceIndexedSearchParamDateTest { @Test public void equalsIsFalseForNonMatchingTimeStamps() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1A, timestamp2A, "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp2A, timestamp1A, "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1A, timestamp1A.toString(), timestamp2A, timestamp2A.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp2A, timestamp2A.toString(), timestamp1A, timestamp1A.toString(), "SomeValue"); assertFalse(param.equals(param2)); assertFalse(param2.equals(param)); @@ -107,8 +107,8 @@ public class ResourceIndexedSearchParamDateTest { @Test public void equalsIsFalseForMixedTimestampsAndDatesThatDoNotMatch() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date2A, "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp2A, timestamp1A, "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date1A.toString(), date2A, date2A.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp2A, timestamp2A.toString(), timestamp1A, timestamp1A.toString(), "SomeValue"); assertFalse(param.equals(param2)); assertFalse(param2.equals(param)); diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java index 79bf1c41f4a..d3b42491c51 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java @@ -605,9 +605,10 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor Date start = extractValueAsDate(myPeriodStartValueChild, theValue); String startAsString = extractValueAsString(myPeriodStartValueChild, theValue); Date end = extractValueAsDate(myPeriodEndValueChild, theValue); + String endAsString = extractValueAsString(myPeriodEndValueChild, theValue); if (start != null || end != null) { - ResourceIndexedSearchParamDate nextEntity = new ResourceIndexedSearchParamDate(theResourceType, theSearchParam.getName(), start, end, startAsString); + ResourceIndexedSearchParamDate nextEntity = new ResourceIndexedSearchParamDate(theResourceType, theSearchParam.getName(), start, startAsString, end, endAsString, startAsString); theParams.add(nextEntity); } } @@ -616,13 +617,17 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor List> values = extractValuesAsFhirDates(myTimingEventValueChild, theValue); TreeSet dates = new TreeSet<>(); + TreeSet dateStrings = new TreeSet<>(); String firstValue = null; + String finalValue = null; for (IPrimitiveType nextEvent : values) { if (nextEvent.getValue() != null) { dates.add(nextEvent.getValue()); + if (firstValue == null) { firstValue = nextEvent.getValueAsString(); } + finalValue = nextEvent.getValueAsString(); } } @@ -634,14 +639,16 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor if ("Period".equals(boundsType)) { Date start = extractValueAsDate(myPeriodStartValueChild, bounds.get()); Date end = extractValueAsDate(myPeriodEndValueChild, bounds.get()); + String endString = extractValueAsString(myPeriodEndValueChild, bounds.get()); dates.add(start); dates.add(end); + finalValue = endString; } } } if (!dates.isEmpty()) { - ResourceIndexedSearchParamDate nextEntity = new ResourceIndexedSearchParamDate(theResourceType, theSearchParam.getName(), dates.first(), dates.last(), firstValue); + ResourceIndexedSearchParamDate nextEntity = new ResourceIndexedSearchParamDate(theResourceType, theSearchParam.getName(), dates.first(), firstValue, dates.last(), finalValue, firstValue); theParams.add(nextEntity); } } @@ -828,7 +835,7 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor private void addDateTimeTypes(String theResourceType, Set theParams, RuntimeSearchParam theSearchParam, IBase theValue) { IPrimitiveType nextBaseDateTime = (IPrimitiveType) theValue; if (nextBaseDateTime.getValue() != null) { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate(theResourceType, theSearchParam.getName(), nextBaseDateTime.getValue(), nextBaseDateTime.getValue(), nextBaseDateTime.getValueAsString()); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate(theResourceType, theSearchParam.getName(), nextBaseDateTime.getValue(), nextBaseDateTime.getValueAsString(), nextBaseDateTime.getValue(), nextBaseDateTime.getValueAsString(), nextBaseDateTime.getValueAsString()); theParams.add(param); } } diff --git a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcherR5Test.java b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcherR5Test.java index 2a80eb7686e..bb111f87e86 100644 --- a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcherR5Test.java +++ b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcherR5Test.java @@ -208,7 +208,7 @@ public class InMemoryResourceMatcherR5Test { private ResourceIndexedSearchParams extractDateSearchParam(Observation theObservation) { ResourceIndexedSearchParams retval = new ResourceIndexedSearchParams(); BaseDateTimeType dateValue = (BaseDateTimeType) theObservation.getEffective(); - ResourceIndexedSearchParamDate dateParam = new ResourceIndexedSearchParamDate("Patient", "date", dateValue.getValue(), dateValue.getValue(), dateValue.getValueAsString()); + ResourceIndexedSearchParamDate dateParam = new ResourceIndexedSearchParamDate("Patient", "date", dateValue.getValue(), dateValue.getValueAsString(), dateValue.getValue(), dateValue.getValueAsString(), dateValue.getValueAsString()); retval.myDateParams.add(dateParam); return retval; } From da7224ff89ef6c5218399a96ef5944a1efe124da Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Tue, 25 Feb 2020 14:07:03 -0500 Subject: [PATCH 07/27] update broken test to reflect new reality of business logic of ordinal date comparison --- .../fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index 5e7e20702da..bef9cbfcab7 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -3965,9 +3965,9 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { @Test public void testDateSearchParametersShouldBeTimezoneIndependent() { - createObservationWithEffective("NO1", "2011-01-02T23:00:00-11:30"); - createObservationWithEffective("NO2", "2011-01-03T00:00:00+01:00"); + createObservationWithEffective("NO1", "2011-01-03T00:00:00+01:00"); + createObservationWithEffective("YES00", "2011-01-02T23:00:00-11:30"); createObservationWithEffective("YES01", "2011-01-02T00:00:00-11:30"); createObservationWithEffective("YES02", "2011-01-02T00:00:00-10:00"); createObservationWithEffective("YES03", "2011-01-02T00:00:00-09:00"); @@ -3992,7 +3992,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { createObservationWithEffective("YES22", "2011-01-02T00:00:00+10:00"); createObservationWithEffective("YES23", "2011-01-02T00:00:00+11:00"); - + SearchParameterMap map = new SearchParameterMap(); map.setLoadSynchronous(true); map.add(Observation.SP_DATE, new DateParam("2011-01-02")); @@ -4000,6 +4000,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { List values = toUnqualifiedVersionlessIdValues(results); Collections.sort(values); assertThat(values.toString(), values, contains( + "Observation/YES00", "Observation/YES01", "Observation/YES02", "Observation/YES03", From 512af5b87d21fd9f7dfd1ad1ea8c53b2725ca527 Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Tue, 25 Feb 2020 14:09:51 -0500 Subject: [PATCH 08/27] Add changelog entry --- .../4_3_0/1499-dont-touch-timezones-on-date-search.yaml | 5 +++++ .../uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1499-dont-touch-timezones-on-date-search.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1499-dont-touch-timezones-on-date-search.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1499-dont-touch-timezones-on-date-search.yaml new file mode 100644 index 00000000000..3595b792d68 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1499-dont-touch-timezones-on-date-search.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 1499 +title: When performing a search with a DateParam that has DAY precision, rely on new ordinal date field for comparison +instead of attempting to find oldest and newest instant that could be valid. diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index bef9cbfcab7..a187ade6c33 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -3992,7 +3992,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { createObservationWithEffective("YES22", "2011-01-02T00:00:00+10:00"); createObservationWithEffective("YES23", "2011-01-02T00:00:00+11:00"); - + SearchParameterMap map = new SearchParameterMap(); map.setLoadSynchronous(true); map.add(Observation.SP_DATE, new DateParam("2011-01-02")); From 97c98254d0db82b0b34140e99e9ef85262ca931a Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Tue, 25 Feb 2020 16:09:08 -0500 Subject: [PATCH 09/27] Extract Common functionality out of CalculateHashesTask, build CalculateOrdinalDatesTask based on it --- .../main/java/ca/uhn/fhir/util/DateUtils.java | 2 - ...9-dont-touch-timezones-on-date-search.yaml | 2 +- ...ask.java => BaseColumnCalculatorTask.java} | 41 ++-- .../fhir/jpa/migrate/taskdef/BaseTask.java | 2 +- .../migrate/taskdef/CalculateHashesTask.java | 230 +----------------- .../taskdef/CalculateOrdinalDatesTask.java | 35 +++ .../tasks/HapiFhirJpaMigrationTasks.java | 18 +- .../ResourceIndexedSearchParamDate.java | 8 +- 8 files changed, 86 insertions(+), 252 deletions(-) rename hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/{CalculateOrdinalDateTask.java => BaseColumnCalculatorTask.java} (83%) create mode 100644 hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java index c9697b2cf26..f34a0ee12e5 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java @@ -177,8 +177,6 @@ public final class DateUtils { Calendar cal = org.apache.commons.lang3.time.DateUtils.toCalendar(theDateValue); SimpleDateFormat format = new SimpleDateFormat(PATTERN_INTEGER_DATE); String theDateString = format.format(theDateValue); - - String s = String.valueOf(cal.get(Calendar.YEAR)) + cal.get(Calendar.MONTH) + cal.get(Calendar.DAY_OF_MONTH); return Integer.parseInt(theDateString); } diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1499-dont-touch-timezones-on-date-search.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1499-dont-touch-timezones-on-date-search.yaml index 3595b792d68..c8619a3a602 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1499-dont-touch-timezones-on-date-search.yaml +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1499-dont-touch-timezones-on-date-search.yaml @@ -2,4 +2,4 @@ type: fix issue: 1499 title: When performing a search with a DateParam that has DAY precision, rely on new ordinal date field for comparison -instead of attempting to find oldest and newest instant that could be valid. +instead of attempting to find oldest and newest instant that could be valid. diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDateTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseColumnCalculatorTask.java similarity index 83% rename from hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDateTask.java rename to hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseColumnCalculatorTask.java index d77e23a9cb9..ddd5300bfcb 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDateTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseColumnCalculatorTask.java @@ -38,11 +38,11 @@ import java.util.*; import java.util.concurrent.*; import java.util.function.Function; -public class CalculateOrdinalDateTask extends BaseTableColumnTask { +public abstract class BaseColumnCalculatorTask extends BaseTableColumnTask { - private static final Logger ourLog = LoggerFactory.getLogger(CalculateOrdinalDateTask.class); + protected static final Logger ourLog = LoggerFactory.getLogger(BaseColumnCalculatorTask.class); private int myBatchSize = 10000; - private Map, Long>> myCalculators = new HashMap<>(); + private Map, Object>> myCalculators = new HashMap<>(); private ThreadPoolExecutor myExecutor; public void setBatchSize(int theBatchSize) { @@ -52,19 +52,24 @@ public class CalculateOrdinalDateTask extends BaseTableColumnTask tableNames = JdbcUtils.getTableNames(getConnectionProperties()); initializeExecutor(); + try { while(true) { @@ -73,7 +78,7 @@ public class CalculateOrdinalDateTask extends BaseTableColumnTask nextRow : theRows) { - Map newValues = new HashMap<>(); + Map newValues = new HashMap<>(); MandatoryKeyMap nextRowMandatoryKeyMap = new MandatoryKeyMap<>(nextRow); // Apply calculators - for (Map.Entry, Long>> nextCalculatorEntry : myCalculators.entrySet()) { + for (Map.Entry, Object>> nextCalculatorEntry : myCalculators.entrySet()) { String nextColumn = nextCalculatorEntry.getKey(); - Function, Long> nextCalculator = nextCalculatorEntry.getValue(); - Long value = nextCalculator.apply(nextRowMandatoryKeyMap); + Function, Object> nextCalculator = nextCalculatorEntry.getValue(); + Object value = nextCalculator.apply(nextRowMandatoryKeyMap); newValues.put(nextColumn, value); } // Generate update SQL StringBuilder sqlBuilder = new StringBuilder(); - List arguments = new ArrayList<>(); + List arguments = new ArrayList<>(); sqlBuilder.append("UPDATE "); sqlBuilder.append(getTableName()); sqlBuilder.append(" SET "); - for (Map.Entry nextNewValueEntry : newValues.entrySet()) { + for (Map.Entry nextNewValueEntry : newValues.entrySet()) { if (arguments.size() > 0) { sqlBuilder.append(", "); } @@ -178,9 +183,7 @@ public class CalculateOrdinalDateTask extends BaseTableColumnTask, Long> theConsumer) { + public BaseColumnCalculatorTask addCalculator(String theColumnName, Function, Object> theConsumer) { Validate.isTrue(myCalculators.containsKey(theColumnName) == false); myCalculators.put(theColumnName, theConsumer); return this; @@ -248,6 +251,10 @@ public class CalculateOrdinalDateTask extends BaseTableColumnTask delegate() { return myWrap; diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java index e68d1609158..1e104f4176a 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java @@ -165,7 +165,7 @@ public abstract class BaseTask { doExecute(); } - public abstract void doExecute() throws SQLException; + protected abstract void doExecute() throws SQLException; public void setFailureAllowed(boolean theFailureAllowed) { myFailureAllowed = theFailureAllowed; diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateHashesTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateHashesTask.java index cc60f48a428..b182a12c099 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateHashesTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateHashesTask.java @@ -38,234 +38,30 @@ import java.util.*; import java.util.concurrent.*; import java.util.function.Function; -public class CalculateHashesTask extends BaseTableColumnTask { - - private static final Logger ourLog = LoggerFactory.getLogger(CalculateHashesTask.class); - private int myBatchSize = 10000; - private Map, Long>> myCalculators = new HashMap<>(); - private ThreadPoolExecutor myExecutor; - - public void setBatchSize(int theBatchSize) { - myBatchSize = theBatchSize; - } +public class CalculateHashesTask extends BaseColumnCalculatorTask { /** * Constructor */ public CalculateHashesTask(VersionEnum theRelease, String theVersion) { - super(theRelease.toString(), theVersion); + super(theRelease, theVersion); setDescription("Calculate resource search parameter index hashes"); } @Override - public synchronized void doExecute() throws SQLException { - if (isDryRun()) { - return; - } - - Set tableNames = JdbcUtils.getTableNames(getConnectionProperties()); - // This table was added shortly after hash indexes were added, so it is a reasonable indicator for whether this - // migration has already been run - if (tableNames.contains("HFJ_RES_REINDEX_JOB")) { - logInfo(ourLog, "The table HFJ_RES_REINDEX_JOB already exists. Skipping calculate hashes task."); - return; - } - - initializeExecutor(); + protected boolean shouldSkipTask() { try { - - while(true) { - MyRowCallbackHandler rch = new MyRowCallbackHandler(); - getTxTemplate().execute(t -> { - JdbcTemplate jdbcTemplate = newJdbcTemplate(); - jdbcTemplate.setMaxRows(100000); - String sql = "SELECT * FROM " + getTableName() + " WHERE " + getColumnName() + " IS NULL"; - logInfo(ourLog, "Finding up to {} rows in {} that requires hashes", myBatchSize, getTableName()); - - jdbcTemplate.query(sql, rch); - rch.done(); - - return null; - }); - - rch.submitNext(); - List> futures = rch.getFutures(); - if (futures.isEmpty()) { - break; - } - - logInfo(ourLog, "Waiting for {} tasks to complete", futures.size()); - for (Future next : futures) { - try { - next.get(); - } catch (Exception e) { - throw new SQLException(e); - } - } - + Set tableNames = JdbcUtils.getTableNames(getConnectionProperties()); + boolean shouldSkip = tableNames.contains("HFJ_RES_REINDEX_JOB"); + // This table was added shortly after hash indexes were added, so it is a reasonable indicator for whether this + // migration has already been run + if (shouldSkip) { + logInfo(ourLog, "The table HFJ_RES_REINDEX_JOB already exists. Skipping calculate hashes task."); } - - } finally { - destroyExecutor(); - } - } - - private void destroyExecutor() { - myExecutor.shutdownNow(); - } - - private void initializeExecutor() { - int maximumPoolSize = Runtime.getRuntime().availableProcessors(); - - LinkedBlockingQueue executorQueue = new LinkedBlockingQueue<>(maximumPoolSize); - BasicThreadFactory threadFactory = new BasicThreadFactory.Builder() - .namingPattern("worker-" + "-%d") - .daemon(false) - .priority(Thread.NORM_PRIORITY) - .build(); - RejectedExecutionHandler rejectedExecutionHandler = new RejectedExecutionHandler() { - @Override - public void rejectedExecution(Runnable theRunnable, ThreadPoolExecutor theExecutor) { - logInfo(ourLog, "Note: Executor queue is full ({} elements), waiting for a slot to become available!", executorQueue.size()); - StopWatch sw = new StopWatch(); - try { - executorQueue.put(theRunnable); - } catch (InterruptedException theE) { - throw new RejectedExecutionException("Task " + theRunnable.toString() + - " rejected from " + theE.toString()); - } - logInfo(ourLog, "Slot become available after {}ms", sw.getMillis()); - } - }; - myExecutor = new ThreadPoolExecutor( - 1, - maximumPoolSize, - 0L, - TimeUnit.MILLISECONDS, - executorQueue, - threadFactory, - rejectedExecutionHandler); - } - - private Future updateRows(List> theRows) { - Runnable task = () -> { - StopWatch sw = new StopWatch(); - getTxTemplate().execute(t -> { - - // Loop through rows - assert theRows != null; - for (Map nextRow : theRows) { - - Map newValues = new HashMap<>(); - MandatoryKeyMap nextRowMandatoryKeyMap = new MandatoryKeyMap<>(nextRow); - - // Apply calculators - for (Map.Entry, Long>> nextCalculatorEntry : myCalculators.entrySet()) { - String nextColumn = nextCalculatorEntry.getKey(); - Function, Long> nextCalculator = nextCalculatorEntry.getValue(); - Long value = nextCalculator.apply(nextRowMandatoryKeyMap); - newValues.put(nextColumn, value); - } - - // Generate update SQL - StringBuilder sqlBuilder = new StringBuilder(); - List arguments = new ArrayList<>(); - sqlBuilder.append("UPDATE "); - sqlBuilder.append(getTableName()); - sqlBuilder.append(" SET "); - for (Map.Entry nextNewValueEntry : newValues.entrySet()) { - if (arguments.size() > 0) { - sqlBuilder.append(", "); - } - sqlBuilder.append(nextNewValueEntry.getKey()).append(" = ?"); - arguments.add(nextNewValueEntry.getValue()); - } - sqlBuilder.append(" WHERE SP_ID = ?"); - arguments.add((Number) nextRow.get("SP_ID")); - - // Apply update SQL - newJdbcTemplate().update(sqlBuilder.toString(), arguments.toArray()); - - } - - return theRows.size(); - }); - logInfo(ourLog, "Updated {} rows on {} in {}", theRows.size(), getTableName(), sw.toString()); - }; - return myExecutor.submit(task); - } - - public CalculateHashesTask addCalculator(String theColumnName, Function, Long> theConsumer) { - Validate.isTrue(myCalculators.containsKey(theColumnName) == false); - myCalculators.put(theColumnName, theConsumer); - return this; - } - - private class MyRowCallbackHandler implements RowCallbackHandler { - - private List> myRows = new ArrayList<>(); - private List> myFutures = new ArrayList<>(); - - @Override - public void processRow(ResultSet rs) throws SQLException { - Map row = new ColumnMapRowMapper().mapRow(rs, 0); - myRows.add(row); - - if (myRows.size() >= myBatchSize) { - submitNext(); - } - } - - private void submitNext() { - if (myRows.size() > 0) { - myFutures.add(updateRows(myRows)); - myRows = new ArrayList<>(); - } - } - - public List> getFutures() { - return myFutures; - } - - public void done() { - if (myRows.size() > 0) { - submitNext(); - } - } - } - - - public static class MandatoryKeyMap extends ForwardingMap { - - private final Map myWrap; - - public MandatoryKeyMap(Map theWrap) { - myWrap = theWrap; - } - - @Override - public V get(Object theKey) { - if (!containsKey(theKey)) { - throw new IllegalArgumentException("No key: " + theKey); - } - return super.get(theKey); - } - - public String getString(String theKey) { - return (String) get(theKey); - } - - @Override - protected Map delegate() { - return myWrap; - } - - public String getResourceType() { - return getString("RES_TYPE"); - } - - public String getParamName() { - return getString("SP_NAME"); + return shouldSkip; + } catch (SQLException e) { + logInfo(ourLog, "Error retrieving table names, skipping task"); + return true; } } } diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java new file mode 100644 index 00000000000..d0d16c1e8d4 --- /dev/null +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java @@ -0,0 +1,35 @@ +package ca.uhn.fhir.jpa.migrate.taskdef; + +/*- + * #%L + * HAPI FHIR JPA Server - Migration + * %% + * Copyright (C) 2014 - 2020 University Health Network + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + +import ca.uhn.fhir.util.VersionEnum; + +public class CalculateOrdinalDatesTask extends BaseColumnCalculatorTask { + + public CalculateOrdinalDatesTask(VersionEnum theRelease, String theVersion) { + super(theRelease, theVersion); + setDescription("Calculate SP_LOW_VALUE_DATE and SP_HIGH_VALUE_DATE based on existing SP_LOW and SP_HIGH date values in Date Search Params"); + } + @Override + protected boolean shouldSkipTask() { + return false; // TODO Is there a case where we should just not do this? + } +} diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index 59f5720002b..6ea71e54b73 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -21,19 +21,13 @@ package ca.uhn.fhir.jpa.migrate.tasks; */ import ca.uhn.fhir.jpa.migrate.DriverTypeEnum; -import ca.uhn.fhir.jpa.migrate.taskdef.AddColumnTask; -import ca.uhn.fhir.jpa.migrate.taskdef.ArbitrarySqlTask; -import ca.uhn.fhir.jpa.migrate.taskdef.BaseTableColumnTypeTask; -import ca.uhn.fhir.jpa.migrate.taskdef.CalculateHashesTask; +import ca.uhn.fhir.jpa.migrate.taskdef.*; import ca.uhn.fhir.jpa.migrate.tasks.api.BaseMigrationTasks; import ca.uhn.fhir.jpa.migrate.tasks.api.Builder; import ca.uhn.fhir.jpa.model.entity.*; import ca.uhn.fhir.util.VersionEnum; -import java.util.Arrays; -import java.util.List; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.stream.Collectors; @SuppressWarnings({"SqlNoDataSourceInspection", "SpellCheckingInspection"}) @@ -74,10 +68,10 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { Builder.BuilderWithTableName spidxDate = version.onTable("HFJ_SPIDX_DATE"); spidxDate.addColumn("20200225.1", "SP_VALUE_LOW_DATE_ORDINAL").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.INT); spidxDate.addColumn("20200225.2", "SP_VALUE_HIGH_DATE_ORDINAL").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.INT); - spidxDate.addTask(new CalculateHashesTask(VersionEnum.V4_3_0, "20200225.3") - .setColumnName("HASH_IDENTITY") - .addCalculator("SP_VALUE_LOW_DATE_ORDINAL", t -> BaseResourceIndexedSearchParam.calculateHashIdentity(t.getResourceType(), t.getString("SP_NAME"))) - .addCalculator("SP_VALUE_HIGH_DATE_ORDINAL", t -> BaseResourceIndexedSearchParam.calculateHashIdentity(t.getResourceType(), t.getString("SP_NAME"))) + spidxDate.addTask(new CalculateOrdinalDatesTask(VersionEnum.V4_3_0, "20200225.3") + .setColumnName("SP_VALUE_LOW_DATE_ORDINAL") //It doesn't matter which of the two we choose as they will both be null. + .addCalculator("SP_VALUE_LOW_DATE_ORDINAL", t -> ResourceIndexedSearchParamDate.calculateOrdinalValue(t.getDate("SP_VALUE_LOW"))) + .addCalculator("SP_VALUE_HIGH_DATE_ORDINAL", t -> ResourceIndexedSearchParamDate.calculateOrdinalValue(t.getDate("SP_VALUE_HIGH"))) ); // } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java index d4a19d9f022..c5ffe82375a 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java @@ -25,6 +25,7 @@ import ca.uhn.fhir.model.api.TemporalPrecisionEnum; import ca.uhn.fhir.model.primitive.InstantDt; import ca.uhn.fhir.rest.param.DateParam; import ca.uhn.fhir.rest.param.DateRangeParam; +import ca.uhn.fhir.util.DateUtils; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.apache.commons.lang3.builder.ToStringBuilder; @@ -57,8 +58,7 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar /** * Field which stores an integer representation of YYYYMDD as calculated by Calendar - * e.g. 2019-01-20 -> 2019020 - * (note that the month is 0 since calendar month counting starts at 0. + * e.g. 2019-01-20 -> 20190120 */ @Column(name="SP_VALUE_LOW_DATE_ORDINAL") public Integer myValueLowDateOrdinal; @@ -242,4 +242,8 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar return result; } + public static Long calculateOrdinalValue(Date theDate) { + return (long) DateUtils.convertDatetoDayInteger(theDate); + }; + } From f56e38148b6b48f18059dbd9c0b307be051b3174 Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Tue, 25 Feb 2020 16:43:17 -0500 Subject: [PATCH 10/27] Add flag to DaoConfig controlling behaviour --- .../java/ca/uhn/fhir/jpa/dao/DaoConfig.java | 43 +++++++++++++++++++ .../dao/predicate/PredicateBuilderDate.java | 6 ++- .../tasks/HapiFhirJpaMigrationTasks.java | 3 +- .../ResourceIndexedSearchParamDate.java | 8 ++++ 4 files changed, 57 insertions(+), 3 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java index 6168d8fe5e4..8d75b767a37 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java @@ -3,6 +3,7 @@ package ca.uhn.fhir.jpa.dao; import ca.uhn.fhir.jpa.interceptor.CascadingDeleteInterceptor; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.entity.ResourceEncodingEnum; +import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamDate; import ca.uhn.fhir.jpa.search.warm.WarmCacheEntry; import ca.uhn.fhir.jpa.searchparam.SearchParamConstants; import ca.uhn.fhir.rest.api.SearchTotalModeEnum; @@ -99,6 +100,10 @@ public class DaoConfig { */ private boolean myAllowInlineMatchUrlReferences = true; private boolean myAllowMultipleDelete; + /** + * Update setter javadoc if default changes. + */ + private boolean myUseOrdinalDatesForDayPrecisionSearches = true; /** * update setter javadoc if default changes */ @@ -1907,6 +1912,44 @@ public class DaoConfig { setPreExpandValueSetsDefaultCount(Math.min(getPreExpandValueSetsDefaultCount(), getPreExpandValueSetsMaxCount())); } + /** + *

+ * Should searches use the integer field {@code SP_VALUE_LOW_DATE_ORDINAL} and {@code SP_VALUE_HIGH_DATE_ORDINAL} in + * {@link ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamDate} when resolving searches where all predicates are using + * precision of {@link ca.uhn.fhir.model.api.TemporalPrecisionEnum#DAY}. + * + * For example, if enabled, the search of {@code Observation?date=2020-02-25} will cause the date to be collapsed down to an + * ordinal {@code 20200225}. It would then be compared against {@link ResourceIndexedSearchParamDate#getValueLowDateOrdinal()} + * and {@link ResourceIndexedSearchParamDate#getValueHighDateOrdinal()} + *

+ * Default is {@literal true} beginning in HAPI FHIR 4.3. + *

+ * + * @since 4.3 + */ + public void setUseOrdinalDatesForDayPrecisionSearches(boolean theUseOrdinalDates) { + myUseOrdinalDatesForDayPrecisionSearches = theUseOrdinalDates; + } + + /** + *

+ * Should searches use the integer field {@code SP_VALUE_LOW_DATE_ORDINAL} and {@code SP_VALUE_HIGH_DATE_ORDINAL} in + * {@link ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamDate} when resolving searches where all predicates are using + * precision of {@link ca.uhn.fhir.model.api.TemporalPrecisionEnum#DAY}. + * + * For example, if enabled, the search of {@code Observation?date=2020-02-25} will cause the date to be collapsed down to an + * integer representing the ordinal date {@code 20200225}. It would then be compared against {@link ResourceIndexedSearchParamDate#getValueLowDateOrdinal()} + * and {@link ResourceIndexedSearchParamDate#getValueHighDateOrdinal()} + *

+ * Default is {@literal true} beginning in HAPI FHIR 4.3. + *

+ * + * @since 4.3 + */ + public boolean getUseOrdinalDatesForDayPrecisionSearches() { + return myUseOrdinalDatesForDayPrecisionSearches; + } + public enum StoreMetaSourceInformationEnum { NONE(false, false), SOURCE_URI(true, false), diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java index eebf0f2987f..fc182a9162b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java @@ -140,7 +140,9 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi DateParam lowerBound = theRange.getLowerBound(); DateParam upperBound = theRange.getUpperBound(); - boolean isOrdinalComparison = isNullOrDayPrecision(lowerBound) && isNullOrDayPrecision(upperBound); + Integer lowerBoundAsOrdinal = theRange.getLowerBoundAsDateInteger(); + Integer upperBoundAsOrdinal = theRange.getUpperBoundAsDateInteger(); + boolean isOrdinalComparison = isNullOrDayPrecision(lowerBound) && isNullOrDayPrecision(upperBound) && myDaoConfig.getUseOrdinalDatesForDayPrecisionSearches(); Predicate lt = null; Predicate gt = null; Predicate lb = null; @@ -152,7 +154,7 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi } //im like 80% sure this should be ub and not lb, as it is an UPPER bound. if (isOrdinalComparison) { - lb = theBuilder.lessThan(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateInteger()); + lb = theBuilder.lessThan(theFrom.get("myValueLowDateOrdinal"), lowerBoundAsOrdinal); } else { lb = theBuilder.lessThan(theFrom.get("myValueLow"), lowerBoundInstant); } diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index 6ea71e54b73..98778c257f2 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -64,10 +64,11 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { version.onTable("HFJ_RES_VER").modifyColumn("20200220.1", "RES_ID").nonNullable().failureAllowed().withType(BaseTableColumnTypeTask.ColumnTypeEnum.LONG); // - // Add support for integer comparisons during day-granularity date search. + // Add support for integer comparisons during day-precision date search. Builder.BuilderWithTableName spidxDate = version.onTable("HFJ_SPIDX_DATE"); spidxDate.addColumn("20200225.1", "SP_VALUE_LOW_DATE_ORDINAL").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.INT); spidxDate.addColumn("20200225.2", "SP_VALUE_HIGH_DATE_ORDINAL").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.INT); + spidxDate.addTask(new CalculateOrdinalDatesTask(VersionEnum.V4_3_0, "20200225.3") .setColumnName("SP_VALUE_LOW_DATE_ORDINAL") //It doesn't matter which of the two we choose as they will both be null. .addCalculator("SP_VALUE_LOW_DATE_ORDINAL", t -> ResourceIndexedSearchParamDate.calculateOrdinalValue(t.getDate("SP_VALUE_LOW"))) diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java index c5ffe82375a..17d011d4e0c 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java @@ -111,6 +111,14 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar this.myValueLowDateOrdinal = generateOrdinalDateInteger(theLow); } + public Integer getValueLowDateOrdinal() { + return myValueLowDateOrdinal; + } + + public Integer getValueHighDateOrdinal() { + return myValueHighDateOrdinal; + } + @Override @PrePersist public void calculateHashes() { From a1b6e37395746b006594ef144e2b33fab754feaf Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Tue, 25 Feb 2020 17:03:50 -0500 Subject: [PATCH 11/27] Fix bug with Period bounds not counting as first value --- .../jpa/model/entity/ResourceIndexedSearchParamDate.java | 8 +++++--- .../searchparam/extractor/BaseSearchParamExtractor.java | 4 ++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java index 17d011d4e0c..d38f4dfe440 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java @@ -102,9 +102,11 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar this.myValueHighDateOrdinal = generateOrdinalDateInteger(theHigh); } private int generateOrdinalDateInteger(String theDateString){ - String t = theDateString.substring(0, theDateString.indexOf("T")); - t = t.replace("-", ""); - return Integer.valueOf(t); + if (theDateString.contains("T")) { + theDateString = theDateString.substring(0, theDateString.indexOf("T")); + } + theDateString = theDateString.replace("-", ""); + return Integer.valueOf(theDateString); } private void computeValueLowDateOrdinal(String theLow) { diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java index d3b42491c51..eb44a8a0a7a 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java @@ -642,6 +642,10 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor String endString = extractValueAsString(myPeriodEndValueChild, bounds.get()); dates.add(start); dates.add(end); + //TODO Check if this logic is valid. Does the start of the first period indicate a lower bound?? + if (firstValue == null) { + firstValue = extractValueAsString(myPeriodStartValueChild, bounds.get()); + } finalValue = endString; } } From 1e833af5f046c464414821011f66921ceb5460cf Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Tue, 25 Feb 2020 17:06:38 -0500 Subject: [PATCH 12/27] Variable reuse --- .../dao/predicate/PredicateBuilderDate.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java index fc182a9162b..ae0e3df79ec 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java @@ -164,7 +164,7 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi } //im like 80% sure this should be ub and not lb, as it is an UPPER bound. if (isOrdinalComparison) { - lb = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateInteger()); + lb = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), upperBoundAsOrdinal); } else { lb = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); } @@ -173,7 +173,7 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi throw new InvalidRequestException("upperBound value not correctly specified for compare operation"); } if (isOrdinalComparison) { - lb = theBuilder.greaterThan(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateInteger()); + lb = theBuilder.greaterThan(theFrom.get("myValueHighDateOrdinal"), upperBoundAsOrdinal); } else { lb = theBuilder.greaterThan(theFrom.get("myValueHigh"), upperBoundInstant); } @@ -182,7 +182,7 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi throw new InvalidRequestException("lowerBound value not correctly specified for compare operation"); } if (isOrdinalComparison) { - lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateInteger()); + lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), lowerBoundAsOrdinal); } else { lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLow"), lowerBoundInstant); } @@ -192,8 +192,8 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi throw new InvalidRequestException("lowerBound and/or upperBound value not correctly specified for compare operation"); } if (isOrdinalComparison){ - lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateInteger()); - gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateInteger()); + lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), lowerBoundAsOrdinal); + gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), upperBoundAsOrdinal); } else { lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLow"), lowerBoundInstant); gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); @@ -203,8 +203,8 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi } else if ((operation == SearchFilterParser.CompareOperation.eq) || (operation == null)) { if (lowerBoundInstant != null) { if (isOrdinalComparison) { - gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getLowerBoundAsDateInteger()); - lt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getLowerBoundAsDateInteger()); + gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), lowerBoundAsOrdinal); + lt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), lowerBoundAsOrdinal); //also try a strict equality here. } else { @@ -220,8 +220,8 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi if (upperBoundInstant != null) { if (isOrdinalComparison) { - gt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), theRange.getUpperBoundAsDateInteger()); - lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), theRange.getUpperBoundAsDateInteger()); + gt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), upperBoundAsOrdinal); + lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), upperBoundAsOrdinal); } else { gt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLow"), upperBoundInstant); lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); @@ -238,7 +238,7 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi operation.name())); } if (isOrdinalComparison) { - ourLog.trace("Ordinal date range is {} - {} ", theRange.getLowerBoundAsDateInteger(), theRange.getUpperBoundAsDateInteger()); + ourLog.trace("Ordinal date range is {} - {} ", lowerBoundAsOrdinal, upperBoundAsOrdinal); } ourLog.trace("Date range is {} - {}", lowerBoundInstant, upperBoundInstant); From 2204fbeabd84a75ffd9171daa6c3a7251bd3ee11 Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Tue, 25 Feb 2020 17:08:11 -0500 Subject: [PATCH 13/27] Adding some comments --- .../uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java index ae0e3df79ec..a2e0aad60cc 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java @@ -20,6 +20,7 @@ package ca.uhn.fhir.jpa.dao.predicate; * #L% */ +import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.dao.SearchBuilder; import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamDate; import ca.uhn.fhir.jpa.model.entity.ResourceTable; @@ -142,7 +143,13 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi DateParam upperBound = theRange.getUpperBound(); Integer lowerBoundAsOrdinal = theRange.getLowerBoundAsDateInteger(); Integer upperBoundAsOrdinal = theRange.getUpperBoundAsDateInteger(); + + /** + * If all present search parameters are of DAY precision, and {@link DaoConfig#getUseOrdinalDatesForDayPrecisionSearches()} is true, + * then we attempt to use the ordinal field for date comparisons instead of the date field. + */ boolean isOrdinalComparison = isNullOrDayPrecision(lowerBound) && isNullOrDayPrecision(upperBound) && myDaoConfig.getUseOrdinalDatesForDayPrecisionSearches(); + Predicate lt = null; Predicate gt = null; Predicate lb = null; From 165f5fd808a4d6cf5907bf5d8d915629e7e11224 Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Wed, 26 Feb 2020 09:32:20 -0500 Subject: [PATCH 14/27] fix bugs in calculating ordinal date --- .../main/java/ca/uhn/fhir/util/DateUtils.java | 8 ++++-- .../ResourceIndexedSearchParamDate.java | 16 +++++++++-- .../ResourceIndexedSearchParamDateTest.java | 27 ++++++++++--------- 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java index f34a0ee12e5..d208e3fe599 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java @@ -171,15 +171,19 @@ public final class DateUtils { return cal.getTime(); } - public static int convertDatetoDayInteger(final Date theDateValue) { notNull(theDateValue, "Date value"); - Calendar cal = org.apache.commons.lang3.time.DateUtils.toCalendar(theDateValue); SimpleDateFormat format = new SimpleDateFormat(PATTERN_INTEGER_DATE); String theDateString = format.format(theDateValue); return Integer.parseInt(theDateString); } + public static String convertDateToIso8601String(final Date theDateValue){ + notNull(theDateValue, "Date value"); + SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSXXX"); + return format.format(theDateValue); + } + /** * Formats the given date according to the RFC 1123 pattern. * diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java index d38f4dfe440..6a16c59b61e 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java @@ -26,6 +26,7 @@ import ca.uhn.fhir.model.primitive.InstantDt; import ca.uhn.fhir.rest.param.DateParam; import ca.uhn.fhir.rest.param.DateRangeParam; import ca.uhn.fhir.util.DateUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.apache.commons.lang3.builder.ToStringBuilder; @@ -34,6 +35,7 @@ import org.hibernate.search.annotations.Field; import org.hl7.fhir.r4.model.DateTimeType; import javax.persistence.*; +import java.text.SimpleDateFormat; import java.util.Date; @Embeddable @@ -93,13 +95,21 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar setParamName(theParamName); setValueLow(theLow); setValueHigh(theHigh); + if (theHigh != null && theHighString == null) { + theHighString = DateUtils.convertDateToIso8601String(theHigh); + } + if (theLow != null && theLowString == null) { + theLowString = DateUtils.convertDateToIso8601String(theLow); + } computeValueHighDateOrdinal(theHighString); computeValueLowDateOrdinal(theLowString); myOriginalValue = theOriginalValue; } private void computeValueHighDateOrdinal(String theHigh) { - this.myValueHighDateOrdinal = generateOrdinalDateInteger(theHigh); + if (!StringUtils.isBlank(theHigh)) { + this.myValueHighDateOrdinal = generateOrdinalDateInteger(theHigh); + } } private int generateOrdinalDateInteger(String theDateString){ if (theDateString.contains("T")) { @@ -110,7 +120,9 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar } private void computeValueLowDateOrdinal(String theLow) { - this.myValueLowDateOrdinal = generateOrdinalDateInteger(theLow); + if (StringUtils.isNotBlank(theLow)) { + this.myValueLowDateOrdinal = generateOrdinalDateInteger(theLow); + } } public Integer getValueLowDateOrdinal() { diff --git a/hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDateTest.java b/hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDateTest.java index 407d05163f5..4ba8d5517e5 100644 --- a/hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDateTest.java +++ b/hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDateTest.java @@ -4,6 +4,7 @@ import org.junit.Before; import org.junit.Test; import java.sql.Timestamp; +import java.time.Instant; import java.util.Calendar; import java.util.Date; @@ -45,8 +46,8 @@ public class ResourceIndexedSearchParamDateTest { @Test public void equalsIsTrueForMatchingDates() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date1A.toString(), date2A, date2A.toString(), "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1B, date1B.toString(), date2B, date2B.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, null, date2A, null, "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1B, null, date2B, null, "SomeValue"); assertTrue(param.equals(param2)); assertTrue(param2.equals(param)); @@ -55,8 +56,8 @@ public class ResourceIndexedSearchParamDateTest { @Test public void equalsIsTrueForMatchingTimeStampsThatMatch() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1A, timestamp1A.toString(), timestamp2A, timestamp2A.toString(), "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1B, timestamp1B.toString(), timestamp2B, timestamp2B.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1A, null, timestamp2A, null, "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1B, null, timestamp2B, null, "SomeValue"); assertTrue(param.equals(param2)); assertTrue(param2.equals(param)); @@ -67,8 +68,8 @@ public class ResourceIndexedSearchParamDateTest { // other will be equivalent but will be a java.sql.Timestamp. Equals should work in both directions. @Test public void equalsIsTrueForMixedTimestampsAndDates() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date1A.toString(), date2A, date2A.toString(), "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1A, timestamp1A.toString(), timestamp2A, timestamp2A.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, null, date2A, null, "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1A, null, timestamp2A, null, "SomeValue"); assertTrue(param.equals(param2)); assertTrue(param2.equals(param)); @@ -77,8 +78,8 @@ public class ResourceIndexedSearchParamDateTest { @Test public void equalsIsFalseForNonMatchingDates() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date1A.toString(), date2A, date2A.toString(), "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date2A, date2A.toString(), date1A, date1A.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, null, date2A, null, "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date2A, null, date1A, null, "SomeValue"); assertFalse(param.equals(param2)); assertFalse(param2.equals(param)); @@ -87,7 +88,7 @@ public class ResourceIndexedSearchParamDateTest { @Test public void equalsIsFalseForNonMatchingDatesNullCase() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date1A.toString(), date2A, date2A.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, null, date2A, null, "SomeValue"); ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", null, null, null, null, "SomeValue"); assertFalse(param.equals(param2)); @@ -97,8 +98,8 @@ public class ResourceIndexedSearchParamDateTest { @Test public void equalsIsFalseForNonMatchingTimeStamps() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1A, timestamp1A.toString(), timestamp2A, timestamp2A.toString(), "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp2A, timestamp2A.toString(), timestamp1A, timestamp1A.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp1A, null, timestamp2A, null, "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp2A, null, timestamp1A, null, "SomeValue"); assertFalse(param.equals(param2)); assertFalse(param2.equals(param)); @@ -107,8 +108,8 @@ public class ResourceIndexedSearchParamDateTest { @Test public void equalsIsFalseForMixedTimestampsAndDatesThatDoNotMatch() { - ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, date1A.toString(), date2A, date2A.toString(), "SomeValue"); - ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp2A, timestamp2A.toString(), timestamp1A, timestamp1A.toString(), "SomeValue"); + ResourceIndexedSearchParamDate param = new ResourceIndexedSearchParamDate("Patient", "SomeResource", date1A, null, date2A, null, "SomeValue"); + ResourceIndexedSearchParamDate param2 = new ResourceIndexedSearchParamDate("Patient", "SomeResource", timestamp2A, null, timestamp1A, null, "SomeValue"); assertFalse(param.equals(param2)); assertFalse(param2.equals(param)); From a97443b1ed075845b501a7f42354a1a941ec1e35 Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Wed, 26 Feb 2020 11:12:27 -0500 Subject: [PATCH 15/27] Fix test failures. One caused by invalid test. One caused by granularity need LT not LTE --- .../ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java | 4 ++-- .../fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java index 586e9692c95..dade7a7fbbe 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java @@ -222,8 +222,8 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi throw new InvalidRequestException("lowerBound and/or upperBound value not correctly specified for compare operation"); } if (isOrdinalComparison){ - lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), lowerBoundAsOrdinal); - gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), upperBoundAsOrdinal); + lt = theBuilder.lessThan(theFrom.get("myValueLowDateOrdinal"), lowerBoundAsOrdinal); + gt = theBuilder.greaterThan(theFrom.get("myValueHighDateOrdinal"), upperBoundAsOrdinal); } else { lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLow"), lowerBoundInstant); gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java index 9d1bbe51f52..143459c3f9b 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java @@ -3282,7 +3282,6 @@ public class FhirResourceDaoR4SearchNoHashesTest extends BaseJpaR4Test { public void testDateSearchParametersShouldBeTimezoneIndependent() { createObservationWithEffective("NO1", "2011-01-02T23:00:00-11:30"); - createObservationWithEffective("NO2", "2011-01-03T00:00:00+01:00"); createObservationWithEffective("YES01", "2011-01-02T00:00:00-11:30"); createObservationWithEffective("YES02", "2011-01-02T00:00:00-10:00"); From 077f9df814640d37be47af9acfa5b9afbd846ccc Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Wed, 26 Feb 2020 12:04:33 -0500 Subject: [PATCH 16/27] I keep finding more tests --- .../fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java | 3 ++- .../module/matcher/InMemorySubscriptionMatcherR4Test.java | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java index 143459c3f9b..3e0d4d144d2 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java @@ -3281,7 +3281,8 @@ public class FhirResourceDaoR4SearchNoHashesTest extends BaseJpaR4Test { @Test public void testDateSearchParametersShouldBeTimezoneIndependent() { - createObservationWithEffective("NO1", "2011-01-02T23:00:00-11:30"); + createObservationWithEffective("NO1", "2011-01-01T23:00:00-11:30"); + createObservationWithEffective("NO2", "2011-01-03T23:00:00+01:30"); createObservationWithEffective("YES01", "2011-01-02T00:00:00-11:30"); createObservationWithEffective("YES02", "2011-01-02T00:00:00-10:00"); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java index ee88580d9d6..77e2044500e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java @@ -884,10 +884,10 @@ public class InMemorySubscriptionMatcherR4Test { public void testDateSearchParametersShouldBeTimezoneIndependent() { List nlist = new ArrayList<>(); - nlist.add(createObservationWithEffective("NO1", "2011-01-02T23:00:00-11:30")); - nlist.add(createObservationWithEffective("NO2", "2011-01-03T00:00:00+01:00")); + nlist.add(createObservationWithEffective("NO1", "2011-01-03T00:00:00+01:00")); List ylist = new ArrayList<>(); + ylist.add(createObservationWithEffective("YES00", "2011-01-02T23:00:00-11:30")); ylist.add(createObservationWithEffective("YES01", "2011-01-02T00:00:00-11:30")); ylist.add(createObservationWithEffective("YES02", "2011-01-02T00:00:00-10:00")); ylist.add(createObservationWithEffective("YES03", "2011-01-02T00:00:00-09:00")); From 2dc94de6bb13962c715e626f0b95c8ac41805b59 Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Wed, 26 Feb 2020 14:31:35 -0500 Subject: [PATCH 17/27] roll back change to inmemory matcher test --- .../module/matcher/InMemorySubscriptionMatcherR4Test.java | 4 ++-- .../jpa/model/entity/ResourceIndexedSearchParamDate.java | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java index 77e2044500e..ee88580d9d6 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java @@ -884,10 +884,10 @@ public class InMemorySubscriptionMatcherR4Test { public void testDateSearchParametersShouldBeTimezoneIndependent() { List nlist = new ArrayList<>(); - nlist.add(createObservationWithEffective("NO1", "2011-01-03T00:00:00+01:00")); + nlist.add(createObservationWithEffective("NO1", "2011-01-02T23:00:00-11:30")); + nlist.add(createObservationWithEffective("NO2", "2011-01-03T00:00:00+01:00")); List ylist = new ArrayList<>(); - ylist.add(createObservationWithEffective("YES00", "2011-01-02T23:00:00-11:30")); ylist.add(createObservationWithEffective("YES01", "2011-01-02T00:00:00-11:30")); ylist.add(createObservationWithEffective("YES02", "2011-01-02T00:00:00-10:00")); ylist.add(createObservationWithEffective("YES03", "2011-01-02T00:00:00-09:00")); diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java index 6a16c59b61e..abb132d093d 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java @@ -242,8 +242,9 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar if (!(theParam instanceof DateParam)) { return false; } - DateParam date = (DateParam) theParam; - DateRangeParam range = new DateRangeParam(date); + DateParam dateParam = (DateParam) theParam; + DateRangeParam range = new DateRangeParam(dateParam); + Date lowerBound = range.getLowerBoundAsInstant(); Date upperBound = range.getUpperBoundAsInstant(); From f86a4c9fa1cad9b1c3e2dd39ac400812ef3568f6 Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Wed, 26 Feb 2020 17:05:23 -0500 Subject: [PATCH 18/27] Add date ordinals to subscription matching checker --- .../java/ca/uhn/fhir/jpa/dao/DaoConfig.java | 40 ----------------- .../dao/predicate/BasePredicateBuilder.java | 1 + .../dao/predicate/PredicateBuilderDate.java | 2 +- .../InMemorySubscriptionMatcherR4Test.java | 2 +- .../BaseResourceIndexedSearchParam.java | 2 +- .../fhir/jpa/model/entity/ModelConfig.java | 45 +++++++++++++++++-- .../ResourceIndexedSearchParamDate.java | 40 ++++++++++++++--- .../ResourceIndexedSearchParamNumber.java | 2 +- .../ResourceIndexedSearchParamQuantity.java | 2 +- .../ResourceIndexedSearchParamString.java | 2 +- .../ResourceIndexedSearchParamToken.java | 2 +- .../entity/ResourceIndexedSearchParamUri.java | 2 +- .../ResourceIndexedSearchParams.java | 4 +- .../matcher/InMemoryResourceMatcher.java | 6 +-- 14 files changed, 91 insertions(+), 61 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java index 8d75b767a37..6d6262c1f1c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java @@ -100,10 +100,6 @@ public class DaoConfig { */ private boolean myAllowInlineMatchUrlReferences = true; private boolean myAllowMultipleDelete; - /** - * Update setter javadoc if default changes. - */ - private boolean myUseOrdinalDatesForDayPrecisionSearches = true; /** * update setter javadoc if default changes */ @@ -1912,43 +1908,7 @@ public class DaoConfig { setPreExpandValueSetsDefaultCount(Math.min(getPreExpandValueSetsDefaultCount(), getPreExpandValueSetsMaxCount())); } - /** - *

- * Should searches use the integer field {@code SP_VALUE_LOW_DATE_ORDINAL} and {@code SP_VALUE_HIGH_DATE_ORDINAL} in - * {@link ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamDate} when resolving searches where all predicates are using - * precision of {@link ca.uhn.fhir.model.api.TemporalPrecisionEnum#DAY}. - * - * For example, if enabled, the search of {@code Observation?date=2020-02-25} will cause the date to be collapsed down to an - * ordinal {@code 20200225}. It would then be compared against {@link ResourceIndexedSearchParamDate#getValueLowDateOrdinal()} - * and {@link ResourceIndexedSearchParamDate#getValueHighDateOrdinal()} - *

- * Default is {@literal true} beginning in HAPI FHIR 4.3. - *

- * - * @since 4.3 - */ - public void setUseOrdinalDatesForDayPrecisionSearches(boolean theUseOrdinalDates) { - myUseOrdinalDatesForDayPrecisionSearches = theUseOrdinalDates; - } - /** - *

- * Should searches use the integer field {@code SP_VALUE_LOW_DATE_ORDINAL} and {@code SP_VALUE_HIGH_DATE_ORDINAL} in - * {@link ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamDate} when resolving searches where all predicates are using - * precision of {@link ca.uhn.fhir.model.api.TemporalPrecisionEnum#DAY}. - * - * For example, if enabled, the search of {@code Observation?date=2020-02-25} will cause the date to be collapsed down to an - * integer representing the ordinal date {@code 20200225}. It would then be compared against {@link ResourceIndexedSearchParamDate#getValueLowDateOrdinal()} - * and {@link ResourceIndexedSearchParamDate#getValueHighDateOrdinal()} - *

- * Default is {@literal true} beginning in HAPI FHIR 4.3. - *

- * - * @since 4.3 - */ - public boolean getUseOrdinalDatesForDayPrecisionSearches() { - return myUseOrdinalDatesForDayPrecisionSearches; - } public enum StoreMetaSourceInformationEnum { NONE(false, false), diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/BasePredicateBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/BasePredicateBuilder.java index f5c6c0480ba..6c613e684bb 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/BasePredicateBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/BasePredicateBuilder.java @@ -50,6 +50,7 @@ abstract class BasePredicateBuilder { @Autowired DaoConfig myDaoConfig; + boolean myDontUseHashesForSearch; final IDao myCallingDao; final CriteriaBuilder myBuilder; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java index dade7a7fbbe..d8bfb3a7876 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java @@ -171,7 +171,7 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi * If all present search parameters are of DAY precision, and {@link DaoConfig#getUseOrdinalDatesForDayPrecisionSearches()} is true, * then we attempt to use the ordinal field for date comparisons instead of the date field. */ - boolean isOrdinalComparison = isNullOrDayPrecision(lowerBound) && isNullOrDayPrecision(upperBound) && myDaoConfig.getUseOrdinalDatesForDayPrecisionSearches(); + boolean isOrdinalComparison = isNullOrDayPrecision(lowerBound) && isNullOrDayPrecision(upperBound) && myDaoConfig.getModelConfig().getUseOrdinalDatesForDayPrecisionSearches(); Predicate lt = null; Predicate gt = null; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java index ee88580d9d6..bb98b5de8d9 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherR4Test.java @@ -884,10 +884,10 @@ public class InMemorySubscriptionMatcherR4Test { public void testDateSearchParametersShouldBeTimezoneIndependent() { List nlist = new ArrayList<>(); - nlist.add(createObservationWithEffective("NO1", "2011-01-02T23:00:00-11:30")); nlist.add(createObservationWithEffective("NO2", "2011-01-03T00:00:00+01:00")); List ylist = new ArrayList<>(); + nlist.add(createObservationWithEffective("YES00", "2011-01-02T23:00:00-11:30")); ylist.add(createObservationWithEffective("YES01", "2011-01-02T00:00:00-11:30")); ylist.add(createObservationWithEffective("YES02", "2011-01-02T00:00:00-10:00")); ylist.add(createObservationWithEffective("YES03", "2011-01-02T00:00:00-09:00")); diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/BaseResourceIndexedSearchParam.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/BaseResourceIndexedSearchParam.java index b891a3889b2..4393d0a5a29 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/BaseResourceIndexedSearchParam.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/BaseResourceIndexedSearchParam.java @@ -133,7 +133,7 @@ public abstract class BaseResourceIndexedSearchParam extends BaseResourceIndex { public abstract IQueryParameterType toQueryParameterType(); - public boolean matches(IQueryParameterType theParam) { + public boolean matches(IQueryParameterType theParam, boolean theUseOrdinalDatesForDayComparison) { throw new UnsupportedOperationException("No parameter matcher for " + theParam); } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ModelConfig.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ModelConfig.java index 171c6d31866..1938f069ef6 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ModelConfig.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ModelConfig.java @@ -60,6 +60,10 @@ public class ModelConfig { private String myEmailFromAddress = "noreply@unknown.com"; private boolean mySubscriptionMatchingEnabled = true; private String myWebsocketContextPath = DEFAULT_WEBSOCKET_CONTEXT_PATH; + /** + * Update setter javadoc if default changes. + */ + private boolean myUseOrdinalDatesForDayPrecisionSearches = true; /** * Constructor @@ -257,7 +261,6 @@ public class ModelConfig { myTreatReferencesAsLogical = new HashSet<>(); } myTreatReferencesAsLogical.add(theTreatReferencesAsLogical); - } /** @@ -340,13 +343,12 @@ public class ModelConfig { return mySubscriptionMatchingEnabled; } + /** * If set to true (default is true) the server will match incoming resources against active subscriptions * and send them to the subscription channel. If set to false no matching or sending occurs. * @since 3.7.0 */ - - public void setSubscriptionMatchingEnabled(boolean theSubscriptionMatchingEnabled) { mySubscriptionMatchingEnabled = theSubscriptionMatchingEnabled; } @@ -388,6 +390,43 @@ public class ModelConfig { myWebsocketContextPath = theWebsocketContextPath; } + /** + *

+ * Should searches use the integer field {@code SP_VALUE_LOW_DATE_ORDINAL} and {@code SP_VALUE_HIGH_DATE_ORDINAL} in + * {@link ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamDate} when resolving searches where all predicates are using + * precision of {@link ca.uhn.fhir.model.api.TemporalPrecisionEnum#DAY}. + * + * For example, if enabled, the search of {@code Observation?date=2020-02-25} will cause the date to be collapsed down to an + * ordinal {@code 20200225}. It would then be compared against {@link ResourceIndexedSearchParamDate#getValueLowDateOrdinal()} + * and {@link ResourceIndexedSearchParamDate#getValueHighDateOrdinal()} + *

+ * Default is {@literal true} beginning in HAPI FHIR 4.3. + *

+ * + * @since 4.3 + */ + public void setUseOrdinalDatesForDayPrecisionSearches(boolean theUseOrdinalDates) { + myUseOrdinalDatesForDayPrecisionSearches = theUseOrdinalDates; + } + + /** + *

+ * Should searches use the integer field {@code SP_VALUE_LOW_DATE_ORDINAL} and {@code SP_VALUE_HIGH_DATE_ORDINAL} in + * {@link ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamDate} when resolving searches where all predicates are using + * precision of {@link ca.uhn.fhir.model.api.TemporalPrecisionEnum#DAY}. + * + * For example, if enabled, the search of {@code Observation?date=2020-02-25} will cause the date to be collapsed down to an + * integer representing the ordinal date {@code 20200225}. It would then be compared against {@link ResourceIndexedSearchParamDate#getValueLowDateOrdinal()} + * and {@link ResourceIndexedSearchParamDate#getValueHighDateOrdinal()} + *

+ * Default is {@literal true} beginning in HAPI FHIR 4.3. + *

+ * + * @since 4.3 + */ + public boolean getUseOrdinalDatesForDayPrecisionSearches() { + return myUseOrdinalDatesForDayPrecisionSearches; + } private static void validateTreatBaseUrlsAsLocal(String theUrl) { Validate.notBlank(theUrl, "Base URL must not be null or empty"); diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java index abb132d093d..9b2b4c93129 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java @@ -35,7 +35,6 @@ import org.hibernate.search.annotations.Field; import org.hl7.fhir.r4.model.DateTimeType; import javax.persistence.*; -import java.text.SimpleDateFormat; import java.util.Date; @Embeddable @@ -238,21 +237,33 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar } @Override - public boolean matches(IQueryParameterType theParam) { + public boolean matches(IQueryParameterType theParam, boolean theUseOrdinalDatesForDayComparison) { if (!(theParam instanceof DateParam)) { return false; } DateParam dateParam = (DateParam) theParam; DateRangeParam range = new DateRangeParam(dateParam); + + + + boolean result; + if (theUseOrdinalDatesForDayComparison) { + result = matchesOrdinalDateBounds(range); + } else { + result = matchesDateBounds(range); + } + + return result; + } + + private boolean matchesDateBounds(DateRangeParam range) { Date lowerBound = range.getLowerBoundAsInstant(); Date upperBound = range.getUpperBoundAsInstant(); - if (lowerBound == null && upperBound == null) { // should never happen return false; } - boolean result = true; if (lowerBound != null) { result &= (myValueLow.after(lowerBound) || myValueLow.equals(lowerBound)); @@ -265,8 +276,27 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar return result; } + private boolean matchesOrdinalDateBounds(DateRangeParam range) { + boolean result = true; + Integer lowerBoundAsDateInteger = range.getLowerBoundAsDateInteger(); + Integer upperBoundAsDateInteger = range.getUpperBoundAsDateInteger(); + if (upperBoundAsDateInteger == null && lowerBoundAsDateInteger == null) { + return false; + } + if (lowerBoundAsDateInteger != null) { + result &= (myValueLowDateOrdinal.equals(lowerBoundAsDateInteger) || myValueLowDateOrdinal > lowerBoundAsDateInteger); + result &= (myValueHighDateOrdinal.equals(lowerBoundAsDateInteger) || myValueHighDateOrdinal > lowerBoundAsDateInteger); + } + if (upperBoundAsDateInteger != null) { + result &= (myValueHighDateOrdinal.equals(upperBoundAsDateInteger) || myValueHighDateOrdinal < upperBoundAsDateInteger); + result &= (myValueLowDateOrdinal.equals(upperBoundAsDateInteger) || myValueLowDateOrdinal < upperBoundAsDateInteger); + } + return result; + } + + public static Long calculateOrdinalValue(Date theDate) { return (long) DateUtils.convertDatetoDayInteger(theDate); - }; + } } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamNumber.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamNumber.java index f88e357ccd7..bb686e6cc5f 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamNumber.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamNumber.java @@ -158,7 +158,7 @@ public class ResourceIndexedSearchParamNumber extends BaseResourceIndexedSearchP } @Override - public boolean matches(IQueryParameterType theParam) { + public boolean matches(IQueryParameterType theParam, boolean theUseOrdinalDatesForDayComparison) { if (!(theParam instanceof NumberParam)) { return false; } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamQuantity.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamQuantity.java index 6107df7884d..8b9787a63db 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamQuantity.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamQuantity.java @@ -237,7 +237,7 @@ public class ResourceIndexedSearchParamQuantity extends BaseResourceIndexedSearc } @Override - public boolean matches(IQueryParameterType theParam) { + public boolean matches(IQueryParameterType theParam, boolean theUseOrdinalDatesForDayComparison) { if (!(theParam instanceof QuantityParam)) { return false; } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamString.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamString.java index 155bf7a8da3..714c2cd6f98 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamString.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamString.java @@ -314,7 +314,7 @@ public class ResourceIndexedSearchParamString extends BaseResourceIndexedSearchP } @Override - public boolean matches(IQueryParameterType theParam) { + public boolean matches(IQueryParameterType theParam, boolean theUseOrdinalDatesForDayComparison) { if (!(theParam instanceof StringParam)) { return false; } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamToken.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamToken.java index 44a14bf41ee..f18c2d53157 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamToken.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamToken.java @@ -239,7 +239,7 @@ public class ResourceIndexedSearchParamToken extends BaseResourceIndexedSearchPa } @Override - public boolean matches(IQueryParameterType theParam) { + public boolean matches(IQueryParameterType theParam, boolean theUseOrdinalDatesForDayComparison) { if (!(theParam instanceof TokenParam)) { return false; } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamUri.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamUri.java index c827890d54b..6247dc668d1 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamUri.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamUri.java @@ -188,7 +188,7 @@ public class ResourceIndexedSearchParamUri extends BaseResourceIndexedSearchPara } @Override - public boolean matches(IQueryParameterType theParam) { + public boolean matches(IQueryParameterType theParam, boolean theUseOrdinalDatesForDayComparison) { if (!(theParam instanceof UriParam)) { return false; } diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParams.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParams.java index 73f0df466c1..9dff7395b62 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParams.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParams.java @@ -217,7 +217,7 @@ public final class ResourceIndexedSearchParams { return myPopulatedResourceLinkParameters; } - public boolean matchParam(String theResourceName, String theParamName, RuntimeSearchParam theParamDef, IQueryParameterType theParam) { + public boolean matchParam(String theResourceName, String theParamName, RuntimeSearchParam theParamDef, IQueryParameterType theParam, boolean theUseOrdinalDatesForDayComparison) { if (theParamDef == null) { return false; } @@ -254,7 +254,7 @@ public final class ResourceIndexedSearchParams { } Predicate namedParamPredicate = param -> param.getParamName().equalsIgnoreCase(theParamName) && - param.matches(theParam); + param.matches(theParam, theUseOrdinalDatesForDayComparison); return resourceParams.stream().anyMatch(namedParamPredicate); } diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java index d83eb5048c9..2b52d347e91 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java @@ -202,7 +202,7 @@ public class InMemoryResourceMatcher { if (theSearchParams == null) { return InMemoryMatchResult.successfulMatch(); } else { - return InMemoryMatchResult.fromBoolean(theAndOrParams.stream().anyMatch(nextAnd -> matchParams(theResourceName, theParamName, theParamDef, nextAnd, theSearchParams))); + return InMemoryMatchResult.fromBoolean(theAndOrParams.stream().anyMatch(nextAnd -> matchParams(theResourceName, theParamName, theParamDef, nextAnd, theSearchParams, myModelConfig.getUseOrdinalDatesForDayPrecisionSearches()))); } case COMPOSITE: case HAS: @@ -219,11 +219,11 @@ public class InMemoryResourceMatcher { } } - private boolean matchParams(String theResourceName, String theParamName, RuntimeSearchParam paramDef, List theNextAnd, ResourceIndexedSearchParams theSearchParams) { + private boolean matchParams(String theResourceName, String theParamName, RuntimeSearchParam paramDef, List theNextAnd, ResourceIndexedSearchParams theSearchParams, boolean theUseOrdinalDatesForDayComparison) { if (paramDef.getParamType() == RestSearchParameterTypeEnum.REFERENCE) { stripBaseUrlsFromReferenceParams(theNextAnd); } - return theNextAnd.stream().anyMatch(token -> theSearchParams.matchParam(theResourceName, theParamName, paramDef, token)); + return theNextAnd.stream().anyMatch(token -> theSearchParams.matchParam(theResourceName, theParamName, paramDef, token, theUseOrdinalDatesForDayComparison)); } private void stripBaseUrlsFromReferenceParams(List theNextAnd) { From b1d7072c42d20e15d319675a923c51961b30e6c3 Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Wed, 26 Feb 2020 22:22:42 -0500 Subject: [PATCH 19/27] Add precision checker for ordinal date returning on DateRangeParam --- .../uhn/fhir/rest/param/DateRangeParam.java | 42 ++++++++++++++++++- .../ResourceIndexedSearchParamDate.java | 2 + .../InMemoryResourceMatcherR5Test.java | 4 +- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java index d5f491a0f07..a948c247b97 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java @@ -272,7 +272,27 @@ public class DateRangeParam implements IQueryParameterAnd { if (myLowerBound == null || myLowerBound.getValue() == null) { return null; } - return DateUtils.convertDatetoDayInteger(myLowerBound.getValue()); + //TODO LOOK AT THE DATE VERSION, WHERE PRECISION OCCASIONALLY CHANGES THE STUPID THING. ESSENTIALLY ADD A SWITCH + int retVal = DateUtils.convertDatetoDayInteger(myLowerBound.getValue()); + + if (myLowerBound.getPrefix() != null) { + switch (myLowerBound.getPrefix()) { + case GREATERTHAN: + case STARTS_AFTER: + retVal += 1; + break; + case EQUAL: + case GREATERTHAN_OR_EQUALS: + break; + case LESSTHAN: + case APPROXIMATE: + case LESSTHAN_OR_EQUALS: + case ENDS_BEFORE: + case NOT_EQUAL: + throw new IllegalStateException("Invalid lower bound comparator: " + myLowerBound.getPrefix()); + } + } + return retVal; } /** @@ -284,7 +304,25 @@ public class DateRangeParam implements IQueryParameterAnd { if (myUpperBound == null || myUpperBound.getValue() == null) { return null; } - return DateUtils.convertDatetoDayInteger(myUpperBound.getValue()); + int retVal = DateUtils.convertDatetoDayInteger(myUpperBound.getValue()); + if (myUpperBound.getPrefix() != null) { + switch (myUpperBound.getPrefix()) { + case LESSTHAN: + case ENDS_BEFORE: + retVal -= 1; + break; + case EQUAL: + case LESSTHAN_OR_EQUALS: + break; + case GREATERTHAN_OR_EQUALS: + case GREATERTHAN: + case APPROXIMATE: + case NOT_EQUAL: + case STARTS_AFTER: + throw new IllegalStateException("Invalid upper bound comparator: " + myUpperBound.getPrefix()); + } + } + return retVal; } public Date getLowerBoundAsInstant() { diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java index 9b2b4c93129..93e90200079 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java @@ -250,6 +250,7 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar boolean result; if (theUseOrdinalDatesForDayComparison) { result = matchesOrdinalDateBounds(range); + result = matchesDateBounds(range); } else { result = matchesDateBounds(range); } @@ -284,6 +285,7 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar return false; } if (lowerBoundAsDateInteger != null) { + //TODO as we run into equality issues result &= (myValueLowDateOrdinal.equals(lowerBoundAsDateInteger) || myValueLowDateOrdinal > lowerBoundAsDateInteger); result &= (myValueHighDateOrdinal.equals(lowerBoundAsDateInteger) || myValueHighDateOrdinal > lowerBoundAsDateInteger); } diff --git a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcherR5Test.java b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcherR5Test.java index bb111f87e86..daf4215ed68 100644 --- a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcherR5Test.java +++ b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcherR5Test.java @@ -148,8 +148,8 @@ public class InMemoryResourceMatcherR5Test { @Test public void testDateSupportedOps() { - testDateSupportedOp(ParamPrefixEnum.GREATERTHAN_OR_EQUALS, true, true, false); testDateSupportedOp(ParamPrefixEnum.GREATERTHAN, true, false, false); + testDateSupportedOp(ParamPrefixEnum.GREATERTHAN_OR_EQUALS, true, true, false); testDateSupportedOp(ParamPrefixEnum.EQUAL, false, true, false); testDateSupportedOp(ParamPrefixEnum.LESSTHAN_OR_EQUALS, false, true, true); testDateSupportedOp(ParamPrefixEnum.LESSTHAN, false, false, true); @@ -165,7 +165,7 @@ public class InMemoryResourceMatcherR5Test { { InMemoryMatchResult result = myInMemoryResourceMatcher.match(equation + OBSERVATION_DATE, myObservation, mySearchParams); assertTrue(result.getUnsupportedReason(), result.supported()); - assertEquals(result.matched(), theSame); + assertEquals(theSame, result.matched()); } { InMemoryMatchResult result = myInMemoryResourceMatcher.match(equation + LATE_DATE, myObservation, mySearchParams); From 6123e4b189e450952043cf29c705ccda0ad1a12e Mon Sep 17 00:00:00 2001 From: Gary Graham Date: Fri, 28 Feb 2020 16:31:16 -0500 Subject: [PATCH 20/27] rework based on PR comments --- .../dao/predicate/PredicateBuilderDate.java | 71 +++++++------------ 1 file changed, 27 insertions(+), 44 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java index d8bfb3a7876..fc509b95aa8 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java @@ -166,7 +166,8 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi DateParam upperBound = theRange.getUpperBound(); Integer lowerBoundAsOrdinal = theRange.getLowerBoundAsDateInteger(); Integer upperBoundAsOrdinal = theRange.getUpperBoundAsDateInteger(); - + Comparable genericLowerBound; + Comparable genericUpperBound; /** * If all present search parameters are of DAY precision, and {@link DaoConfig#getUseOrdinalDatesForDayPrecisionSearches()} is true, * then we attempt to use the ordinal field for date comparisons instead of the date field. @@ -177,70 +178,56 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi Predicate gt = null; Predicate lb = null; Predicate ub = null; + String lowValueField; + String highValueField; + + if (isOrdinalComparison) { + lowValueField = "myValueLowDateOrdinal"; + highValueField = "myValueHighDateOrdinal"; + genericLowerBound = lowerBoundAsOrdinal; + genericUpperBound = upperBoundAsOrdinal; + } else { + lowValueField = "myValueLow"; + highValueField = "myValueHigh"; + genericLowerBound = lowerBoundInstant; + genericUpperBound = upperBoundInstant; + } if (operation == SearchFilterParser.CompareOperation.lt) { if (lowerBoundInstant == null) { throw new InvalidRequestException("lowerBound value not correctly specified for compare operation"); } //im like 80% sure this should be ub and not lb, as it is an UPPER bound. - if (isOrdinalComparison) { - lb = theBuilder.lessThan(theFrom.get("myValueLowDateOrdinal"), lowerBoundAsOrdinal); - } else { - lb = theBuilder.lessThan(theFrom.get("myValueLow"), lowerBoundInstant); - } + lb = theBuilder.lessThan(theFrom.get(lowValueField), genericLowerBound); } else if (operation == SearchFilterParser.CompareOperation.le) { if (upperBoundInstant == null) { throw new InvalidRequestException("upperBound value not correctly specified for compare operation"); } //im like 80% sure this should be ub and not lb, as it is an UPPER bound. - if (isOrdinalComparison) { - lb = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), upperBoundAsOrdinal); - } else { - lb = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); - } + lb = theBuilder.lessThanOrEqualTo(theFrom.get(highValueField), genericUpperBound); } else if (operation == SearchFilterParser.CompareOperation.gt) { if (upperBoundInstant == null) { throw new InvalidRequestException("upperBound value not correctly specified for compare operation"); } - if (isOrdinalComparison) { - lb = theBuilder.greaterThan(theFrom.get("myValueHighDateOrdinal"), upperBoundAsOrdinal); - } else { - lb = theBuilder.greaterThan(theFrom.get("myValueHigh"), upperBoundInstant); - } + lb = theBuilder.greaterThan(theFrom.get(highValueField), genericUpperBound); } else if (operation == SearchFilterParser.CompareOperation.ge) { if (lowerBoundInstant == null) { throw new InvalidRequestException("lowerBound value not correctly specified for compare operation"); } - if (isOrdinalComparison) { - lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), lowerBoundAsOrdinal); - } else { - lb = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLow"), lowerBoundInstant); - } + lb = theBuilder.greaterThanOrEqualTo(theFrom.get(lowValueField), genericLowerBound); } else if (operation == SearchFilterParser.CompareOperation.ne) { if ((lowerBoundInstant == null) || (upperBoundInstant == null)) { throw new InvalidRequestException("lowerBound and/or upperBound value not correctly specified for compare operation"); } - if (isOrdinalComparison){ - lt = theBuilder.lessThan(theFrom.get("myValueLowDateOrdinal"), lowerBoundAsOrdinal); - gt = theBuilder.greaterThan(theFrom.get("myValueHighDateOrdinal"), upperBoundAsOrdinal); - } else { - lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLow"), lowerBoundInstant); - gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); - } + lt = theBuilder.lessThan(theFrom.get(lowValueField), genericLowerBound); + gt = theBuilder.greaterThan(theFrom.get(highValueField), genericUpperBound); lb = theBuilder.or(lt, gt); } else if ((operation == SearchFilterParser.CompareOperation.eq) || (operation == null)) { if (lowerBoundInstant != null) { - if (isOrdinalComparison) { - gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), lowerBoundAsOrdinal); - lt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), lowerBoundAsOrdinal); - //also try a strict equality here. - } - else { - gt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueLow"), lowerBoundInstant); - lt = theBuilder.greaterThanOrEqualTo(theFrom.get("myValueHigh"), lowerBoundInstant); - } + gt = theBuilder.greaterThanOrEqualTo(theFrom.get(lowValueField), genericLowerBound); + lt = theBuilder.greaterThanOrEqualTo(theFrom.get(highValueField), genericLowerBound); if (lowerBound.getPrefix() == ParamPrefixEnum.STARTS_AFTER || lowerBound.getPrefix() == ParamPrefixEnum.EQUAL) { lb = gt; } else { @@ -249,13 +236,9 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi } if (upperBoundInstant != null) { - if (isOrdinalComparison) { - gt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLowDateOrdinal"), upperBoundAsOrdinal); - lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHighDateOrdinal"), upperBoundAsOrdinal); - } else { - gt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueLow"), upperBoundInstant); - lt = theBuilder.lessThanOrEqualTo(theFrom.get("myValueHigh"), upperBoundInstant); - } + gt = theBuilder.lessThanOrEqualTo(theFrom.get(lowValueField), genericUpperBound); + lt = theBuilder.lessThanOrEqualTo(theFrom.get(highValueField), genericUpperBound); + if (theRange.getUpperBound().getPrefix() == ParamPrefixEnum.ENDS_BEFORE || theRange.getUpperBound().getPrefix() == ParamPrefixEnum.EQUAL) { ub = lt; From df7a9916c39d66422eb8a67f7d3fa4ebf4accaf3 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 3 Mar 2020 14:01:12 -0800 Subject: [PATCH 21/27] Swap from trace to info logging --- hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml b/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml index c8a33e5568b..6818eb0a0f3 100644 --- a/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml +++ b/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml @@ -27,7 +27,7 @@ - + From a5105439339d6cb94ed22308c97822932d20ad0e Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 3 Mar 2020 14:03:55 -0800 Subject: [PATCH 22/27] Fix log conditional --- .../ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java index fc509b95aa8..cb80ed64a5e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderDate.java @@ -252,8 +252,9 @@ public class PredicateBuilderDate extends BasePredicateBuilder implements IPredi } if (isOrdinalComparison) { ourLog.trace("Ordinal date range is {} - {} ", lowerBoundAsOrdinal, upperBoundAsOrdinal); + } else { + ourLog.trace("Date range is {} - {}", lowerBoundInstant, upperBoundInstant); } - ourLog.trace("Date range is {} - {}", lowerBoundInstant, upperBoundInstant); if (lb != null && ub != null) { return (theBuilder.and(lb, ub)); From 8b223a1fd9d7a34f22fdb688472ae59db524e93e Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 3 Mar 2020 16:07:25 -0800 Subject: [PATCH 23/27] Rework selection generation of calculator. Add null check for empty dates --- .../migrate/taskdef/BaseColumnCalculatorTask.java | 6 +++--- .../jpa/migrate/taskdef/BaseTableColumnTask.java | 14 +++++++++++++- .../fhir/jpa/migrate/taskdef/BaseTableTask.java | 3 ++- .../migrate/taskdef/CalculateOrdinalDatesTask.java | 4 +++- .../entity/ResourceIndexedSearchParamDate.java | 3 +++ 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseColumnCalculatorTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseColumnCalculatorTask.java index ddd5300bfcb..909d830bec3 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseColumnCalculatorTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseColumnCalculatorTask.java @@ -20,7 +20,6 @@ package ca.uhn.fhir.jpa.migrate.taskdef; * #L% */ -import ca.uhn.fhir.jpa.migrate.JdbcUtils; import ca.uhn.fhir.util.StopWatch; import ca.uhn.fhir.util.VersionEnum; import com.google.common.collect.ForwardingMap; @@ -77,8 +76,9 @@ public abstract class BaseColumnCalculatorTask extends BaseTableColumnTask { JdbcTemplate jdbcTemplate = newJdbcTemplate(); jdbcTemplate.setMaxRows(100000); - String sql = "SELECT * FROM " + getTableName() + " WHERE " + getColumnName() + " IS NULL"; - logInfo(ourLog, "Finding up to {} rows in {} that requires calculations", myBatchSize, getTableName()); + + String sql = "SELECT * FROM " + getTableName() + " WHERE " + getWhereClause(); + logInfo(ourLog, "Finding up to {} rows in {} that requires calculations, using query: {}", myBatchSize, getTableName(), sql); jdbcTemplate.query(sql, rch); rch.done(); diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableColumnTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableColumnTask.java index e4e024ed1e1..dc38d081ad4 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableColumnTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableColumnTask.java @@ -30,6 +30,8 @@ import java.util.Locale; public abstract class BaseTableColumnTask> extends BaseTableTask { private String myColumnName; + //If a concrete class decides to, they can define a custom WHERE clause for the task. + private String myWhereClause; public BaseTableColumnTask(String theProductVersion, String theSchemaVersion) { super(theProductVersion, theSchemaVersion); @@ -41,11 +43,21 @@ public abstract class BaseTableColumnTask> extends Ba return (T) this; } - public String getColumnName() { return myColumnName; } + protected void setWhereClause(String theWhereClause) { + this.myWhereClause = theWhereClause; + } + protected String getWhereClause() { + if (myWhereClause == null) { + return getColumnName() + " IS NULL"; + } else { + return myWhereClause; + } + } + @Override public void validate() { super.validate(); diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableTask.java index 3fce1b54fa3..73def6f5bc0 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableTask.java @@ -27,6 +27,8 @@ import org.apache.commons.lang3.builder.HashCodeBuilder; public abstract class BaseTableTask> extends BaseTask { private String myTableName; + + public BaseTableTask(String theProductVersion, String theSchemaVersion) { super(theProductVersion, theSchemaVersion); } @@ -34,7 +36,6 @@ public abstract class BaseTableTask> extends BaseTask public String getTableName() { return myTableName; } - public T setTableName(String theTableName) { Validate.notBlank(theTableName); myTableName = theTableName; diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java index d0d16c1e8d4..29371cc1c6d 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java @@ -26,8 +26,10 @@ public class CalculateOrdinalDatesTask extends BaseColumnCalculatorTask { public CalculateOrdinalDatesTask(VersionEnum theRelease, String theVersion) { super(theRelease, theVersion); - setDescription("Calculate SP_LOW_VALUE_DATE and SP_HIGH_VALUE_DATE based on existing SP_LOW and SP_HIGH date values in Date Search Params"); + setDescription("Calculate SP_LOW_VALUE_DATE_ORDINAL and SP_HIGH_VALUE_DATE_ORDINAL based on existing SP_VALUE_LOW and SP_VALUE_HIGH date values in Date Search Params"); + setWhereClause("SP_VALUE_LOW_DATE_ORDINAL IS NULL AND SP_VALUE_LOW IS NOT NULL) OR (SP_VALUE_HIGH_DATE_ORDINAL IS NULL AND SP_VALUE_HIGH IS NOT NULL"); } + @Override protected boolean shouldSkipTask() { return false; // TODO Is there a case where we should just not do this? diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java index 93e90200079..8ef1d524fe2 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDate.java @@ -298,6 +298,9 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar public static Long calculateOrdinalValue(Date theDate) { + if (theDate == null) { + return null; + } return (long) DateUtils.convertDatetoDayInteger(theDate); } From 14f671ec0bfc158a4dd3b5909983733dd15b744c Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 3 Mar 2020 22:31:37 -0800 Subject: [PATCH 24/27] Fix SQL Typo --- .../uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java index 29371cc1c6d..22e1c0ec206 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java @@ -27,7 +27,7 @@ public class CalculateOrdinalDatesTask extends BaseColumnCalculatorTask { public CalculateOrdinalDatesTask(VersionEnum theRelease, String theVersion) { super(theRelease, theVersion); setDescription("Calculate SP_LOW_VALUE_DATE_ORDINAL and SP_HIGH_VALUE_DATE_ORDINAL based on existing SP_VALUE_LOW and SP_VALUE_HIGH date values in Date Search Params"); - setWhereClause("SP_VALUE_LOW_DATE_ORDINAL IS NULL AND SP_VALUE_LOW IS NOT NULL) OR (SP_VALUE_HIGH_DATE_ORDINAL IS NULL AND SP_VALUE_HIGH IS NOT NULL"); + setWhereClause("SP_VALUE_LOW_DATE_ORDINAL IS NULL AND SP_VALUE_LOW IS NOT NULL) OR (SP_VALUE_HIGH_DATE_ORDINAL IS NULL AND SP_VALUE_HIGH IS NOT NULL)"); } @Override From 598bf56e871604eec1cdcf841de19990e4706c1e Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 4 Mar 2020 08:56:03 -0800 Subject: [PATCH 25/27] Broken SQL Statement --- .../uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java index 22e1c0ec206..78cbfda2474 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/CalculateOrdinalDatesTask.java @@ -27,7 +27,7 @@ public class CalculateOrdinalDatesTask extends BaseColumnCalculatorTask { public CalculateOrdinalDatesTask(VersionEnum theRelease, String theVersion) { super(theRelease, theVersion); setDescription("Calculate SP_LOW_VALUE_DATE_ORDINAL and SP_HIGH_VALUE_DATE_ORDINAL based on existing SP_VALUE_LOW and SP_VALUE_HIGH date values in Date Search Params"); - setWhereClause("SP_VALUE_LOW_DATE_ORDINAL IS NULL AND SP_VALUE_LOW IS NOT NULL) OR (SP_VALUE_HIGH_DATE_ORDINAL IS NULL AND SP_VALUE_HIGH IS NOT NULL)"); + setWhereClause("(SP_VALUE_LOW_DATE_ORDINAL IS NULL AND SP_VALUE_LOW IS NOT NULL) OR (SP_VALUE_HIGH_DATE_ORDINAL IS NULL AND SP_VALUE_HIGH IS NOT NULL)"); } @Override From 828ddd51854164d18b0f276af45cff0ac78330aa Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 4 Mar 2020 12:14:55 -0800 Subject: [PATCH 26/27] Update docs --- .../src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java | 1 - .../main/resources/ca/uhn/hapi/fhir/docs/server_jpa/search.md | 4 ---- 2 files changed, 5 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java index a948c247b97..609a49ebd06 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java @@ -272,7 +272,6 @@ public class DateRangeParam implements IQueryParameterAnd { if (myLowerBound == null || myLowerBound.getValue() == null) { return null; } - //TODO LOOK AT THE DATE VERSION, WHERE PRECISION OCCASIONALLY CHANGES THE STUPID THING. ESSENTIALLY ADD A SWITCH int retVal = DateUtils.convertDatetoDayInteger(myLowerBound.getValue()); if (myLowerBound.getPrefix() != null) { diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/search.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/search.md index 26fe08dc30c..44f8fc19b8f 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/search.md +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/search.md @@ -2,10 +2,6 @@ The HAPI FHIR JPA Server fully implements most [FHIR search](https://www.hl7.org/fhir/search.html) operations for most versions of FHIR. However, there are some known limitations of the current implementation. Here is a partial list of search functionality that is not currently supported in HAPI FHIR: -### Date searches without timestamp - -Searching by date with no timestamp currently doesn't match all records it should. See [Issue 1499](https://github.com/jamesagnew/hapi-fhir/issues/1499). - ### Chains within _has Chains within _has are not currently supported for performance reasons. For example, this search is not currently supported From a5e1b3d159b545f5806913cca21ba69a1784afa7 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 29 Apr 2020 16:18:02 -0700 Subject: [PATCH 27/27] Minor changes for merge conflicts --- .../migrate/taskdef/BaseColumnCalculatorTask.java | 9 +-------- .../jpa/migrate/taskdef/BaseTableColumnTask.java | 14 ++++++++++++-- .../migrate/tasks/HapiFhirJpaMigrationTasks.java | 10 +++++----- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseColumnCalculatorTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseColumnCalculatorTask.java index 909d830bec3..447a95df39c 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseColumnCalculatorTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseColumnCalculatorTask.java @@ -37,11 +37,10 @@ import java.util.*; import java.util.concurrent.*; import java.util.function.Function; -public abstract class BaseColumnCalculatorTask extends BaseTableColumnTask { +public abstract class BaseColumnCalculatorTask extends BaseTableColumnTask { protected static final Logger ourLog = LoggerFactory.getLogger(BaseColumnCalculatorTask.class); private int myBatchSize = 10000; - private Map, Object>> myCalculators = new HashMap<>(); private ThreadPoolExecutor myExecutor; public void setBatchSize(int theBatchSize) { @@ -191,12 +190,6 @@ public abstract class BaseColumnCalculatorTask extends BaseTableColumnTask, Object> theConsumer) { - Validate.isTrue(myCalculators.containsKey(theColumnName) == false); - myCalculators.put(theColumnName, theConsumer); - return this; - } - private class MyRowCallbackHandler implements RowCallbackHandler { private List> myRows = new ArrayList<>(); diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableColumnTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableColumnTask.java index 567ebf7e052..703e64c510b 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableColumnTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableColumnTask.java @@ -25,13 +25,17 @@ import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.thymeleaf.util.StringUtils; +import java.util.HashMap; import java.util.Locale; +import java.util.Map; +import java.util.function.Function; public abstract class BaseTableColumnTask extends BaseTableTask { - private String myColumnName; + protected Map, Object>> myCalculators = new HashMap<>(); + protected String myColumnName; //If a concrete class decides to, they can define a custom WHERE clause for the task. - private String myWhereClause; + protected String myWhereClause; public BaseTableColumnTask(String theProductVersion, String theSchemaVersion) { super(theProductVersion, theSchemaVersion); @@ -75,4 +79,10 @@ public abstract class BaseTableColumnTask extends BaseTableTask { super.generateHashCode(theBuilder); theBuilder.append(myColumnName); } + + public BaseTableColumnTask addCalculator(String theColumnName, Function, Object> theConsumer) { + Validate.isTrue(myCalculators.containsKey(theColumnName) == false); + myCalculators.put(theColumnName, theConsumer); + return this; + } } diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index 7b8fdb0c097..7220bf0ced6 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -78,9 +78,9 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { spidxDate.addColumn("20200225.2", "SP_VALUE_HIGH_DATE_ORDINAL").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.INT); spidxDate.addTask(new CalculateOrdinalDatesTask(VersionEnum.V4_3_0, "20200225.3") - .setColumnName("SP_VALUE_LOW_DATE_ORDINAL") //It doesn't matter which of the two we choose as they will both be null. .addCalculator("SP_VALUE_LOW_DATE_ORDINAL", t -> ResourceIndexedSearchParamDate.calculateOrdinalValue(t.getDate("SP_VALUE_LOW"))) .addCalculator("SP_VALUE_HIGH_DATE_ORDINAL", t -> ResourceIndexedSearchParamDate.calculateOrdinalValue(t.getDate("SP_VALUE_HIGH"))) + .setColumnName("SP_VALUE_LOW_DATE_ORDINAL") //It doesn't matter which of the two we choose as they will both be null. ); // @@ -541,8 +541,8 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { .withColumns("HASH_IDENTITY", "SP_LATITUDE", "SP_LONGITUDE"); spidxCoords .addTask(new CalculateHashesTask(VersionEnum.V3_5_0, "20180903.5") - .setColumnName("HASH_IDENTITY") .addCalculator("HASH_IDENTITY", t -> BaseResourceIndexedSearchParam.calculateHashIdentity(new PartitionSettings(), null, t.getResourceType(), t.getString("SP_NAME"))) + .setColumnName("HASH_IDENTITY") ); } @@ -564,8 +564,8 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { .dropIndex("20180903.9", "IDX_SP_DATE"); spidxDate .addTask(new CalculateHashesTask(VersionEnum.V3_5_0, "20180903.10") - .setColumnName("HASH_IDENTITY") .addCalculator("HASH_IDENTITY", t -> BaseResourceIndexedSearchParam.calculateHashIdentity(new PartitionSettings(), null, t.getResourceType(), t.getString("SP_NAME"))) + .setColumnName("HASH_IDENTITY") ); } @@ -585,8 +585,8 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { .withColumns("HASH_IDENTITY", "SP_VALUE"); spidxNumber .addTask(new CalculateHashesTask(VersionEnum.V3_5_0, "20180903.14") - .setColumnName("HASH_IDENTITY") .addCalculator("HASH_IDENTITY", t -> BaseResourceIndexedSearchParam.calculateHashIdentity(new PartitionSettings(), null, t.getResourceType(), t.getString("SP_NAME"))) + .setColumnName("HASH_IDENTITY") ); } @@ -622,10 +622,10 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { .withColumns("HASH_IDENTITY_SYS_UNITS", "SP_VALUE"); spidxQuantity .addTask(new CalculateHashesTask(VersionEnum.V3_5_0, "20180903.22") - .setColumnName("HASH_IDENTITY") .addCalculator("HASH_IDENTITY", t -> BaseResourceIndexedSearchParam.calculateHashIdentity(new PartitionSettings(), null, t.getResourceType(), t.getString("SP_NAME"))) .addCalculator("HASH_IDENTITY_AND_UNITS", t -> ResourceIndexedSearchParamQuantity.calculateHashUnits(new PartitionSettings(), null, t.getResourceType(), t.getString("SP_NAME"), t.getString("SP_UNITS"))) .addCalculator("HASH_IDENTITY_SYS_UNITS", t -> ResourceIndexedSearchParamQuantity.calculateHashSystemAndUnits(new PartitionSettings(), null, t.getResourceType(), t.getString("SP_NAME"), t.getString("SP_SYSTEM"), t.getString("SP_UNITS"))) + .setColumnName("HASH_IDENTITY") ); }