Refactor/95 value expression visitor (#101)

* #95 Refactored `ValueExpressionVisitor` * removed TreeNode structure; combined BooleanExpressionRenderer and ValueExpressionRenderer Co-authored-by: Sebastian Bär <[email protected]>
exasol · Nov 10, 2020 · 6493f7e · 6493f7e
1 parent a041d23
commit 6493f7e
Show file tree

Hide file tree

Showing 64 changed files with 560 additions and 1,118 deletions.
diff --git a/doc/changes/changes_4.3.1.md b/doc/changes/changes_4.3.1.md
@@ -7,3 +7,4 @@ Code Name: Refactoring
 * #98: Refactored comparison and like class structure
    The refactoring changed the internal representation of the `Comparison`.
    The public API from `BooleanTerm` did however not change.
+* #95: Refactored `ValueExpressionVisitor` 
diff --git a/doc/diagrams/interfaces.puml b/doc/diagrams/interfaces.puml
@@ -0,0 +1,13 @@
+@startuml
+interface ValueExpression
+interface BooleanExpression
+interface Function
+interface Comparison
+interface Literal
+
+ValueExpression <|-- BooleanExpression
+ValueExpression <|-- Function
+ValueExpression <|-- Literal
+BooleanExpression <|-- Comparison
+
+@enduml
diff --git a/src/main/java/com/exasol/sql/UnnamedPlaceholder.java b/src/main/java/com/exasol/sql/UnnamedPlaceholder.java
@@ -1,11 +1,12 @@
 package com.exasol.sql;
 
-import com.exasol.sql.expression.*;
+import com.exasol.sql.expression.ValueExpression;
+import com.exasol.sql.expression.ValueExpressionVisitor;
 
 /**
  * This class implements a nameless placeholder ("?") in an SQL statement
  */
-public class UnnamedPlaceholder extends AbstractValueExpression implements ValueExpression {
+public class UnnamedPlaceholder implements ValueExpression {
     @Override
     public void accept(final ValueExpressionVisitor visitor) {
         visitor.visit(this);

diff --git a/src/main/java/com/exasol/sql/ValueTableRow.java b/src/main/java/com/exasol/sql/ValueTableRow.java
@@ -1,8 +1,11 @@
 package com.exasol.sql;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 
-import com.exasol.sql.expression.*;
+import com.exasol.sql.expression.ValueExpression;
+import com.exasol.sql.expression.literal.*;
 
 /**
  * This class represents a row in a {@link ValueTable}.
@@ -14,7 +17,7 @@ public class ValueTableRow extends AbstractFragment {
     /**
      * Create a value table row from a list of expressions.
      *
-     * @param root root node of the SQL statement
+     * @param root        root node of the SQL statement
      * @param expressions value expressions
      */
     public ValueTableRow(final Fragment root, final ValueExpression... expressions) {
@@ -25,7 +28,7 @@ public ValueTableRow(final Fragment root, final ValueExpression... expressions)
     /**
      * Create a value table row from a list of string literals.
      *
-     * @param root root node of the SQL statement
+     * @param root   root node of the SQL statement
      * @param values sting literals
      */
     public ValueTableRow(final Fragment root, final String... values) {

diff --git a/src/main/java/com/exasol/sql/dml/merge/MergeUpdateClause.java b/src/main/java/com/exasol/sql/dml/merge/MergeUpdateClause.java
@@ -4,7 +4,9 @@
 import java.util.List;
 
 import com.exasol.sql.Fragment;
-import com.exasol.sql.expression.*;
+import com.exasol.sql.expression.DefaultValue;
+import com.exasol.sql.expression.ValueExpression;
+import com.exasol.sql.expression.literal.*;
 
 /**
  * Represents the {@code MERGE} strategy of updating matched rows.
@@ -24,7 +26,7 @@ public MergeUpdateClause(final Fragment root) {
     /**
      * Update a column with a value expression.
      *
-     * @param column column to be updated
+     * @param column     column to be updated
      * @param expression value expression
      * @return {@code this} for fluent programming
      */
@@ -36,7 +38,7 @@ public MergeUpdateClause set(final String column, final ValueExpression expressi
     /**
      * Update a column with a string value.
      *
-     * @param column column to be updated
+     * @param column  column to be updated
      * @param literal string literal
      * @return {@code this} for fluent programming
      */
@@ -47,7 +49,7 @@ public MergeUpdateClause set(final String column, final String literal) {
     /**
      * Update a column with a char value.
      *
-     * @param column column to be updated
+     * @param column  column to be updated
      * @param literal char literal
      * @return {@code this} for fluent programming
      */
@@ -58,7 +60,7 @@ public MergeUpdateClause set(final String column, final char literal) {
     /**
      * Update a column with an integer value.
      *
-     * @param column column to be updated
+     * @param column  column to be updated
      * @param literal integer literal
      * @return {@code this} for fluent programming
      */
@@ -69,7 +71,7 @@ public MergeUpdateClause set(final String column, final int literal) {
     /**
      * Update a column with a long value.
      *
-     * @param column column to be updated
+     * @param column  column to be updated
      * @param literal long literal
      * @return {@code this} for fluent programming
      */
@@ -80,7 +82,7 @@ public MergeUpdateClause set(final String column, final long literal) {
     /**
      * Update a column with a double value.
      *
-     * @param column column to be updated
+     * @param column  column to be updated
      * @param literal double literal
      * @return {@code this} for fluent programming
      */
@@ -91,7 +93,7 @@ public MergeUpdateClause set(final String column, final double literal) {
     /**
      * Update a column with a float value.
      *
-     * @param column column to be updated
+     * @param column  column to be updated
      * @param literal float literal
      * @return {@code this} for fluent programming
      */
@@ -102,7 +104,7 @@ public MergeUpdateClause set(final String column, final float literal) {
     /**
      * Update a column with a boolean value.
      *
-     * @param column column to be updated
+     * @param column  column to be updated
      * @param literal boolean literal
      * @return {@code this} for fluent programming
      */

diff --git a/src/main/java/com/exasol/sql/dml/merge/rendering/MergeRenderer.java b/src/main/java/com/exasol/sql/dml/merge/rendering/MergeRenderer.java
@@ -1,6 +1,9 @@
 package com.exasol.sql.dml.merge.rendering;
 
-import com.exasol.sql.*;
+import com.exasol.sql.DerivedColumn;
+import com.exasol.sql.Table;
+import com.exasol.sql.ValueTable;
+import com.exasol.sql.ValueTableRow;
 import com.exasol.sql.dml.insert.InsertFields;
 import com.exasol.sql.dml.insert.rendering.InsertRenderer;
 import com.exasol.sql.dml.merge.*;
@@ -47,7 +50,7 @@ public void visit(final UsingClause using) {
     @Override
     public void visit(final OnClause onClause) {
         appendKeyWord(" ON ");
-        appendRenderedBooleanExpression(onClause.getCondition());
+        appendRenderedValueExpression(onClause.getCondition());
         setLastVisited(onClause);
     }
 
@@ -155,7 +158,7 @@ public void leave(final ValueTableRow valueTableRow) {
     @Override
     public void visit(final WhereClause whereClause) {
         appendKeyWord(" WHERE ");
-        appendRenderedBooleanExpression(whereClause.getExpression());
+        appendRenderedValueExpression(whereClause.getExpression());
         setLastVisited(whereClause);
     }
 }
diff --git a/src/main/java/com/exasol/sql/dql/select/rendering/SelectRenderer.java b/src/main/java/com/exasol/sql/dql/select/rendering/SelectRenderer.java
@@ -2,7 +2,10 @@
 
 import java.util.List;
 
-import com.exasol.sql.*;
+import com.exasol.sql.DerivedColumn;
+import com.exasol.sql.Table;
+import com.exasol.sql.ValueTable;
+import com.exasol.sql.ValueTableRow;
 import com.exasol.sql.dql.select.*;
 import com.exasol.sql.expression.BooleanExpression;
 import com.exasol.sql.rendering.AbstractFragmentRenderer;
@@ -84,7 +87,7 @@ public void visit(final Join join) {
     @Override
     public void visit(final WhereClause whereClause) {
         appendKeyWord(" WHERE ");
-        appendRenderedBooleanExpression(whereClause.getExpression());
+        appendRenderedValueExpression(whereClause.getExpression());
         setLastVisited(whereClause);
     }
 
@@ -95,7 +98,7 @@ public void visit(final GroupByClause groupByClause) {
         final BooleanExpression having = groupByClause.getHavingBooleanExpression();
         if (having != null) {
             appendKeyWord(" HAVING ");
-            appendRenderedBooleanExpression(having);
+            appendRenderedValueExpression(having);
         }
         setLastVisited(groupByClause);
     }

diff --git a/src/main/java/com/exasol/sql/expression/AbstractBooleanExpression.java b/src/main/java/com/exasol/sql/expression/AbstractBooleanExpression.java
@@ -1,46 +1,9 @@
 package com.exasol.sql.expression;
 
-import com.exasol.util.AbstractBottomUpTreeNode;
-import com.exasol.util.TreeNode;
-
 /**
  * Abstract base class for all types of BooleanExpressions.
  */
-public abstract class AbstractBooleanExpression extends AbstractBottomUpTreeNode implements BooleanExpression {
-    protected AbstractBooleanExpression() {
-        super();
-    }
-
-    protected AbstractBooleanExpression(final BooleanExpression expression) {
-        super(expression);
-    }
-
-    protected AbstractBooleanExpression(final BooleanExpression... expressions) {
-        super(expressions);
-    }
-
-    @Override
-    public void accept(final BooleanExpressionVisitor visitor) {
-        acceptConcrete(visitor);
-        for (final TreeNode child : this.getChildren()) {
-            ((BooleanExpression) child).accept(visitor);
-        }
-        dismissConcrete(visitor);
-    }
-
-    /**
-     * Sub-classes must override this method so that the visitor knows the type of the visited class at compile time.
-     *
-     * @param visitor visitor to accept
-     */
-    public abstract void acceptConcrete(final BooleanExpressionVisitor visitor);
-
-    /**
-     * Sub-classes must override this method so that the visitor knows the type of the visited class at compile time.
-     *
-     * @param visitor visitor to accept
-     */
-    public abstract void dismissConcrete(final BooleanExpressionVisitor visitor);
+public abstract class AbstractBooleanExpression implements BooleanExpression {
 
     @Override
     public void accept(final ValueExpressionVisitor visitor) {

diff --git a/src/main/java/com/exasol/sql/expression/AbstractValueExpression.java b/src/main/java/com/exasol/sql/expression/AbstractValueExpression.java
diff --git a/src/main/java/com/exasol/sql/expression/And.java b/src/main/java/com/exasol/sql/expression/And.java
@@ -1,16 +1,23 @@
 package com.exasol.sql.expression;
 
+import java.util.Arrays;
+import java.util.List;
+
+import com.exasol.sql.expression.literal.BooleanLiteral;
+
 /**
  * This class represents a logical AND predicate.
  */
 public class And extends AbstractBooleanExpression {
+    private final List<BooleanExpression> operands;
+
     /**
      * Create a new {@link And} instance
      *
-     * @param expressions boolean expressions to be connected by a logical AND
+     * @param operands boolean expressions to be connected by a logical AND
      */
-    public And(final BooleanExpression... expressions) {
-        super(expressions);
+    public And(final BooleanExpression... operands) {
+        this.operands = Arrays.asList(operands);
     }
 
     /**
@@ -22,13 +29,17 @@ public And(final boolean... values) {
         this(BooleanLiteral.toBooleanExpressions(values));
     }
 
-    @Override
-    public void acceptConcrete(final BooleanExpressionVisitor visitor) {
-        visitor.visit(this);
+    /**
+     * Get the operands of this AND.
+     * 
+     * @return operands of this AND
+     */
+    public List<BooleanExpression> getOperands() {
+        return this.operands;
     }
 
     @Override
-    public void dismissConcrete(final BooleanExpressionVisitor visitor) {
-        visitor.leave(this);
+    public void accept(final BooleanExpressionVisitor visitor) {
+        visitor.visit(this);
     }
-}
+}
diff --git a/src/main/java/com/exasol/sql/expression/BinaryArithmeticExpression.java b/src/main/java/com/exasol/sql/expression/BinaryArithmeticExpression.java
@@ -1,14 +1,12 @@
 package com.exasol.sql.expression;
 
-import com.exasol.util.AbstractTreeNode;
-
 /**
  * This class represents a binary arithmetic expression.
  * <p>
  * Examples: +, -, *, /
  * </p>
  */
-public class BinaryArithmeticExpression extends AbstractTreeNode implements ValueExpression {
+public class BinaryArithmeticExpression implements ValueExpression {
     private final BinaryArithmeticOperator arithmeticOperator;
     private final ValueExpression left;
     private final ValueExpression right;
@@ -18,16 +16,14 @@ private BinaryArithmeticExpression(final BinaryArithmeticOperator arithmeticOper
         this.arithmeticOperator = arithmeticOperator;
         this.left = left;
         this.right = right;
-        addChild(left);
-        addChild(right);
     }
 
     /**
      * Create a new {@link BinaryArithmeticExpression} instance.
      * 
      * @param operator arithmetic operator represented by {@link BinaryArithmeticOperator}
-     * @param left left part of the expression
-     * @param right right part of the expression
+     * @param left     left part of the expression
+     * @param right    right part of the expression
      * @return new {@link BinaryArithmeticExpression} instance
      */
     public static BinaryArithmeticExpression of(final BinaryArithmeticOperator operator, final ValueExpression left,

diff --git a/src/main/java/com/exasol/sql/expression/BooleanExpressionVisitor.java b/src/main/java/com/exasol/sql/expression/BooleanExpressionVisitor.java
@@ -1,6 +1,7 @@
 package com.exasol.sql.expression;
 
 import com.exasol.sql.expression.comparison.Comparison;
+import com.exasol.sql.expression.literal.BooleanLiteral;
 
 /**
  * Visitor interface for a {@link BooleanTerm}
@@ -12,13 +13,7 @@ public interface BooleanExpressionVisitor {
 
     public void visit(And and);
 
-    public void leave(Not not);
-
-    public void leave(And and);
-
     public void visit(Or or);
 
-    public void leave(Or or);
-
     public void visit(Comparison comparison);
 }
diff --git a/src/main/java/com/exasol/sql/expression/BooleanTerm.java b/src/main/java/com/exasol/sql/expression/BooleanTerm.java
@@ -3,6 +3,7 @@
 import com.exasol.sql.expression.comparison.LikeComparison;
 import com.exasol.sql.expression.comparison.SimpleComparison;
 import com.exasol.sql.expression.comparison.SimpleComparisonOperator;
+import com.exasol.sql.expression.literal.BooleanLiteral;
 
 // [impl->dsn~boolean-operators~1]
 public abstract class BooleanTerm extends AbstractBooleanExpression {