From 03fd7dcfd9b88755a4b4adc9d90173411a1e56c6 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Thu, 3 Sep 2026 15:48:23 +0300 Subject: [PATCH] Convert date bounds correctly: space separator, and whole-day rounding Two defects in _convert_date_to_solr_format, both reachable from upstream nyc_taxis bodies and neither addressed by the bracket fix. 1. A datetime written with a space, `"2016-01-01 00:00:00"`, had no pattern in the format map. The function logged "Could not parse date ... using as-is" and returned it unchanged, so the converter emitted dropoff_datetime:[2015-01-01 00:00:00 TO 2016-01-01 00:00:00} and Solr answers HTTP 400, "SyntaxError: Cannot parse ... Encountered ". The space ends the range term. Two nyc_taxis operations, date_histogram_calendar_interval and date_histogram_fixed_interval, are written this way upstream, so neither could run at all after conversion. 2. A bound with no time names a day, and OpenSearch rounds it to that day's edge: `lte` and `gt` to its last millisecond, `gte` and `lt` to its first. Solr rounds nothing. So `lte: "21/01/2015"` with format dd/MM/yyyy became the FIRST instant of the 21st, and the query lost a whole day of documents. The two bounds that round to the end of the day now name the start of the following day and exclude it, which says the same thing without depending on how fine Solr's date precision is. The function now reports whether its input named a day or an instant, because rounding cannot be decided from the value alone -- it needs the bound it belongs to. Rounding is gated on the bound that was actually used, not on the mere presence of a key. A range carrying both `gte` and `gt` takes its lower bound from `gte`, so gating on `"gt" in bounds` rounded a bound that rounds down: `gte: "01/01/2015"` came out as the 2nd. A value that could not be parsed is no longer reported as naming a whole day. That was decided from string length alone, which is reachable only when every format in the map has already failed, so it could only ever fire on something that is not a date - and the caller then flipped the bracket to exclusive while leaving the value untouched. `lte: "0000000010"` on a string field became `serial_no:[0000000001 TO 0000000010}`, silently excluding the bound asked for. Verified against the three real upstream nyc_taxis bodies: the output is now byte-identical to what the reviewed workload ships by hand. Each of the seven new tests fails on its own when the behaviour it describes is reverted, except the one guarding `gte`/`lt`, which must NOT move. --- solrorbit/conversion/query.py | 89 ++++++++++++++-------- tests/unit/solr/test_workload_converter.py | 52 +++++++++++++ 2 files changed, 109 insertions(+), 32 deletions(-) diff --git a/solrorbit/conversion/query.py b/solrorbit/conversion/query.py index 432b22b0..41f26786 100644 --- a/solrorbit/conversion/query.py +++ b/solrorbit/conversion/query.py @@ -26,7 +26,7 @@ """ import logging -from datetime import datetime +from datetime import datetime, timedelta from .field import normalize_field_name @@ -183,21 +183,25 @@ def _translate_query_node(node: dict, fq_list: list = None) -> str: field = normalize_field_name(field) # gt/lt are exclusive; Solr spells that with a curly bracket. if "gte" in bounds: - lo, lo_bracket = bounds["gte"], "[" + lo, lo_bracket, lo_key = bounds["gte"], "[", "gte" elif "gt" in bounds: - lo, lo_bracket = bounds["gt"], "{" + lo, lo_bracket, lo_key = bounds["gt"], "{", "gt" else: - lo, lo_bracket = "*", "[" + lo, lo_bracket, lo_key = "*", "[", None if "lte" in bounds: - hi, hi_bracket = bounds["lte"], "]" + hi, hi_bracket, hi_key = bounds["lte"], "]", "lte" elif "lt" in bounds: - hi, hi_bracket = bounds["lt"], "}" + hi, hi_bracket, hi_key = bounds["lt"], "}", "lt" else: - hi, hi_bracket = "*", "]" + hi, hi_bracket, hi_key = "*", "]", None # Convert dates if format is specified (common for date fields) os_format = bounds.get("format") - lo = _convert_date_to_solr_format(lo, os_format) - hi = _convert_date_to_solr_format(hi, os_format) + lo, lo_is_date_only = _convert_date_to_solr_format(lo, os_format) + hi, hi_is_date_only = _convert_date_to_solr_format(hi, os_format) + if hi_is_date_only and hi_key == "lte": + hi, hi_bracket = _round_date_only_bound(hi), "}" + if lo_is_date_only and lo_key == "gt": + lo, lo_bracket = _round_date_only_bound(lo), "[" return f"{field}:{lo_bracket}{lo} TO {hi}{hi_bracket}" if "exists" in node: @@ -529,7 +533,18 @@ def _calendar_interval_to_solr_gap(interval: str) -> str: return mapping.get(str(interval).lower(), "+1MONTH") -def _convert_date_to_solr_format(date_str, os_format=None) -> str: +OS_TO_PYTHON_FORMAT = { + "dd/MM/yyyy": ("%d/%m/%Y", False), + "MM/dd/yyyy": ("%m/%d/%Y", False), + "yyyy-MM-dd": ("%Y-%m-%d", False), + "yyyy/MM/dd": ("%Y/%m/%d", False), + "dd-MM-yyyy": ("%d-%m-%Y", False), + "MM-dd-yyyy": ("%m-%d-%Y", False), + "yyyy-MM-dd HH:mm:ss": ("%Y-%m-%d %H:%M:%S", True), +} + + +def _convert_date_to_solr_format(date_str, os_format=None) -> tuple: """ Convert an OpenSearch date string to Solr ISO 8601 format. @@ -538,50 +553,60 @@ def _convert_date_to_solr_format(date_str, os_format=None) -> str: os_format: Optional OpenSearch date format pattern (e.g., "dd/MM/yyyy") Returns: - ISO 8601 date string for Solr (e.g., "2015-01-01T00:00:00Z") + (value, is_date_only) — the ISO 8601 date string for Solr + (e.g., "2015-01-01T00:00:00Z"), and whether the source named a whole + day rather than an instant. If the date is already in ISO format or conversion fails, returns the original string unchanged. """ if not isinstance(date_str, str) or date_str in ("*", "now"): - return date_str - - # Map OpenSearch date format patterns to Python strptime format - OS_TO_PYTHON_FORMAT = { - "dd/MM/yyyy": "%d/%m/%Y", - "MM/dd/yyyy": "%m/%d/%Y", - "yyyy-MM-dd": "%Y-%m-%d", - "yyyy/MM/dd": "%Y/%m/%d", - "dd-MM-yyyy": "%d-%m-%Y", - "MM-dd-yyyy": "%m-%d-%Y", - # Add more as needed - } + return date_str, False # If format is provided, use it to parse the date if os_format: - python_fmt = OS_TO_PYTHON_FORMAT.get(os_format) - if python_fmt: + pattern = OS_TO_PYTHON_FORMAT.get(os_format) + if pattern: + python_fmt, has_time = pattern try: dt = datetime.strptime(date_str, python_fmt) - return dt.strftime("%Y-%m-%dT%H:%M:%SZ") + return dt.strftime("%Y-%m-%dT%H:%M:%SZ"), not has_time except ValueError: logger.warning(f"Failed to parse date '{date_str}' with format '{os_format}'") - return date_str + return date_str, False else: logger.warning(f"Unknown OpenSearch date format: '{os_format}'") # Try common patterns if no format specified - for python_fmt in OS_TO_PYTHON_FORMAT.values(): + for python_fmt, has_time in OS_TO_PYTHON_FORMAT.values(): try: dt = datetime.strptime(date_str, python_fmt) - return dt.strftime("%Y-%m-%dT%H:%M:%SZ") + return dt.strftime("%Y-%m-%dT%H:%M:%SZ"), not has_time except ValueError: continue # If it's already in ISO-like format, return as-is # (handles cases like "2015-01-01T00:00:00Z" or partial ISO) - if "T" in date_str or len(date_str) == 10: # YYYY-MM-DD - return date_str + if "T" in date_str: + return date_str, False logger.warning(f"Could not parse date '{date_str}', using as-is") - return date_str + return date_str, False + + +def _round_date_only_bound(value: str) -> str: + """ + Advance a whole-day bound to the start of the following day. + + A date without a time names a day, and OpenSearch rounds it to the edge of + that day: `lte` and `gt` go to its LAST millisecond, `gte` and `lt` to its + first. Solr rounds nothing, so only the two that move to the end of the day + need translating, and naming the next day's first instant says that without + depending on how fine Solr's date precision happens to be. + """ + try: + dt = datetime.strptime(value, "%Y-%m-%dT%H:%M:%SZ") + except ValueError: + logger.warning(f"Could not round whole-day bound '{value}', using as-is") + return value + return (dt + timedelta(days=1)).strftime("%Y-%m-%dT%H:%M:%SZ") diff --git a/tests/unit/solr/test_workload_converter.py b/tests/unit/solr/test_workload_converter.py index 423dc9c7..fd1566bb 100644 --- a/tests/unit/solr/test_workload_converter.py +++ b/tests/unit/solr/test_workload_converter.py @@ -228,6 +228,58 @@ def test_a_missing_bound_is_open_and_inclusive(self): self.assertEqual("fare_amount:[* TO 100}", self._range({"lt": 100})) self.assertEqual("fare_amount:[* TO 100]", self._range({"lte": 100})) + def _date_range(self, bounds): + return translate_to_solr_json_dsl( + {"query": {"range": {"dropoff_datetime": bounds}}})["query"] + + def test_a_datetime_with_a_space_separator_is_converted(self): + self.assertEqual( + "dropoff_datetime:[2015-01-01T00:00:00Z TO 2016-01-01T00:00:00Z}", + self._date_range({"gte": "2015-01-01 00:00:00", "lt": "2016-01-01 00:00:00"})) + + def test_a_whole_day_lte_covers_that_day(self): + self.assertEqual( + "dropoff_datetime:[2015-01-01T00:00:00Z TO 2015-01-22T00:00:00Z}", + self._date_range({"gte": "01/01/2015", "lte": "21/01/2015", + "format": "dd/MM/yyyy"})) + + def test_a_whole_day_gt_excludes_that_day(self): + self.assertEqual( + "dropoff_datetime:[2015-01-02T00:00:00Z TO *]", + self._date_range({"gt": "01/01/2015", "format": "dd/MM/yyyy"})) + + def test_the_bounds_that_round_down_do_not_move(self): + self.assertEqual( + "dropoff_datetime:[2015-01-01T00:00:00Z TO *]", + self._date_range({"gte": "01/01/2015", "format": "dd/MM/yyyy"})) + self.assertEqual( + "dropoff_datetime:[* TO 2015-01-01T00:00:00Z}", + self._date_range({"lt": "01/01/2015", "format": "dd/MM/yyyy"})) + + def test_only_the_bound_that_is_used_is_rounded(self): + self.assertEqual( + "dropoff_datetime:[2015-01-01T00:00:00Z TO 2015-01-22T00:00:00Z}", + self._date_range({"gte": "01/01/2015", "gt": "05/01/2015", + "lte": "21/01/2015", "lt": "10/01/2015", + "format": "dd/MM/yyyy"})) + + def test_a_bound_that_is_not_a_date_keeps_its_bracket(self): + for value in ("0000000010", "not-a-date", "2015-02-30"): + self.assertEqual( + f"serial_no:[0000000001 TO {value}]", + translate_to_solr_json_dsl( + {"query": {"range": {"serial_no": {"gte": "0000000001", + "lte": value}}}})["query"]) + + def test_an_instant_is_not_rounded(self): + self.assertEqual( + "dropoff_datetime:[2015-01-01T00:00:00Z TO 2016-01-01T00:00:00Z]", + self._date_range({"gte": "2015-01-01T00:00:00Z", + "lte": "2016-01-01T00:00:00Z"})) + self.assertEqual( + "dropoff_datetime:[* TO 2016-01-01T00:00:00Z]", + self._date_range({"lte": "2016-01-01 00:00:00"})) + def test_bool_with_filter_goes_to_fq(self): body = { "query": {