diff --git a/docs/api/filter.rst b/docs/api/filter.rst index bcd11ab3f..068c70b13 100644 --- a/docs/api/filter.rst +++ b/docs/api/filter.rst @@ -68,3 +68,14 @@ GeoRadius :members: :special-members: :exclude-members: __hash__ + + +Timestamp +========= + +.. currentmodule:: redisvl.query.filter + +.. autoclass:: Timestamp + :members: + :special-members: + :exclude-members: __hash__ diff --git a/redisvl/query/filter.py b/redisvl/query/filter.py index c452a2997..bd411a5fe 100644 --- a/redisvl/query/filter.py +++ b/redisvl/query/filter.py @@ -716,6 +716,24 @@ def _is_date_only(iso_string: str) -> bool: date_pattern = r"^\d{4}-\d{2}-\d{2}$" return bool(re.match(date_pattern, iso_string)) + @staticmethod + def _as_date(value: Any) -> Any: + """Normalize a date-only ISO string to a date, leaving anything else alone. + + Returns a datetime.date for a "YYYY-MM-DD" string, and the value + unchanged for every other input, including a date-shaped string that is + not a real calendar date. + """ + if isinstance(value, str) and Timestamp._is_date_only(value): + try: + return datetime.datetime.strptime(value, "%Y-%m-%d").date() + except ValueError: + # Date-shaped but not a real date, e.g. "2023-02-30": _is_date_only + # only checks the digit pattern. Hand it back so the caller below + # rejects it with one consistent message. + return value + return value + def _convert_to_timestamp(self, value, end_date=False): """ Convert various inputs to a Unix timestamp (seconds since epoch in UTC). @@ -737,6 +755,10 @@ def _convert_to_timestamp(self, value, end_date=False): # Already a Unix timestamp return float(value) + # Coerce before the fromisoformat call below, which would otherwise turn a + # date-only string into a midnight datetime and skip the end_date branch. + value = self._as_date(value) + if isinstance(value, str): # Parse ISO format try: @@ -825,13 +847,18 @@ def __gt__(self, other): """ Filter for timestamps greater than the specified value. + For a bare date (or date-only ISO string), this means after the *end* of + that UTC day, so the day itself is excluded. + Args: other: A datetime, date, ISO string, or Unix timestamp Returns: self: The filter object for method chaining """ - timestamp = self._convert_to_timestamp(other) + # end_date anchors a bare date to 23:59:59.999999 so the exclusive lower + # bound skips the whole day rather than just its first instant. + timestamp = self._convert_to_timestamp(other, end_date=True) self._set_value(timestamp, self.SUPPORTED_TYPES, FilterOperator.GT) return FilterExpression(str(self)) @@ -839,6 +866,9 @@ def __lt__(self, other): """ Filter for timestamps less than the specified value. + For a bare date (or date-only ISO string), this means before the *start* + of that UTC day, so the day itself is excluded. + Args: other: A datetime, date, ISO string, or Unix timestamp @@ -853,6 +883,9 @@ def __ge__(self, other): """ Filter for timestamps greater than or equal to the specified value. + For a bare date (or date-only ISO string), this means from the *start* of + that UTC day, so the day itself is included. + Args: other: A datetime, date, ISO string, or Unix timestamp @@ -867,13 +900,18 @@ def __le__(self, other): """ Filter for timestamps less than or equal to the specified value. + For a bare date (or date-only ISO string), this means through the *end* of + that UTC day, so the day itself is included. + Args: other: A datetime, date, ISO string, or Unix timestamp Returns: self: The filter object for method chaining """ - timestamp = self._convert_to_timestamp(other) + # end_date anchors a bare date to 23:59:59.999999 so the inclusive upper + # bound covers the whole day rather than just its first instant. + timestamp = self._convert_to_timestamp(other, end_date=True) self._set_value(timestamp, self.SUPPORTED_TYPES, FilterOperator.LE) return FilterExpression(str(self)) @@ -881,9 +919,15 @@ def between(self, start, end, inclusive: str = "both"): """ Filter for timestamps between start and end (inclusive). + Bare dates (and date-only ISO strings) span whole UTC calendar days: + start anchors to 00:00:00 of its day and end to 23:59:59.999999 of its + day, so both endpoint days are covered in full. + Args: start: A datetime, date, ISO string, or Unix timestamp end: A datetime, date, ISO string, or Unix timestamp + inclusive: Which endpoints to include -- "both" (default), "left", + "right", or "neither". Returns: self: The filter object for method chaining diff --git a/tests/unit/test_filter.py b/tests/unit/test_filter.py index c8c336dae..db1dd08d3 100644 --- a/tests/unit/test_filter.py +++ b/tests/unit/test_filter.py @@ -1,4 +1,5 @@ import calendar +import operator import time as time_module from datetime import date, datetime, time, timedelta, timezone @@ -554,6 +555,120 @@ def test_timestamp_operators(): assert str(ts) == f"@created_at:[({ts_value} {ts_value2}]" +# The four comparison operators, keyed by the symbol used in failure messages. +TIMESTAMP_COMPARISONS = { + ">": operator.gt, + "<": operator.lt, + ">=": operator.ge, + "<=": operator.le, +} + + +def test_timestamp_comparison_operators_with_date(): + """A bare date bounds the whole UTC day, so > and <= sit at its end. + + Hard-coded against 2023-03-17T00:00:00Z / T23:59:59.999999Z so the test + cannot drift along with the implementation. + """ + expected = { + ">": "@created_at:[(1679097599.999999 +inf]", + "<": "@created_at:[-inf (1679011200.0]", + ">=": "@created_at:[1679011200.0 +inf]", + "<=": "@created_at:[-inf 1679097599.999999]", + } + + for symbol, op in TIMESTAMP_COMPARISONS.items(): + # Bare dates and date-only ISO strings take the same branch + for value in (date(2023, 3, 17), "2023-03-17"): + assert ( + str(op(Timestamp("created_at"), value)) == expected[symbol] + ), f"{symbol} {value!r}" + + +def test_timestamp_between_date_only_string_matches_date_object(): + """A date-only string endpoint spans the same whole day a date object does. + + `end` is the interesting one: it goes through `end_date=True`, which only + reaches the end of the day if the string was coerced to a date first. + """ + by_date = str(Timestamp("created_at").between(date(2023, 3, 1), date(2023, 3, 17))) + by_string = str(Timestamp("created_at").between("2023-03-01", "2023-03-17")) + + assert by_string == by_date + assert by_date == "@created_at:[1677628800.0 1679097599.999999]" + + +@pytest.mark.parametrize("symbol", list(TIMESTAMP_COMPARISONS)) +@pytest.mark.parametrize("value", ["2023-02-30", "2023-13-45", "0000-00-00"]) +def test_timestamp_date_shaped_but_invalid_string(symbol, value): + """Every operator rejects a date-shaped non-date the same way. + + `_is_date_only` matches on the digit pattern alone, so these reach the + coercion and have to fall through to one shared error message. + """ + with pytest.raises(ValueError, match=f"must be in ISO format: {value}"): + TIMESTAMP_COMPARISONS[symbol](Timestamp("created_at"), value) + + +@pytest.mark.skipif(not hasattr(time_module, "tzset"), reason="tzset() is POSIX-only") +@pytest.mark.parametrize( + "d", + [ + date(2023, 3, 17), # ordinary date + date(2023, 3, 12), # US DST spring-forward + date(2023, 11, 5), # US DST fall-back + date(1970, 1, 1), # Unix epoch + date(2038, 1, 20), # past the signed 32-bit rollover + ], +) +def test_timestamp_comparison_date_bounds_are_utc_days(d, monkeypatch): + """Comparison operators bound the UTC day, whatever the host's local zone. + + CI runs in UTC, where the correct and the local-time conversions agree, so + this is the only test that would catch a regression to local-day bounds. + """ + # calendar.timegm is a timezone-independent oracle that shares no code with + # the conversion path under test. Caveat for anyone extending the list above: + # adding the microseconds back on is float-exact only for non-negative + # timestamps, so a pre-epoch date such as 1969-12-31 fails here spuriously + # (oracle -1.0000000000287557e-06 vs. a correct -1e-06). Assert those against + # datetime.combine(d, time.max, tzinfo=timezone.utc).timestamp() instead. + start = float(calendar.timegm(datetime.combine(d, time.min).timetuple())) + end = float(calendar.timegm(datetime.combine(d, time.max).timetuple())) + 0.999999 + + # > and <= exclude/include the whole day, so they anchor to its end; >= and < + # anchor to its start. + expected = { + ">": f"@created_at:[({end} +inf]", + "<": f"@created_at:[-inf ({start}]", + ">=": f"@created_at:[{start} +inf]", + "<=": f"@created_at:[-inf {end}]", + } + + try: + # Includes zones with non-hour offsets (+05:45, +12:45/+13:45), which an + # hour-granularity mistake would pass. + for tz in ( + "UTC", + "America/New_York", + "Asia/Tokyo", + "Asia/Kathmandu", + "Pacific/Chatham", + ): + monkeypatch.setenv("TZ", tz) + time_module.tzset() + + for symbol, op in TIMESTAMP_COMPARISONS.items(): + for value in (d, d.isoformat()): + assert ( + str(op(Timestamp("created_at"), value)) == expected[symbol] + ), f"{symbol} {value!r} in TZ={tz}" + finally: + # Restore TZ and re-read it, so later tests see the original zone + monkeypatch.undo() + time_module.tzset() + + def test_timestamp_between(): """Test the between method for date ranges.""" start = datetime(2023, 3, 1, 0, 0, 0, tzinfo=timezone.utc)