From c605947be73f21cbd7b8a72c2488539faca13468 Mon Sep 17 00:00:00 2001
From: Alan Cai <caialan@amazon.com>
Date: Fri, 8 Mar 2024 16:28:32 -0800
Subject: [PATCH] Fixes SqlDialect output for unary ops

---
 CHANGELOG.md                                  |  1 +
 .../kotlin/org/partiql/ast/sql/SqlDialect.kt  | 10 +++--
 .../org/partiql/ast/sql/SqlDialectTest.kt     | 45 +++++++++++++++++--
 3 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 28803d07d9..411def065c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -36,6 +36,7 @@ Thank you to all who have contributed!
 ### Deprecated
 
 ### Fixed
+- partiql-ast: `SqlDialect` will wrap unary ops (`NOT`, `+`, `-`) in parens
 
 ### Removed
 
diff --git a/partiql-ast/src/main/kotlin/org/partiql/ast/sql/SqlDialect.kt b/partiql-ast/src/main/kotlin/org/partiql/ast/sql/SqlDialect.kt
index bb77efa4c9..02847e7003 100644
--- a/partiql-ast/src/main/kotlin/org/partiql/ast/sql/SqlDialect.kt
+++ b/partiql-ast/src/main/kotlin/org/partiql/ast/sql/SqlDialect.kt
@@ -230,13 +230,15 @@ public abstract class SqlDialect : AstBaseVisitor<SqlBlock, SqlBlock>() {
 
     override fun visitExprUnary(node: Expr.Unary, head: SqlBlock): SqlBlock {
         val op = when (node.op) {
-            Expr.Unary.Op.NOT -> "NOT "
-            Expr.Unary.Op.POS -> "+"
-            Expr.Unary.Op.NEG -> "-"
+            Expr.Unary.Op.NOT -> "NOT ("
+            Expr.Unary.Op.POS -> "+("
+            Expr.Unary.Op.NEG -> "-("
         }
         var h = head
         h = h concat r(op)
-        return visitExprWrapped(node.expr, h)
+        h = visitExprWrapped(node.expr, h)
+        h = h concat r(")")
+        return h
     }
 
     override fun visitExprBinary(node: Expr.Binary, head: SqlBlock): SqlBlock {
diff --git a/partiql-ast/src/test/kotlin/org/partiql/ast/sql/SqlDialectTest.kt b/partiql-ast/src/test/kotlin/org/partiql/ast/sql/SqlDialectTest.kt
index 28b161bef6..039fbd4112 100644
--- a/partiql-ast/src/test/kotlin/org/partiql/ast/sql/SqlDialectTest.kt
+++ b/partiql-ast/src/test/kotlin/org/partiql/ast/sql/SqlDialectTest.kt
@@ -227,24 +227,63 @@ class SqlDialectTest {
 
         @JvmStatic
         fun exprOperators() = listOf(
-            expect("NOT NULL") {
+            expect("NOT (NULL)") {
                 exprUnary {
                     op = Expr.Unary.Op.NOT
                     expr = NULL
                 }
             },
-            expect("+NULL") {
+            expect("+(NULL)") {
                 exprUnary {
                     op = Expr.Unary.Op.POS
                     expr = NULL
                 }
             },
-            expect("-NULL") {
+            expect("-(NULL)") {
                 exprUnary {
                     op = Expr.Unary.Op.NEG
                     expr = NULL
                 }
             },
+            expect("NOT (NOT (NULL))") {
+                exprUnary {
+                    op = Expr.Unary.Op.NOT
+                    expr = exprUnary {
+                        op = Expr.Unary.Op.NOT
+                        expr = NULL
+                    }
+                }
+            },
+            expect("+(+(NULL))") {
+                exprUnary {
+                    op = Expr.Unary.Op.POS
+                    expr = exprUnary {
+                        op = Expr.Unary.Op.POS
+                        expr = NULL
+                    }
+                }
+            },
+            expect("-(-(NULL))") {
+                exprUnary {
+                    op = Expr.Unary.Op.NEG
+                    expr = exprUnary {
+                        op = Expr.Unary.Op.NEG
+                        expr = NULL
+                    }
+                }
+            },
+            expect("+(-(+(NULL)))") {
+                exprUnary {
+                    op = Expr.Unary.Op.POS
+                    expr = exprUnary {
+                        op = Expr.Unary.Op.NEG
+                        expr = exprUnary {
+                            op = Expr.Unary.Op.POS
+                            expr = NULL
+                        }
+                    }
+                }
+            },
             expect("NULL + NULL") {
                 exprBinary {
                     op = Expr.Binary.Op.PLUS