Merge pull request #286 from domaframework/removable-clause

Support automatic removal of unnecessary ORDER BY and GROUP BY clauses

Author: nakamura-to

docs/sql.rst

@@ -463,11 +463,17 @@ You can nest condition directives as follows:
     /*%end*/
   /*%end*/
 
-Removal of WHERE and HAVING clauses on condition directive
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Removal of clauses on the condition directive
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Following clauses can become unnecessary on the condition directive:
+
+* WHERE
+* HAVING
+* ORDER BY
+* GROUP BY
 
-WHERE and HAVING clauses can be unnecessary on condition directive.
-Those clauses are removed automatically.
+In the case, they are removed automatically.
 
 Suppose you have the following SQL template:
 
@@ -488,11 +494,11 @@ the generated SQL statement is as follows:
 Because the SQL clause ``where`` followed by ``/*%if ...*/`` is unnecessary,
 it is removed automatically.
 
-Removal of AND and OR keywords on condition directives
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Removal of AND and OR keywords on the condition directives
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-AND and OR keywords can be unnecessary on condition directive.
-Those clauses are removed automatically.
+AND and OR keywords can become unnecessary on the condition directive.
+In the case, they are removed automatically.
 
 Suppose you have the following SQL template:
 
@@ -588,11 +594,17 @@ the generated SQL statement is as follows:
   or
   employee_name like ?
 
-Removal of WHERE and HAVING clauses on loop directive
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Removal of clauses on the loop directive
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Following clauses can become unnecessary on the loop directive:
+
+* WHERE
+* HAVING
+* ORDER BY
+* GROUP BY
 
-WHERE and HAVING clauses can be unnecessary on loop directive.
-Those clauses are removed automatically.
+In the case, they are removed automatically.
 
 Suppose you have the following SQL template:
 
@@ -616,11 +628,11 @@ the generated SQL statement is as follows:
 Because the SQL clause ``where`` followed by ``/*%for ...*/`` is unnecessary,
 it is removed automatically.
 
-Removal of AND and OR keywords on loop directive
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Removal of AND and OR keywords on the loop directive
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-AND and OR keywords can be unnecessary on loop directive.
-Those keywords are removed automatically.
+AND and OR keywords can become unnecessary on the loop directive.
+In the case, they are removed automatically.
 
 Suppose you have the following SQL template:
 

src/main/java/org/seasar/doma/internal/jdbc/sql/NodePreparedSqlBuilder.java

@@ -24,7 +24,6 @@
 import org.seasar.doma.internal.jdbc.scalar.Scalars;
 import org.seasar.doma.internal.jdbc.sql.node.AnonymousNode;
 import org.seasar.doma.internal.jdbc.sql.node.BindVariableNode;
-import org.seasar.doma.internal.jdbc.sql.node.ClauseNode;
 import org.seasar.doma.internal.jdbc.sql.node.CommentNode;
 import org.seasar.doma.internal.jdbc.sql.node.ElseNode;
 import org.seasar.doma.internal.jdbc.sql.node.ElseifNode;
@@ -48,6 +47,7 @@
 import org.seasar.doma.internal.jdbc.sql.node.OtherNode;
 import org.seasar.doma.internal.jdbc.sql.node.ParensNode;
 import org.seasar.doma.internal.jdbc.sql.node.PopulateNode;
+import org.seasar.doma.internal.jdbc.sql.node.RemovableClauseNode;
 import org.seasar.doma.internal.jdbc.sql.node.SelectClauseNode;
 import org.seasar.doma.internal.jdbc.sql.node.SelectStatementNode;
 import org.seasar.doma.internal.jdbc.sql.node.SetClauseNode;
@@ -541,23 +541,19 @@ public Void visitFromClauseNode(FromClauseNode node, Context p) {
 
   @Override
   public Void visitWhereClauseNode(WhereClauseNode node, Context p) {
-    handleConditionalClauseNode(node, p);
+    handleRemovableClauseNode(node, p);
     return null;
   }
 
   @Override
   public Void visitGroupByClauseNode(GroupByClauseNode node, Context p) {
-    WordNode wordNode = node.getWordNode();
-    wordNode.accept(this, p);
-    for (SqlNode child : node.getChildren()) {
-      child.accept(this, p);
-    }
+    handleRemovableClauseNode(node, p);
     return null;
   }
 
   @Override
   public Void visitHavingClauseNode(HavingClauseNode node, Context p) {
-    handleConditionalClauseNode(node, p);
+    handleRemovableClauseNode(node, p);
     return null;
   }
 
@@ -573,11 +569,7 @@ public Void visitOptionClauseNode(OptionClauseNode node, Context p) {
 
   @Override
   public Void visitOrderByClauseNode(OrderByClauseNode node, Context p) {
-    WordNode wordNode = node.getWordNode();
-    wordNode.accept(this, p);
-    for (SqlNode child : node.getChildren()) {
-      child.accept(this, p);
-    }
+    handleRemovableClauseNode(node, p);
     return null;
   }
 
@@ -591,7 +583,7 @@ public Void visitForUpdateClauseNode(ForUpdateClauseNode node, Context p) {
     return null;
   }
 
-  protected void handleConditionalClauseNode(ClauseNode node, Context p) {
+  protected void handleRemovableClauseNode(RemovableClauseNode node, Context p) {
     Context context = new Context(p);
     for (SqlNode child : node.getChildren()) {
       child.accept(this, context);

src/main/java/org/seasar/doma/internal/jdbc/sql/node/GroupByClauseNode.java

@@ -3,7 +3,7 @@
 import org.seasar.doma.DomaNullPointerException;
 import org.seasar.doma.jdbc.SqlNodeVisitor;
 
-public class GroupByClauseNode extends AbstractClauseNode {
+public class GroupByClauseNode extends AbstractClauseNode implements RemovableClauseNode {
 
   public GroupByClauseNode(String word) {
     super(word);

src/main/java/org/seasar/doma/internal/jdbc/sql/node/HavingClauseNode.java

@@ -3,7 +3,7 @@
 import org.seasar.doma.DomaNullPointerException;
 import org.seasar.doma.jdbc.SqlNodeVisitor;
 
-public class HavingClauseNode extends AbstractClauseNode {
+public class HavingClauseNode extends AbstractClauseNode implements RemovableClauseNode {
 
   public HavingClauseNode(String word) {
     super(word);

src/main/java/org/seasar/doma/internal/jdbc/sql/node/OrderByClauseNode.java

@@ -3,7 +3,7 @@
 import org.seasar.doma.DomaNullPointerException;
 import org.seasar.doma.jdbc.SqlNodeVisitor;
 
-public class OrderByClauseNode extends AbstractClauseNode {
+public class OrderByClauseNode extends AbstractClauseNode implements RemovableClauseNode {
 
   public OrderByClauseNode(String word) {
     super(word);

src/main/java/org/seasar/doma/internal/jdbc/sql/node/RemovableClauseNode.java

@@ -0,0 +1,3 @@
+package org.seasar.doma.internal.jdbc.sql.node;
+
+public interface RemovableClauseNode extends ClauseNode {}

src/main/java/org/seasar/doma/internal/jdbc/sql/node/WhereClauseNode.java

@@ -3,7 +3,7 @@
 import org.seasar.doma.DomaNullPointerException;
 import org.seasar.doma.jdbc.SqlNodeVisitor;
 
-public class WhereClauseNode extends AbstractClauseNode {
+public class WhereClauseNode extends AbstractClauseNode implements RemovableClauseNode {
 
   public WhereClauseNode(String word) {
     super(word);

src/test/java/org/seasar/doma/internal/jdbc/sql/SqlParserTest.java

@@ -448,6 +448,36 @@ public void testIf_removeWhere() throws Exception {
     assertEquals(0, sql.getParameters().size());
   }
 
+  public void testIf_removeOrderBy() throws Exception {
+    ExpressionEvaluator evaluator = new ExpressionEvaluator();
+    evaluator.add("name", new Value(String.class, null));
+    String testSql = "select * from aaa order by /*%if name != null*/bbb/*%end*/";
+    SqlParser parser = new SqlParser(testSql);
+    SqlNode sqlNode = parser.parse();
+    PreparedSql sql =
+        new NodePreparedSqlBuilder(
+                config, SqlKind.SELECT, "dummyPath", evaluator, SqlLogType.FORMATTED)
+            .build(sqlNode, Function.identity());
+    assertEquals("select * from aaa", sql.getRawSql());
+    assertEquals("select * from aaa", sql.getFormattedSql());
+    assertEquals(0, sql.getParameters().size());
+  }
+
+  public void testIf_removeGroupBy() throws Exception {
+    ExpressionEvaluator evaluator = new ExpressionEvaluator();
+    evaluator.add("name", new Value(String.class, null));
+    String testSql = "select * from aaa group by /*%if name != null*/bbb/*%end*/";
+    SqlParser parser = new SqlParser(testSql);
+    SqlNode sqlNode = parser.parse();
+    PreparedSql sql =
+        new NodePreparedSqlBuilder(
+                config, SqlKind.SELECT, "dummyPath", evaluator, SqlLogType.FORMATTED)
+            .build(sqlNode, Function.identity());
+    assertEquals("select * from aaa", sql.getRawSql());
+    assertEquals("select * from aaa", sql.getFormattedSql());
+    assertEquals(0, sql.getParameters().size());
+  }
+
   public void testIf_removeAnd() throws Exception {
     ExpressionEvaluator evaluator = new ExpressionEvaluator();
     evaluator.add("name", new Value(String.class, null));
@@ -628,6 +658,40 @@ public void testFor_removeWhere() throws Exception {
     assertEquals(0, sql.getParameters().size());
   }
 
+  public void testFor_removeOrderBy() throws Exception {
+    ExpressionEvaluator evaluator = new ExpressionEvaluator();
+    ArrayList<String> list = new ArrayList<String>();
+    evaluator.add("names", new Value(List.class, list));
+    String testSql =
+        "select * from aaa order by /*%for n : names*/name = /*n*/'a' /*%if n_has_next */, /*%end*//*%end*/";
+    SqlParser parser = new SqlParser(testSql);
+    SqlNode sqlNode = parser.parse();
+    PreparedSql sql =
+        new NodePreparedSqlBuilder(
+                config, SqlKind.SELECT, "dummyPath", evaluator, SqlLogType.FORMATTED)
+            .build(sqlNode, Function.identity());
+    assertEquals("select * from aaa", sql.getRawSql());
+    assertEquals("select * from aaa", sql.getFormattedSql());
+    assertEquals(0, sql.getParameters().size());
+  }
+
+  public void testFor_removeGroupBy() throws Exception {
+    ExpressionEvaluator evaluator = new ExpressionEvaluator();
+    ArrayList<String> list = new ArrayList<String>();
+    evaluator.add("names", new Value(List.class, list));
+    String testSql =
+        "select * from aaa group by /*%for n : names*/name = /*n*/'a' /*%if n_has_next */, /*%end*//*%end*/";
+    SqlParser parser = new SqlParser(testSql);
+    SqlNode sqlNode = parser.parse();
+    PreparedSql sql =
+        new NodePreparedSqlBuilder(
+                config, SqlKind.SELECT, "dummyPath", evaluator, SqlLogType.FORMATTED)
+            .build(sqlNode, Function.identity());
+    assertEquals("select * from aaa", sql.getRawSql());
+    assertEquals("select * from aaa", sql.getFormattedSql());
+    assertEquals(0, sql.getParameters().size());
+  }
+
   public void testFor_removeOr() throws Exception {
     ExpressionEvaluator evaluator = new ExpressionEvaluator();
     ArrayList<String> list = new ArrayList<String>();