Repository: tobymao/sqlglot

Issue:
Incorrect transpiling of MOD function from trino to snowflake
Appears in 3.14
The following expression in Trino
`(MOD((DAY_OF_YEAR(world."orders.date") % 7) - 1 + 7, 7))`
is transpiled in Snowflake as 
`((DAYOFYEAR(orders.date) % 7) - 1 + 7 % 7)`

The transpiled expression returns incorrect result, as values could be negative (-1). Missing a parenthesis before the modulo operator, to make it correct.

Expected result:
`(MOD((DAYOFYEAR(orders.date) % 7) - 1 + 7, 7))`
or
`(((DAYOFYEAR(orders.date) % 7) - 1 + 7 ) % 7)`


**Fully reproducible code snippet**
```
from sqlglot import Dialects, parse_one
trino_sql = 'SELECT (MOD((DAY_OF_YEAR(orders.date) % 7) - 1 + 7, 7)) FROM orders'
parsed = parse_one(sql, dialect=Dialects.TRINO)
snowflake_sql = parsed.sql(dialect=Dialects.SNOWFLAKE)
```


**Official Documentation**
https://trino.io/docs/current/functions/datetime.html#day_of_year
https://trino.io/docs/current/functions/math.html#mod
https://docs.snowflake.com/en/sql-reference/functions/year
https://docs.snowflake.com/en/sql-reference/functions/mod

Discussion:

Patch:
diff --git a/sqlglot/dialects/bigquery.py b/sqlglot/dialects/bigquery.py
index 3d2fb3d9..cba74403 100644
--- a/sqlglot/dialects/bigquery.py
+++ b/sqlglot/dialects/bigquery.py
@@ -609,7 +609,7 @@ class BigQuery(Dialect):
             exp.IntDiv: rename_func("DIV"),
             exp.JSONFormat: rename_func("TO_JSON_STRING"),
             exp.Max: max_or_greatest,
-            exp.Mod: rename_func("MOD"),
+            exp.Mod: lambda self, e: self.func("MOD", e.this.unnest(), e.expression.unnest()),
             exp.MD5: lambda self, e: self.func("TO_HEX", self.func("MD5", e.this)),
             exp.MD5Digest: rename_func("MD5"),
             exp.Min: min_or_least,
diff --git a/sqlglot/generator.py b/sqlglot/generator.py
index 6c56d1c2..37c04409 100644
--- a/sqlglot/generator.py
+++ b/sqlglot/generator.py
@@ -2845,7 +2845,8 @@ class Generator(metaclass=_Generator):
                 stack.append(expression.right)
                 if expression.comments and self.comments:
                     for comment in expression.comments:
-                        op += f" /*{self.pad_comment(comment)}*/"
+                        if comment:
+                            op += f" /*{self.pad_comment(comment)}*/"
                 stack.extend((op, expression.left))
             return op
 
diff --git a/sqlglot/parser.py b/sqlglot/parser.py
index 8f5050ee..2f995f61 100644
--- a/sqlglot/parser.py
+++ b/sqlglot/parser.py
@@ -74,6 +74,17 @@ def build_extract_json_with_path(expr_type: t.Type[E]) -> t.Callable[[t.List, Di
     return _builder
 
 
+def build_mod(args: t.List) -> exp.Mod:
+    this = seq_get(args, 0)
+    expression = seq_get(args, 1)
+
+    # Wrap the operands if they are binary nodes, e.g. MOD(a + 1, 7) -> (a + 1) % 7
+    this = exp.Paren(this=this) if isinstance(this, exp.Binary) else this
+    expression = exp.Paren(this=expression) if isinstance(expression, exp.Binary) else expression
+
+    return exp.Mod(this=this, expression=expression)
+
+
 class _Parser(type):
     def __new__(cls, clsname, bases, attrs):
         klass = super().__new__(cls, clsname, bases, attrs)
@@ -123,7 +134,7 @@ class Parser(metaclass=_Parser):
         "LOG": build_logarithm,
         "LOG2": lambda args: exp.Log(this=exp.Literal.number(2), expression=seq_get(args, 0)),
         "LOG10": lambda args: exp.Log(this=exp.Literal.number(10), expression=seq_get(args, 0)),
-        "MOD": lambda args: exp.Mod(this=seq_get(args, 0), expression=seq_get(args, 1)),
+        "MOD": build_mod,
         "TIME_TO_TIME_STR": lambda args: exp.Cast(
             this=seq_get(args, 0),
             to=exp.DataType(this=exp.DataType.Type.TEXT),


Test patch:
diff --git a/tests/dialects/test_bigquery.py b/tests/dialects/test_bigquery.py
index 301cd572..338a979b 100644
--- a/tests/dialects/test_bigquery.py
+++ b/tests/dialects/test_bigquery.py
@@ -1464,3 +1464,13 @@ OPTIONS (
 
         with self.assertRaises(ParseError):
             transpile("SELECT JSON_OBJECT('a', 1, 'b') AS json_data", read="bigquery")
+
+    def test_mod(self):
+        for sql in ("MOD(a, b)", "MOD('a', b)", "MOD(5, 2)", "MOD((a + 1) * 8, 5 - 1)"):
+            with self.subTest(f"Testing BigQuery roundtrip of modulo operation: {sql}"):
+                self.validate_identity(sql)
+
+        self.validate_identity(
+            "MOD((a + 1), b)",
+            "MOD(a + 1, b)",
+        )
diff --git a/tests/dialects/test_dialect.py b/tests/dialects/test_dialect.py
index dda0eb2a..809cb6da 100644
--- a/tests/dialects/test_dialect.py
+++ b/tests/dialects/test_dialect.py
@@ -1681,6 +1681,26 @@ class TestDialect(Validator):
                 "tsql": "CAST(a AS FLOAT) / b",
             },
         )
+        self.validate_all(
+            "MOD(8 - 1 + 7, 7)",
+            write={
+                "": "(8 - 1 + 7) % 7",
+                "hive": "(8 - 1 + 7) % 7",
+                "presto": "(8 - 1 + 7) % 7",
+                "snowflake": "(8 - 1 + 7) % 7",
+                "bigquery": "MOD(8 - 1 + 7, 7)",
+            },
+        )
+        self.validate_all(
+            "MOD(a, b + 1)",
+            write={
+                "": "a % (b + 1)",
+                "hive": "a % (b + 1)",
+                "presto": "a % (b + 1)",
+                "snowflake": "a % (b + 1)",
+                "bigquery": "MOD(a, b + 1)",
+            },
+        )
 
     def test_typeddiv(self):
         typed_div = exp.Div(this=exp.column("a"), expression=exp.column("b"), typed=True)
diff --git a/tests/dialects/test_duckdb.py b/tests/dialects/test_duckdb.py
index 2d0af135..f736264c 100644
--- a/tests/dialects/test_duckdb.py
+++ b/tests/dialects/test_duckdb.py
@@ -803,7 +803,7 @@ class TestDuckDB(Validator):
             write={"duckdb": "SELECT (90 * INTERVAL '1' DAY)"},
         )
         self.validate_all(
-            "SELECT ((DATE_TRUNC('DAY', CAST(CAST(DATE_TRUNC('DAY', CURRENT_TIMESTAMP) AS DATE) AS TIMESTAMP) + INTERVAL (0 - (DAYOFWEEK(CAST(CAST(DATE_TRUNC('DAY', CURRENT_TIMESTAMP) AS DATE) AS TIMESTAMP)) % 7) - 1 + 7 % 7) DAY) + (7 * INTERVAL (-5) DAY))) AS t1",
+            "SELECT ((DATE_TRUNC('DAY', CAST(CAST(DATE_TRUNC('DAY', CURRENT_TIMESTAMP) AS DATE) AS TIMESTAMP) + INTERVAL (0 - ((DAYOFWEEK(CAST(CAST(DATE_TRUNC('DAY', CURRENT_TIMESTAMP) AS DATE) AS TIMESTAMP)) % 7) - 1 + 7) % 7) DAY) + (7 * INTERVAL (-5) DAY))) AS t1",
             read={
                 "presto": "SELECT ((DATE_ADD('week', -5, DATE_TRUNC('DAY', DATE_ADD('day', (0 - MOD((DAY_OF_WEEK(CAST(CAST(DATE_TRUNC('DAY', NOW()) AS DATE) AS TIMESTAMP)) % 7) - 1 + 7, 7)), CAST(CAST(DATE_TRUNC('DAY', NOW()) AS DATE) AS TIMESTAMP)))))) AS t1",
             },
diff --git a/tests/test_transpile.py b/tests/test_transpile.py
index 22085b32..dea9985b 100644
--- a/tests/test_transpile.py
+++ b/tests/test_transpile.py
@@ -551,6 +551,13 @@ FROM x""",
             pretty=True,
         )
 
+        self.validate(
+            """SELECT X FROM  catalog.db.table WHERE Y 
+        --
+        AND Z""",
+            """SELECT X FROM catalog.db.table WHERE Y AND Z""",
+        )
+
     def test_types(self):
         self.validate("INT 1", "CAST(1 AS INT)")
         self.validate("VARCHAR 'x' y", "CAST('x' AS VARCHAR) AS y")