Copilot commented on code in PR #74:
URL: https://github.com/apache/solr-orbit/pull/74#discussion_r3928465273
##########
solrorbit/conversion/query.py:
##########
@@ -196,8 +196,16 @@ def _translate_query_node(node: dict, fq_list: list =
None) -> str:
hi, hi_bracket = "*", "]"
# 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)
+ # A whole-day bound that OpenSearch rounds to the END of the day
+ # covers that day; Solr must be given the next day and told to
+ # exclude it. `gte`/`lt` round to the day's start, which is what
+ # the conversion above already produced.
+ if hi_is_date_only and "lte" in bounds:
+ hi, hi_bracket = _round_date_only_bound(hi), "}"
+ if lo_is_date_only and "gt" in bounds:
Review Comment:
The rounding logic for a date-only lower bound triggers whenever the input
contains a "gt" key, even if a "gte" key is also present (in which case the
code actually uses "gte"). This can incorrectly round and change bracket
semantics when both keys are provided.
##########
solrorbit/conversion/query.py:
##########
@@ -538,50 +561,62 @@ 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
+ if len(date_str) == 10: # YYYY-MM-DD, reached only if strptime rejected it
+ return date_str, True
Review Comment:
If parsing fails, the fallback `len(date_str) == 10` branch returns
`is_date_only=True` based only on string length. This can cause the caller to
apply whole-day rounding and/or change bracket exclusivity even though the
value was never successfully parsed as a date.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
