Reduce parentheses produced by PPL expression generator

- Found opensearch-project/sql#3272 (comment)
- Found opensearch-project/sql#3273

Signed-off-by: Simeon Widdis <[email protected]>

Author: Swiddis

src/main/scala/queries/ppl/Expr.scala

@@ -13,12 +13,12 @@ class PplNull(val flavor: "NULL" | "MISSING"):
   */
 type PplBoolean = PplNull | Boolean
 
-/** The `Expr` trait represents the top-level type for SQL expressions (e.g. the
-  * argument to a `WHERE` clause, or a variable used in a `SELECT` clause).
+/** The `Expr` trait represents the top-level type for PPL expressions (e.g. the
+  * argument to a `WHERE` clause, or a variable used in a `FIELDS` clause).
   *
   * @tparam T
-  *   The type that the expression evaluates to, used to avoid cases like
-  *   `SELECT * WHERE -NULL`.
+  *   The type that the expression evaluates to, used to avoid cases like `WHERE
+  *   -NULL`.
   */
 sealed trait Expr[T]:
   def serialize(): String
@@ -35,11 +35,24 @@ case class UnaryOp[A, B](op: String, arg: Expr[A]) extends Expr[B]:
 
 case class BinaryOp[A, B](left: Expr[A], op: String, right: Expr[A])
     extends Expr[B]:
-  // We always wrap binary ops in parentheses to make precedence "just work"
+  /** To avoid generating too many redundant parentheses, we only add
+    * parentheses around the sides of binary operations where order is more
+    * likely to matter.
+    *
+    * @see
+    *   https://github.com/opensearch-project/sql/issues/3272
+    */
+  private def serializeWithMaybeParens(ex: Expr[A]): String = {
+    ex match {
+      case BinaryOp(left, op, right) => "(" + ex.serialize() + ")"
+      case _                         => ex.serialize()
+    }
+  }
+
   override def serialize(): String =
-    "(" + op
-      .replace("$1", left.serialize())
-      .replace("$2", right.serialize()) + ")"
+    op
+      .replace("$1", serializeWithMaybeParens(left))
+      .replace("$2", serializeWithMaybeParens(right))
 
 /** `ExprGen` constructs ScalaCheck generators for `Expr`s.
   *

src/main/scala/queries/ppl/SourceQuery.scala

@@ -10,16 +10,8 @@ case class SourceQuery(
 ) extends QuerySerializable {
   override def serialize(): String = {
     val whereClause = where match {
-      case None => ""
-      case Some(ex) => {
-        val ser = ex.serialize()
-        // PPL can't handle outer-level parentheses for WHERE clauses, so we remove them if we're using a binary
-        // operator which generates them by default
-        // Tracking: https://github.com/opensearch-project/sql/issues/3272
-        if ex.isInstanceOf[BinaryOp[?, ?]] then
-          "| WHERE " + ser.substring(1, ser.length - 1)
-        else "| WHERE " + ser
-      }
+      case None     => ""
+      case Some(ex) => "| WHERE " + ex.serialize()
     }
     s"SOURCE = $index $whereClause"
   }