feat(duckdb): Addressed review comments, calculation logic updated to adhere to ISO day numbering conventions

Author: fivetran-amrutabhimsenayachit

sqlglot/dialects/bigquery.py

@@ -17,6 +17,7 @@
     date_add_interval_sql,
     datestrtodate_sql,
     build_formatted_time,
+    extract_week_unit_info,
     filter_array_using_unnest,
     if_sql,
     inline_array_unless_query,
@@ -209,45 +210,37 @@ def _ts_or_ds_diff_sql(self: BigQuery.Generator, expression: exp.TsOrDsDiff) ->
     return self.func("DATE_DIFF", expression.this, expression.expression, unit)
 
 
-def _serialize_bq_datetime_diff_unit(self: BigQuery.Generator, expression: exp.Expression) -> str:
+def _generate_bq_datetime_diff_unit(self: BigQuery.Generator, expression: exp.Expression) -> str:
     """
-    Serialize unit for *_DIFF functions, converting Week expressions to BigQuery syntax.
+    Generate unit for *_DIFF functions, converting Week expressions to BigQuery syntax.
 
     Canonical form -> BigQuery syntax:
     - Week(SUNDAY) -> WEEK (BigQuery's default)
     - Week(MONDAY) -> WEEK(MONDAY) (preserved during round-trip)
     - Var(ISOWEEK) -> ISOWEEK (preserved as-is)
-    - WeekStart -> preserved as-is
-    - Week(other day) -> WEEK(day)
-    - Other units -> use unit_to_var
 
     """
-    from sqlglot.dialects.dialect import extract_week_unit_info
-
     unit = expression.args.get("unit")
 
-    # Preserve ISOWEEK/WEEKISO as-is (don't convert to WEEK(MONDAY))
+    # Preserve ISOWEEK/WEEKISO as-is.
     if isinstance(unit, exp.Var):
-        unit_name = unit.this.upper() if isinstance(unit.this, str) else str(unit.this)
-        if unit_name in ("ISOWEEK", "WEEKISO"):
+        if unit.name.upper() in ("ISOWEEK", "WEEKISO"):
             return self.sql(unit)
 
     week_info = extract_week_unit_info(unit)
 
     if week_info:
-        day_name, _ = week_info  # Extract day name, ignore DOW number
+        day_name, _ = week_info
         if day_name == "SUNDAY":
-            return self.sql(exp.var("WEEK"))
-        elif day_name == "MONDAY":
-            if isinstance(unit, exp.WeekStart):
-                return self.sql(unit)
-            else:
-                return self.sql(exp.Week(this=exp.var(day_name)))
+            return "WEEK"
+        elif isinstance(unit, exp.WeekStart):
+            # Preserve WeekStart expressions as-is
+            return self.sql(unit)
         else:
+            # Convert to WEEK(day) for all other cases
             return self.sql(exp.Week(this=exp.var(day_name)))
 
-    unit_expr = unit_to_var(expression)
-    return self.sql(unit_expr) if unit_expr else "DAY"
+    return self.sql(unit_to_var(expression))
 
 
 def _unix_to_time_sql(self: BigQuery.Generator, expression: exp.UnixToTime) -> str:
@@ -286,18 +279,15 @@ def _build_datetime(args: t.List) -> exp.Func:
 def _normalize_week_unit(unit: t.Optional[exp.Expression]) -> t.Optional[exp.Expression]:
     """
     In BigQuery, plain WEEK defaults to Sunday-start weeks.
-     Normalize plain WEEK to WEEK(SUNDAY) to preserve the semantic in the AST for correct cross-dialect transpilation.
+    Normalize plain WEEK to WEEK(SUNDAY) to preserve the semantic in the AST for correct cross-dialect transpilation.
     """
-    unit_name = None
-
-    if isinstance(unit, exp.Var):
-        unit_name = str(unit.this)
-    elif isinstance(unit, exp.Column) and isinstance(unit.this, exp.Identifier):
-        unit_name = str(unit.this.this)
-
-    if unit_name and unit_name.upper() == "WEEK":
+    if isinstance(unit, exp.Var) and unit.name.upper() == "WEEK":
         return exp.Week(this=exp.var("SUNDAY"))
 
+    if isinstance(unit, exp.Column) and not unit.table and isinstance(unit.this, exp.Identifier):
+        if unit.name.upper() == "WEEK":
+            return exp.Week(this=exp.var("SUNDAY"))
+
     return unit
 
 
@@ -1165,7 +1155,7 @@ class Generator(generator.Generator):
             exp.CTE: transforms.preprocess([_pushdown_cte_column_names]),
             exp.DateAdd: date_add_interval_sql("DATE", "ADD"),
             exp.DateDiff: lambda self, e: self.func(
-                "DATE_DIFF", e.this, e.expression, _serialize_bq_datetime_diff_unit(self, e)
+                "DATE_DIFF", e.this, e.expression, _generate_bq_datetime_diff_unit(self, e)
             ),
             exp.DateFromParts: rename_func("DATE"),
             exp.DateStrToDate: datestrtodate_sql,

sqlglot/dialects/dialect.py

@@ -828,23 +828,24 @@ class Dialect(metaclass=_Dialect):
         "CENTURIES": "CENTURY",
     }
 
-    # Mapping of week unit names to (start_day, dow_number) tuples
-    # dow_number is the numeric day-of-week value (0=Sunday, 1=Monday, etc.)
+    # Mapping of week unit names to (start_day, iso_dow) tuples
+    # iso_dow follows ISO 8601 day-of-week numbering (Monday=1, ..., Sunday=7)
     WEEK_UNIT_SEMANTICS = {
-        "WEEK": ("SUNDAY", 0),
+        "WEEK": ("SUNDAY", 7),
         "ISOWEEK": ("MONDAY", 1),
         "WEEKISO": ("MONDAY", 1),
     }
 
-    # Days of week to DOW numbers (ISO standard: Sunday=0, Monday=1)
+    # Days of week to ISO 8601 day-of-week numbers
+    # ISO 8601 standard: Monday=1, Tuesday=2, Wednesday=3, Thursday=4, Friday=5, Saturday=6, Sunday=7
     WEEK_START_DAY_TO_DOW = {
-        "SUNDAY": 0,
         "MONDAY": 1,
         "TUESDAY": 2,
         "WEDNESDAY": 3,
         "THURSDAY": 4,
         "FRIDAY": 5,
         "SATURDAY": 6,
+        "SUNDAY": 7,
     }
 
     # Specifies what types a given type can be coerced into
@@ -1721,29 +1722,32 @@ def map_date_part(part, dialect: DialectType = Dialect):
 
 
 def extract_week_unit_info(
-    unit: t.Optional[exp.Expression], dialect: DialectType = Dialect
+    unit: t.Optional[exp.Expression], dialect: DialectType = None
 ) -> t.Optional[t.Tuple[str, int]]:
     """
     Extract week unit information from AST node.
 
     This helper provides a unified way to handle week units across dialects.
 
     Args:
-        unit: The unit expression (Var, Week, or WeekStart)
-        dialect: Dialect class or instance for accessing WEEK_UNIT_SEMANTICS and WEEK_START_DAY_TO_DOW
+        unit: The unit expression (Var for ISOWEEK/WEEKISO, Week, or WeekStart)
+        dialect: Dialect used to access WEEK_UNIT_SEMANTICS and WEEK_START_DAY_TO_DOW mappings.
 
     Returns:
-        - (day_name, dow_number) tuple (e.g., ("SUNDAY", 0) or ("MONDAY", 1))
-        - None if not a week unit or if day is dynamic (not a constant)
+        Tuple of (day_name, iso_dow) where iso_dow is ISO 8601 day number (Monday=1, Sunday=7),
+        or None if not a week unit or if day is dynamic (not a constant).
+
+        Examples:
+            Week(Var('SUNDAY')) → ('SUNDAY', 7)
+            Var('ISOWEEK') → ('MONDAY', 1)
+            Column('week') → None (dynamic, not a constant)
 
     """
     dialect_instance = Dialect.get_or_raise(dialect)
 
     # Handle plain Var expressions for ISOWEEK/WEEKISO only
-    # NOTE: Plain Var('WEEK') is NOT handled to avoid breaking other dialects
     if isinstance(unit, exp.Var):
-        unit_name = unit.this.upper() if isinstance(unit.this, str) else str(unit.this)
-        # Only handle ISOWEEK/WEEKISO variants, not plain WEEK
+        unit_name = unit.name.upper()
         if unit_name in ("ISOWEEK", "WEEKISO"):
             week_info = dialect_instance.WEEK_UNIT_SEMANTICS.get(unit_name)
             if week_info:
@@ -1754,7 +1758,7 @@ def extract_week_unit_info(
     if isinstance(unit, (exp.Week, exp.WeekStart)):
         day_var = unit.this
         if isinstance(day_var, exp.Var):
-            day_name = day_var.this.upper() if isinstance(day_var.this, str) else str(day_var.this)
+            day_name = day_var.name.upper()
             dow_value = dialect_instance.WEEK_START_DAY_TO_DOW.get(day_name)
             return (day_name, dow_value) if dow_value is not None else None
 

sqlglot/dialects/duckdb.py

@@ -253,36 +253,54 @@ def _implicit_datetime_cast(
 def _build_week_trunc_expression(date_expr: exp.Expression, start_dow: int) -> exp.Expression:
     """
     Build DATE_TRUNC expression for week boundaries with custom start day.
-    Formula: shift = 1 - start_dow, then DATE_TRUNC('week', date + INTERVAL shift DAY)
+
+    Args:
+        date_expr: The date expression to truncate
+        start_dow: ISO 8601 day-of-week number (Monday=1, ..., Sunday=7)
+
+    DuckDB's DATE_TRUNC('WEEK', ...) aligns weeks to Monday (ISO standard).
+    To align to a different start day, we shift the date before truncating.
+
+    Shift formula: Sunday (7) gets +1, others get (1 - start_dow)
+    Examples:
+        Monday (1): shift = 0 (no shift needed)
+        Tuesday (2): shift = -1 (shift back 1 day) ...
+        Sunday (7): shift = +1 (shift forward 1 day, wraps to next Monday-based week)
     """
-    if start_dow == 1:
-        # No shift needed for Monday-based weeks (ISO standard)
-        return exp.DateTrunc(unit=exp.var("week"), this=date_expr)
+    # Calculate shift: Sunday is special case, others use formula
+    shift_days = 1 if start_dow == 7 else 1 - start_dow
 
-    # Shift date to align week boundaries with ISO Monday
-    shift_days = 1 - start_dow
+    if shift_days == 0:
+        # No shift needed for Monday-based weeks
+        return exp.DateTrunc(unit=exp.var("WEEK"), this=date_expr)
+
+    # Shift date to align week boundaries with the desired start day
     shifted_date = exp.DateAdd(
         this=date_expr,
         expression=exp.Interval(this=exp.Literal.string(str(shift_days)), unit=exp.var("DAY")),
     )
 
-    return exp.DateTrunc(unit=exp.var("week"), this=shifted_date)
+    return exp.DateTrunc(unit=exp.var("WEEK"), this=shifted_date)
 
 
 def _date_diff_sql(self: DuckDB.Generator, expression: exp.DateDiff) -> str:
     """
     Generate DATE_DIFF SQL for DuckDB using DATE_TRUNC-based week alignment.
 
-    Note: When week start day is dynamic (from a column), week_start will be None
-    and we fall back to standard DATE_DIFF, since compile-time offsets are required.
+    DuckDB's DATE_DIFF doesn't support WEEK(day) units. We transpile these to
+    DATE_TRUNC-based calculations when the day is a compile-time constant.
+
+    Non-literal week units (e.g., columns, placeholders) cannot be transpiled safely
+    and are passed through as-is, following the pattern used elsewhere in sqlglot
+
     """
     from sqlglot.dialects.dialect import extract_week_unit_info
 
     this = _implicit_datetime_cast(expression.this)
     expr = _implicit_datetime_cast(expression.expression)
     unit = expression.args.get("unit")
 
-    # Extract week start day; returns None if day is dynamic (column reference)
+    # Extract week start day; returns None if day is dynamic (column/placeholder)
     week_start = extract_week_unit_info(unit)
     if week_start and this and expr:
         _, start_dow = week_start
@@ -293,7 +311,7 @@ def _date_diff_sql(self: DuckDB.Generator, expression: exp.DateDiff) -> str:
 
         # Calculate week difference
         day_diff = exp.DateDiff(
-            this=truncated_this, expression=truncated_expr, unit=exp.Literal.string("day")
+            this=truncated_this, expression=truncated_expr, unit=exp.Literal.string("DAY")
         )
         result = exp.IntDiv(this=day_diff, expression=exp.Literal.number(7))