diff --git a/solrorbit/conversion/query.py b/solrorbit/conversion/query.py index 41f26786..cf6b16ba 100644 --- a/solrorbit/conversion/query.py +++ b/solrorbit/conversion/query.py @@ -26,6 +26,7 @@ """ import logging +import re from datetime import datetime, timedelta from .field import normalize_field_name @@ -452,12 +453,7 @@ def _convert_single_agg(agg_name: str, agg_def: dict): if not field: logger.warning("date_histogram agg '%s' has no field — skipping", agg_name) return None - interval = ( - dh_conf.get("calendar_interval") - or dh_conf.get("fixed_interval") - or dh_conf.get("interval", "month") - ) - gap = _calendar_interval_to_solr_gap(interval) + gap = _date_histogram_gap(dh_conf, agg_name) facet_def = { "type": "range", "field": field, @@ -512,25 +508,82 @@ def _convert_single_agg(agg_name: str, agg_def: dict): return None -def _calendar_interval_to_solr_gap(interval: str) -> str: - """Convert an OpenSearch calendar_interval or fixed_interval to a Solr range gap string.""" - mapping = { - "minute": "+1MINUTE", - "1m": "+1MINUTE", - "hour": "+1HOUR", - "1h": "+1HOUR", - "day": "+1DAY", - "1d": "+1DAY", - "week": "+7DAYS", - "1w": "+7DAYS", - "month": "+1MONTH", - "1m_month": "+1MONTH", # avoid conflict with 1m (minute) - "quarter": "+3MONTHS", - "1q": "+3MONTHS", - "year": "+1YEAR", - "1y": "+1YEAR", - } - return mapping.get(str(interval).lower(), "+1MONTH") +CALENDAR_INTERVAL_NAMES = { + "minute": "+1MINUTE", + "hour": "+1HOUR", + "day": "+1DAY", + "week": "+7DAYS", + "month": "+1MONTH", + "quarter": "+3MONTHS", + "year": "+1YEAR", +} + +CALENDAR_INTERVAL_ABBREVIATIONS = { + "1m": "+1MINUTE", + "1h": "+1HOUR", + "1d": "+1DAY", + "1w": "+7DAYS", + "1M": "+1MONTH", + "1q": "+3MONTHS", + "1y": "+1YEAR", +} + +FIXED_INTERVAL_UNITS = { + "ms": "MILLI", + "s": "SECOND", + "m": "MINUTE", + "h": "HOUR", + "d": "DAY", +} + +FIXED_INTERVAL_PATTERN = re.compile(r"^(\d+)(ms|s|m|h|d)$") + +DEFAULT_GAP = "+1MONTH" + + +def _date_histogram_gap(dh_conf: dict, agg_name: str | None = None) -> str: + """ + Solr range gap for an OpenSearch date_histogram. + + The key the interval arrives under decides how it reads: ``calendar_interval: 1m`` + is one minute and ``calendar_interval: 1M`` is one month, while a fixed_interval is + a multiple of a fixed unit and takes no calendar names at all. The deprecated + ``interval`` key accepts either form, so both are tried. + + An interval that is neither is logged and falls back to DEFAULT_GAP. + """ + if "calendar_interval" in dh_conf: + interval = dh_conf["calendar_interval"] + gap = _calendar_interval_to_solr_gap(interval) + elif "fixed_interval" in dh_conf: + interval = dh_conf["fixed_interval"] + gap = _fixed_interval_to_solr_gap(interval) + else: + interval = dh_conf.get("interval", "month") + gap = _calendar_interval_to_solr_gap(interval) or _fixed_interval_to_solr_gap(interval) + if gap: + return gap + logger.warning( + "date_histogram agg '%s' has interval '%s', which is neither an OpenSearch " + "calendar_interval nor a fixed_interval — using %s, so the buckets will not " + "have the width the workload asks for.", + agg_name, interval, DEFAULT_GAP, + ) + return DEFAULT_GAP + + +def _calendar_interval_to_solr_gap(interval) -> str | None: + """Solr range gap for an OpenSearch calendar_interval, or None if it is not one.""" + return (CALENDAR_INTERVAL_ABBREVIATIONS.get(str(interval)) + or CALENDAR_INTERVAL_NAMES.get(str(interval).lower())) + + +def _fixed_interval_to_solr_gap(interval) -> str | None: + """Solr range gap for an OpenSearch fixed_interval, or None if it is not one.""" + match = FIXED_INTERVAL_PATTERN.match(str(interval)) + if not match: + return None + return "+{}{}".format(match.group(1), FIXED_INTERVAL_UNITS[match.group(2)]) OS_TO_PYTHON_FORMAT = { diff --git a/tests/unit/solr/test_workload_converter.py b/tests/unit/solr/test_workload_converter.py index fd1566bb..480117bd 100644 --- a/tests/unit/solr/test_workload_converter.py +++ b/tests/unit/solr/test_workload_converter.py @@ -32,6 +32,8 @@ translate_to_solr_json_dsl, _convert_aggregations_to_facets, _calendar_interval_to_solr_gap, + _date_histogram_gap, + _fixed_interval_to_solr_gap, ) @@ -397,13 +399,72 @@ def test_known_intervals(self): self.assertEqual("+1MONTH", _calendar_interval_to_solr_gap("month")) self.assertEqual("+1YEAR", _calendar_interval_to_solr_gap("year")) self.assertEqual("+1HOUR", _calendar_interval_to_solr_gap("hour")) + self.assertEqual("+7DAYS", _calendar_interval_to_solr_gap("week")) + self.assertEqual("+3MONTHS", _calendar_interval_to_solr_gap("quarter")) - def test_unknown_defaults_to_month(self): - self.assertEqual("+1MONTH", _calendar_interval_to_solr_gap("fortnight")) - - def test_case_insensitive(self): + def test_names_are_case_insensitive(self): self.assertEqual("+1MONTH", _calendar_interval_to_solr_gap("MONTH")) + def test_single_unit_abbreviations_are_case_sensitive(self): + """OpenSearch reads 1M as a month and 1m as a minute; both occur in real workloads.""" + self.assertEqual("+1MONTH", _calendar_interval_to_solr_gap("1M")) + self.assertEqual("+1MINUTE", _calendar_interval_to_solr_gap("1m")) + + def test_not_a_calendar_interval(self): + self.assertIsNone(_calendar_interval_to_solr_gap("fortnight")) + self.assertIsNone(_calendar_interval_to_solr_gap("60d")) + + +class TestFixedIntervalToSolrGap(unittest.TestCase): + def test_every_unit_opensearch_accepts(self): + self.assertEqual("+2699999MILLI", _fixed_interval_to_solr_gap("2699999ms")) + self.assertEqual("+30SECOND", _fixed_interval_to_solr_gap("30s")) + self.assertEqual("+90MINUTE", _fixed_interval_to_solr_gap("90m")) + self.assertEqual("+3HOUR", _fixed_interval_to_solr_gap("3h")) + self.assertEqual("+60DAY", _fixed_interval_to_solr_gap("60d")) + self.assertEqual("+2000DAY", _fixed_interval_to_solr_gap("2000d")) + + def test_not_a_fixed_interval(self): + self.assertIsNone(_fixed_interval_to_solr_gap("month")) + self.assertIsNone(_fixed_interval_to_solr_gap("1M")) + self.assertIsNone(_fixed_interval_to_solr_gap("60")) + + +class TestDateHistogramGap(unittest.TestCase): + def test_the_key_decides_how_the_interval_reads(self): + self.assertEqual("+1MONTH", _date_histogram_gap({"calendar_interval": "1M"})) + self.assertEqual("+1MINUTE", _date_histogram_gap({"calendar_interval": "1m"})) + self.assertEqual("+60DAY", _date_histogram_gap({"fixed_interval": "60d"})) + + def test_deprecated_interval_key_accepts_either_form(self): + self.assertEqual("+1MONTH", _date_histogram_gap({"interval": "1M"})) + self.assertEqual("+60DAY", _date_histogram_gap({"interval": "60d"})) + + def test_missing_interval_defaults_to_month(self): + self.assertEqual("+1MONTH", _date_histogram_gap({"field": "dropoff_datetime"})) + + def test_unconvertible_interval_warns(self): + with self.assertLogs("solrorbit.conversion.query", level="WARNING") as log: + gap = _date_histogram_gap({"calendar_interval": "fortnight"}, "dropoffs_over_time") + self.assertEqual("+1MONTH", gap) + self.assertTrue(any("fortnight" in msg for msg in log.output)) + self.assertTrue(any("dropoffs_over_time" in msg for msg in log.output)) + + +class TestDateHistogramFacetGap(unittest.TestCase): + def _gap(self, date_histogram): + body = {"aggs": {"dropoffs_over_time": {"date_histogram": date_histogram}}} + facets = _convert_aggregations_to_facets(body["aggs"]) + return facets["dropoffs_over_time"]["gap"] + + def test_fixed_interval_keeps_its_width(self): + gap = self._gap({"field": "dropoff_datetime", "fixed_interval": "60d"}) + self.assertEqual("+60DAY", gap) + + def test_calendar_month_abbreviation_is_not_a_minute(self): + gap = self._gap({"field": "dropoff_datetime", "calendar_interval": "1M"}) + self.assertEqual("+1MONTH", gap) + if __name__ == "__main__": unittest.main()