Fixes SqlDialect output for unary ops (#1384)

partiql · Mar 9, 2024 · b938fda · b938fda · github-actions · Mar 9, 2024
1 parent 5121093
commit b938fda
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 7 deletions.
diff --git 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
@@ -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
@@ -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