Fix Copilot review issues for date support

- Reject DATE_FORMAT in WHERE conditions (format string not representable)
- Fix datetime parsing for fractional seconds (2024-01-01T12:00:00.123Z)
- Fix timezone offset normalization (+0000 -> +00:00 for fromisoformat)
- Fix date function alias mismatch: always compute canonical alias for FILTER
  even when SELECT uses custom alias
- Remove DATE_FORMAT from _is_date_function_condition (rejected at parse time)
- Add 4 new tests for edge cases

Author: nkanu17

sql_redis/parser.py

@@ -506,6 +506,13 @@ def _add_condition(
         elif isinstance(expression.this, exp.Anonymous):
             # Function call like DISTANCE(location, POINT(...)) or DATE_FORMAT
             func_name = expression.this.name.upper()
+            # DATE_FORMAT in WHERE is not supported - format string can't be
+            # represented in the Condition model. Use DATE_FORMAT in SELECT instead.
+            if func_name == "DATE_FORMAT":
+                raise ValueError(
+                    "DATE_FORMAT in WHERE conditions is not supported. "
+                    "Use DATE_FORMAT in the SELECT clause instead."
+                )
             if expression.this.expressions:
                 first_arg = expression.this.expressions[0]
                 if isinstance(first_arg, exp.Column):
@@ -645,32 +652,20 @@ def _parse_date_to_timestamp(self, value: str) -> int | None:
             # Normalize: replace space with T for parsing
             normalized = value.replace(" ", "T")
 
-            # Handle timezone variations
+            # Normalize 'Z' (UTC designator) to '+00:00' for fromisoformat
+            if normalized.endswith("Z"):
+                normalized = normalized[:-1] + "+00:00"
+
+            # Normalize timezone offsets without colon (+0000 -> +00:00)
+            # This ensures compatibility with datetime.fromisoformat
+            normalized = re.sub(r"([+-]\d{2})(\d{2})$", r"\1:\2", normalized)
+
             try:
-                # Try with Z suffix (UTC)
-                if normalized.endswith("Z"):
-                    dt = datetime.strptime(normalized, "%Y-%m-%dT%H:%M:%SZ")
+                # Use fromisoformat for robust parsing (handles fractional seconds)
+                dt = datetime.fromisoformat(normalized)
+                # If no timezone info, treat as UTC
+                if dt.tzinfo is None:
                     dt = dt.replace(tzinfo=timezone.utc)
-                    return int(dt.timestamp())
-
-                # Try with timezone offset (+00:00 or +0000)
-                if "+" in normalized or normalized.count("-") > 2:
-                    # Has timezone offset
-                    try:
-                        dt = datetime.fromisoformat(normalized)
-                        return int(dt.timestamp())
-                    except ValueError:
-                        pass
-
-                # No timezone - treat as UTC
-                # Handle optional milliseconds
-                if "." in normalized:
-                    dt = datetime.strptime(
-                        normalized.split(".")[0], "%Y-%m-%dT%H:%M:%S"
-                    )
-                else:
-                    dt = datetime.strptime(normalized, "%Y-%m-%dT%H:%M:%S")
-                dt = dt.replace(tzinfo=timezone.utc)
                 return int(dt.timestamp())
             except ValueError:
                 return None

sql_redis/translator.py

@@ -296,19 +296,27 @@ def _build_aggregate(
                     "are not supported"
                 )
 
-        # Track which date functions we've already computed (avoid duplicates)
-        computed_date_funcs = {
-            (df.function, df.field) for df in analyzed.date_functions
+        # Track which date functions we've already computed with canonical alias.
+        # Only skip if SELECT used the canonical alias (e.g., year_created_at).
+        # If SELECT used a custom alias (e.g., YEAR(created_at) AS year),
+        # we still need to compute the canonical alias for FILTER.
+        computed_canonical_aliases = {
+            (df.function, df.field)
+            for df in analyzed.date_functions
+            if df.alias == f"{df.function.lower()}_{df.field}"
         }
         for condition in date_func_conditions:
             parts = condition.operator.rsplit("_", 1)
             func_name = parts[0]
             redis_func = SQL_TO_REDIS_DATE_FUNCTIONS.get(func_name)
-            if redis_func and (func_name, condition.field) not in computed_date_funcs:
+            if (
+                redis_func
+                and (func_name, condition.field) not in computed_canonical_aliases
+            ):
                 expression = f"{redis_func}(@{condition.field})"
                 alias = f"{func_name.lower()}_{condition.field}"
                 args.extend(["APPLY", expression, "AS", alias])
-                computed_date_funcs.add((func_name, condition.field))
+                computed_canonical_aliases.add((func_name, condition.field))
 
         # FILTER for date function conditions
         for condition in date_func_conditions:
@@ -395,6 +403,8 @@ def _is_date_function_condition(self, condition) -> bool:
         """Check if a condition involves a date function.
 
         Date function conditions have operators like YEAR_=, MONTH_>, etc.
+        Note: DATE_FORMAT is excluded - it's rejected at parse time because
+        the format string can't be represented in the Condition model.
         """
         date_prefixes = (
             "YEAR_",
@@ -404,7 +414,6 @@ def _is_date_function_condition(self, condition) -> bool:
             "DAYOFYEAR_",
             "HOUR_",
             "MINUTE_",
-            "DATE_FORMAT_",
         )
         return condition.operator.startswith(date_prefixes)
 

tests/test_date_fields.py

@@ -61,6 +61,25 @@ def test_datetime_literal_with_timezone_offset(self, parser):
         assert len(result.conditions) == 1
         assert result.conditions[0].value == 1704110400
 
+    def test_datetime_literal_with_fractional_seconds(self, parser):
+        """Datetime literal with fractional seconds should be converted."""
+        result = parser.parse(
+            "SELECT * FROM events WHERE created_at = '2024-01-01T12:00:00.123Z'"
+        )
+
+        assert len(result.conditions) == 1
+        # Should parse successfully (fractional seconds truncated to int timestamp)
+        assert result.conditions[0].value == 1704110400
+
+    def test_datetime_literal_with_timezone_no_colon(self, parser):
+        """Datetime literal with +0000 format should be converted."""
+        result = parser.parse(
+            "SELECT * FROM events WHERE created_at = '2024-01-01T12:00:00+0000'"
+        )
+
+        assert len(result.conditions) == 1
+        assert result.conditions[0].value == 1704110400
+
     def test_date_between_range(self, parser):
         """BETWEEN with date literals should convert both bounds."""
         result = parser.parse(

tests/test_date_functions.py

@@ -210,3 +210,25 @@ def test_negated_date_function_raises_error(self, date_translator, date_index):
         sql = f"SELECT * FROM {date_index} WHERE NOT YEAR(created_at) = 2024"
         with pytest.raises(ValueError, match="Negated date function"):
             date_translator.translate(sql)
+
+    def test_date_format_in_where_raises_error(self, date_translator, date_index):
+        """DATE_FORMAT in WHERE should raise a clear error."""
+        sql = f"SELECT * FROM {date_index} WHERE DATE_FORMAT(created_at, '%Y-%m') = '2024-01'"
+        with pytest.raises(ValueError, match="DATE_FORMAT in WHERE"):
+            date_translator.translate(sql)
+
+    def test_custom_alias_with_where_filter(self, date_translator, date_index):
+        """SELECT with custom alias + WHERE should compute canonical alias for FILTER."""
+        result = date_translator.translate(
+            f"SELECT YEAR(created_at) AS year FROM {date_index} "
+            f"WHERE YEAR(created_at) = 2024"
+        )
+        # Should have APPLY for both: custom alias 'year' AND canonical 'year_created_at'
+        assert result.command == "FT.AGGREGATE"
+        apply_indices = [i for i, arg in enumerate(result.args) if arg == "APPLY"]
+        # Should have at least 2 APPLYs (one for SELECT, one for FILTER)
+        assert len(apply_indices) >= 2
+        # FILTER should reference the canonical alias
+        assert "FILTER" in result.args
+        filter_idx = result.args.index("FILTER")
+        assert "year_created_at" in result.args[filter_idx + 1]