address comments

Author: chelsea-lin

bigframes/core/compile/sqlglot/expressions/unary_compiler.py

@@ -187,7 +187,10 @@ def _(op: ops.base_ops.UnaryOp, expr: TypedExpr) -> sge.Expression:
 
 @UNARY_OP_REGISTRATION.register(ops.dayofweek_op)
 def _(op: ops.base_ops.UnaryOp, expr: TypedExpr) -> sge.Expression:
-    return sge.Extract(this=sge.Identifier(this="DAYOFWEEK"), expression=expr.expr)
+    # Plus 1 because iso day of week uses 1-based indexing
+    return sge.Extract(
+        this=sge.Identifier(this="DAYOFWEEK"), expression=expr.expr
+    ) + sge.convert(1)
 
 
 @UNARY_OP_REGISTRATION.register(ops.dayofyear_op)

tests/unit/core/compile/sqlglot/expressions/test_unary_compiler.py

@@ -37,6 +37,7 @@ def _apply_unary_op(obj: bpd.DataFrame, op: ops.UnaryOp, arg: str) -> str:
 def test_arccosh(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["float64_col"]]
     sql = _apply_unary_op(bf_df, ops.arccosh_op, "float64_col")
+
     snapshot.assert_match(sql, "out.sql")
 
 
@@ -57,6 +58,7 @@ def test_arcsin(scalar_types_df: bpd.DataFrame, snapshot):
 def test_arcsinh(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["float64_col"]]
     sql = _apply_unary_op(bf_df, ops.arcsinh_op, "float64_col")
+
     snapshot.assert_match(sql, "out.sql")
 
 
@@ -70,18 +72,21 @@ def test_arctan(scalar_types_df: bpd.DataFrame, snapshot):
 def test_arctanh(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["float64_col"]]
     sql = _apply_unary_op(bf_df, ops.arctanh_op, "float64_col")
+
     snapshot.assert_match(sql, "out.sql")
 
 
 def test_abs(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["float64_col"]]
     sql = _apply_unary_op(bf_df, ops.abs_op, "float64_col")
+
     snapshot.assert_match(sql, "out.sql")
 
 
 def test_capitalize(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["string_col"]]
     sql = _apply_unary_op(bf_df, ops.capitalize_op, "string_col")
+
     snapshot.assert_match(sql, "out.sql")
 
 
@@ -94,42 +99,49 @@ def test_ceil(scalar_types_df: bpd.DataFrame, snapshot):
 def test_date(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["timestamp_col"]]
     sql = _apply_unary_op(bf_df, ops.date_op, "timestamp_col")
+
     snapshot.assert_match(sql, "out.sql")
 
 
 def test_day(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["timestamp_col"]]
     sql = _apply_unary_op(bf_df, ops.day_op, "timestamp_col")
+
     snapshot.assert_match(sql, "out.sql")
 
 
 def test_dayofweek(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["timestamp_col"]]
     sql = _apply_unary_op(bf_df, ops.dayofweek_op, "timestamp_col")
+
     snapshot.assert_match(sql, "out.sql")
 
 
 def test_dayofyear(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["timestamp_col"]]
     sql = _apply_unary_op(bf_df, ops.dayofyear_op, "timestamp_col")
+
     snapshot.assert_match(sql, "out.sql")
 
 
 def test_exp(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["float64_col"]]
     sql = _apply_unary_op(bf_df, ops.exp_op, "float64_col")
+
     snapshot.assert_match(sql, "out.sql")
 
 
 def test_expm1(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["float64_col"]]
     sql = _apply_unary_op(bf_df, ops.expm1_op, "float64_col")
+
     snapshot.assert_match(sql, "out.sql")
 
 
 def test_floor(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["float64_col"]]
     sql = _apply_unary_op(bf_df, ops.floor_op, "float64_col")
+
     snapshot.assert_match(sql, "out.sql")
 
 
@@ -171,6 +183,7 @@ def test_cos(scalar_types_df: bpd.DataFrame, snapshot):
 def test_cosh(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["float64_col"]]
     sql = _apply_unary_op(bf_df, ops.cosh_op, "float64_col")
+
     snapshot.assert_match(sql, "out.sql")
 
 
@@ -219,6 +232,7 @@ def test_tan(scalar_types_df: bpd.DataFrame, snapshot):
 def test_tanh(scalar_types_df: bpd.DataFrame, snapshot):
     bf_df = scalar_types_df[["float64_col"]]
     sql = _apply_unary_op(bf_df, ops.tanh_op, "float64_col")
+
     snapshot.assert_match(sql, "out.sql")