diff --git a/src/main/java/com/avaje/ebean/Junction.java b/src/main/java/com/avaje/ebean/Junction.java index f7b233998..35ec6aa90 100644 --- a/src/main/java/com/avaje/ebean/Junction.java +++ b/src/main/java/com/avaje/ebean/Junction.java @@ -59,11 +59,11 @@ package com.avaje.ebean; * .and() * .startsWith("name", "r") * .eq("anniversary", onAfter) - * .endJunction() + * .endAnd() * .and() * .eq("status", Customer.Status.ACTIVE) * .gt("id", 0) - * .endJunction() + * .endAnd() * .order().asc("name"); * * q.findList(); @@ -84,39 +84,41 @@ public interface Junction extends Expression, ExpressionList { /** * AND group. */ - AND(" and ", ""), + AND(" and ", "", false), /** * OR group. */ - OR(" or ", ""), + OR(" or ", "", false), /** * NOT group. */ - NOT(" and ", "not "), + NOT(" and ", "not ", false), /** * Text search AND group. */ - MUST("must", ""), + MUST("must", "", true), /** * Text search NOT group. */ - MUST_NOT("must_not", ""), + MUST_NOT("must_not", "", true), /** * Text search OR group. */ - SHOULD("should", ""); + SHOULD("should", "", true); - String prefix; - String literal; + private String prefix; + private String literal; + private boolean text; - Type(String literal, String prefix) { + Type(String literal, String prefix, boolean text) { this.literal = literal; this.prefix = prefix; + this.text = text; } /** @@ -132,6 +134,14 @@ public interface Junction extends Expression, ExpressionList { public String prefix() { return prefix; } + + /** + * Return true if this is a text type. + */ + public boolean isText() { + return text; + } + } } diff --git a/src/main/java/com/avaje/ebeaninternal/api/SpiExpression.java b/src/main/java/com/avaje/ebeaninternal/api/SpiExpression.java index c96ad7332..0c996e741 100644 --- a/src/main/java/com/avaje/ebeaninternal/api/SpiExpression.java +++ b/src/main/java/com/avaje/ebeaninternal/api/SpiExpression.java @@ -13,7 +13,12 @@ import java.io.IOException; */ public interface SpiExpression extends Expression { - /** + /** + * Simplify nested expressions if possible. + */ + void simplify(); + + /** * Write the expression as an elastic search expression. */ void writeDocQuery(DocQueryContext context) throws IOException; diff --git a/src/main/java/com/avaje/ebeaninternal/api/SpiQuery.java b/src/main/java/com/avaje/ebeaninternal/api/SpiQuery.java index feb9cfb92..dec547896 100644 --- a/src/main/java/com/avaje/ebeaninternal/api/SpiQuery.java +++ b/src/main/java/com/avaje/ebeaninternal/api/SpiQuery.java @@ -693,4 +693,8 @@ public interface SpiQuery extends Query { */ OrmUpdateProperties getUpdateProperties(); + /** + * Simplify nested expression lists where possible. + */ + void simplifyExpressions(); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/expression/AbstractExpression.java b/src/main/java/com/avaje/ebeaninternal/server/expression/AbstractExpression.java index 8b156c92b..5b9569496 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/expression/AbstractExpression.java +++ b/src/main/java/com/avaje/ebeaninternal/server/expression/AbstractExpression.java @@ -21,6 +21,11 @@ public abstract class AbstractExpression implements SpiExpression { this.propName = propName; } + @Override + public void simplify() { + // do nothing + } + @Override public Object getIdEqualTo(String idName) { // override on SimpleExpression diff --git a/src/main/java/com/avaje/ebeaninternal/server/expression/DefaultExampleExpression.java b/src/main/java/com/avaje/ebeaninternal/server/expression/DefaultExampleExpression.java index 3033e4c09..f8491c300 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/expression/DefaultExampleExpression.java +++ b/src/main/java/com/avaje/ebeaninternal/server/expression/DefaultExampleExpression.java @@ -88,6 +88,11 @@ public class DefaultExampleExpression implements SpiExpression, ExampleExpressio } } + @Override + public void simplify() { + // do nothing + } + @Override public void writeDocQuery(DocQueryContext context) throws IOException { if (!list.isEmpty()) { diff --git a/src/main/java/com/avaje/ebeaninternal/server/expression/DefaultExpressionList.java b/src/main/java/com/avaje/ebeaninternal/server/expression/DefaultExpressionList.java index 2d6f3e033..a6cd1001d 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/expression/DefaultExpressionList.java +++ b/src/main/java/com/avaje/ebeaninternal/server/expression/DefaultExpressionList.java @@ -97,6 +97,16 @@ public class DefaultExpressionList implements SpiExpressionList { } } + void simplifyEntries() { + for (SpiExpression element : list) { + element.simplify(); + } + } + + public void simplify() { + simplifyEntries(); + } + /** * Write being aware if it is the Top level "text" expressions. *

diff --git a/src/main/java/com/avaje/ebeaninternal/server/expression/ExistsQueryExpression.java b/src/main/java/com/avaje/ebeaninternal/server/expression/ExistsQueryExpression.java index fdc8467a2..aa1abcfe7 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/expression/ExistsQueryExpression.java +++ b/src/main/java/com/avaje/ebeaninternal/server/expression/ExistsQueryExpression.java @@ -36,6 +36,11 @@ class ExistsQueryExpression implements SpiExpression, UnsupportedDocStoreExpress this.subQuery = null; } + @Override + public void simplify() { + // do nothing + } + @Override public void writeDocQuery(DocQueryContext context) throws IOException { throw new IllegalStateException("Not supported"); diff --git a/src/main/java/com/avaje/ebeaninternal/server/expression/InQueryExpression.java b/src/main/java/com/avaje/ebeaninternal/server/expression/InQueryExpression.java index 7768da6cf..ba1bd8857 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/expression/InQueryExpression.java +++ b/src/main/java/com/avaje/ebeaninternal/server/expression/InQueryExpression.java @@ -38,6 +38,11 @@ class InQueryExpression extends AbstractExpression implements UnsupportedDocStor this.bindParams = bindParams; } + @Override + public void simplify() { + // do nothing + } + @Override public void writeDocQuery(DocQueryContext context) throws IOException { throw new IllegalStateException("Not supported"); diff --git a/src/main/java/com/avaje/ebeaninternal/server/expression/JunctionExpression.java b/src/main/java/com/avaje/ebeaninternal/server/expression/JunctionExpression.java index 3736665cc..6b49e2492 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/expression/JunctionExpression.java +++ b/src/main/java/com/avaje/ebeaninternal/server/expression/JunctionExpression.java @@ -39,9 +39,9 @@ import java.util.Set; */ class JunctionExpression implements SpiJunction, SpiExpression, ExpressionList { - protected final DefaultExpressionList exprList; + protected DefaultExpressionList exprList; - protected final Junction.Type type; + protected Junction.Type type; JunctionExpression(Junction.Type type, Query query, ExpressionList parent) { this.type = type; @@ -56,6 +56,31 @@ class JunctionExpression implements SpiJunction, SpiExpression, Expression this.exprList = exprList; } + /** + * Simplify nested expressions where possible. + *

+ * This is expected to only used after expressions are built via query language parsing. + *

+ */ + public void simplify() { + exprList.simplifyEntries(); + + List list = exprList.list; + if (list.size() == 1 && list.get(0) instanceof JunctionExpression) { + JunctionExpression nested = (JunctionExpression)list.get(0); + if (type == Type.AND && !nested.type.isText()) { + // and (and (a, b, c)) -> and (a, b, c) + // and (not (a, b, c)) -> not (a, b, c) + // and (or (a, b, c)) -> or (a, b, c) + this.exprList = nested.exprList; + this.type = nested.type; + } else if (type == Type.NOT && nested.type == Type.AND) { + // not (and (a, b, c)) -> not (a, b, c) + this.exprList = nested.exprList; + } + } + } + public SpiExpression copyForPlanKey() { return new JunctionExpression(type, exprList.copyForPlanKey()); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/expression/LogicExpression.java b/src/main/java/com/avaje/ebeaninternal/server/expression/LogicExpression.java index 86074d3f8..44281d369 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/expression/LogicExpression.java +++ b/src/main/java/com/avaje/ebeaninternal/server/expression/LogicExpression.java @@ -57,6 +57,11 @@ abstract class LogicExpression implements SpiExpression { this.expTwo = (SpiExpression) expTwo; } + @Override + public void simplify() { + // do nothing + } + @Override public void writeDocQuery(DocQueryContext context) throws IOException { diff --git a/src/main/java/com/avaje/ebeaninternal/server/expression/NestedPathWrapperExpression.java b/src/main/java/com/avaje/ebeaninternal/server/expression/NestedPathWrapperExpression.java index 2f67b008a..6f4434486 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/expression/NestedPathWrapperExpression.java +++ b/src/main/java/com/avaje/ebeaninternal/server/expression/NestedPathWrapperExpression.java @@ -24,6 +24,11 @@ class NestedPathWrapperExpression implements SpiExpression { this.delegate = delegate; } + @Override + public void simplify() { + // do nothing + } + @Override public void writeDocQuery(DocQueryContext context) throws IOException { context.startNested(nestedPath); diff --git a/src/main/java/com/avaje/ebeaninternal/server/expression/NonPrepareExpression.java b/src/main/java/com/avaje/ebeaninternal/server/expression/NonPrepareExpression.java index aa8e33b46..39432b690 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/expression/NonPrepareExpression.java +++ b/src/main/java/com/avaje/ebeaninternal/server/expression/NonPrepareExpression.java @@ -8,6 +8,11 @@ import com.avaje.ebeaninternal.api.SpiExpression; */ abstract class NonPrepareExpression implements SpiExpression { + @Override + public void simplify() { + // do nothing + } + @Override public void prepareExpression(BeanQueryRequest request) { // do nothing diff --git a/src/main/java/com/avaje/ebeaninternal/server/expression/NoopExpression.java b/src/main/java/com/avaje/ebeaninternal/server/expression/NoopExpression.java index 813ccf244..3cbfbc381 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/expression/NoopExpression.java +++ b/src/main/java/com/avaje/ebeaninternal/server/expression/NoopExpression.java @@ -17,6 +17,11 @@ class NoopExpression implements SpiExpression { protected static final NoopExpression INSTANCE = new NoopExpression(); + @Override + public void simplify() { + // do nothing + } + @Override public SpiExpression copyForPlanKey() { return this; diff --git a/src/main/java/com/avaje/ebeaninternal/server/expression/NotExpression.java b/src/main/java/com/avaje/ebeaninternal/server/expression/NotExpression.java index b5945927f..b0e8a00ef 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/expression/NotExpression.java +++ b/src/main/java/com/avaje/ebeaninternal/server/expression/NotExpression.java @@ -22,6 +22,11 @@ final class NotExpression implements SpiExpression { this.exp = (SpiExpression) exp; } + @Override + public void simplify() { + // do nothing + } + @Override public void writeDocQuery(DocQueryContext context) throws IOException { context.startBoolMustNot(); diff --git a/src/main/java/com/avaje/ebeaninternal/server/grammer/EqlAdapter.java b/src/main/java/com/avaje/ebeaninternal/server/grammer/EqlAdapter.java index 706cbabb3..fe04c9073 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/grammer/EqlAdapter.java +++ b/src/main/java/com/avaje/ebeaninternal/server/grammer/EqlAdapter.java @@ -27,8 +27,8 @@ class EqlAdapter extends EQLBaseListener { private boolean textMode; - public EqlAdapter(Query query) { - this.query = (SpiQuery)query; + public EqlAdapter(SpiQuery query) { + this.query = query; this.helper = new EqlAdapterHelper(this); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/grammer/EqlParser.java b/src/main/java/com/avaje/ebeaninternal/server/grammer/EqlParser.java index 980aad011..d5e5754e1 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/grammer/EqlParser.java +++ b/src/main/java/com/avaje/ebeaninternal/server/grammer/EqlParser.java @@ -1,6 +1,6 @@ package com.avaje.ebeaninternal.server.grammer; -import com.avaje.ebean.Query; +import com.avaje.ebeaninternal.api.SpiQuery; import com.avaje.ebeaninternal.server.grammer.antlr.EQLLexer; import com.avaje.ebeaninternal.server.grammer.antlr.EQLParser; import org.antlr.v4.runtime.ANTLRInputStream; @@ -9,7 +9,7 @@ import org.antlr.v4.runtime.tree.ParseTreeWalker; public class EqlParser { - public static void parse(String raw, Query query) { + public static void parse(String raw, SpiQuery query) { EQLLexer lexer = new EQLLexer(new ANTLRInputStream(raw)); CommonTokenStream tokens = new CommonTokenStream(lexer); @@ -20,5 +20,7 @@ public class EqlParser { ParseTreeWalker walker = new ParseTreeWalker(); walker.walk(adapter, context); + + query.simplifyExpressions(); } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java b/src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java index 7a8bd1a69..eb2a8deac 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java +++ b/src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java @@ -1349,6 +1349,13 @@ public class DefaultOrmQuery implements SpiQuery { return whereExpressions; } + @Override + public void simplifyExpressions() { + if (whereExpressions != null) { + whereExpressions.simplify(); + } + } + @Override public DefaultOrmQuery having(Expression expression) { having().add(expression); diff --git a/src/test/java/com/avaje/ebeaninternal/server/grammer/EqlParserTest.java b/src/test/java/com/avaje/ebeaninternal/server/grammer/EqlParserTest.java index 11e9c7d8d..5a246c4d0 100644 --- a/src/test/java/com/avaje/ebeaninternal/server/grammer/EqlParserTest.java +++ b/src/test/java/com/avaje/ebeaninternal/server/grammer/EqlParserTest.java @@ -2,8 +2,8 @@ package com.avaje.ebeaninternal.server.grammer; import com.avaje.ebean.Ebean; import com.avaje.ebean.Query; +import com.avaje.ebeaninternal.api.SpiQuery; import com.avaje.tests.model.basic.Customer; -import org.junit.Ignore; import org.junit.Test; import static org.assertj.core.api.Assertions.assertThat; @@ -75,10 +75,27 @@ public class EqlParserTest { assertThat(query.getGeneratedSql()).contains("where ((t0.name = ? or t0.status = ? ) and t0.smallnote is null )"); } + @Test + public void test_simplifyExpressions() throws Exception { + + Query query = parse("where not (name = 'Rob' and status = 'NEW')"); + query.findList(); + assertThat(query.getGeneratedSql()).contains("where not (t0.name = ? and t0.status = ? )"); + + query = parse("where not ((name = 'Rob' and status = 'NEW'))"); + query.findList(); + assertThat(query.getGeneratedSql()).contains("where not (t0.name = ? and t0.status = ? )"); + + query = parse("where not (((name = 'Rob') and (status = 'NEW')))"); + query.findList(); + assertThat(query.getGeneratedSql()).contains("where not (t0.name = ? and t0.status = ? )"); + } + + private Query parse(String raw) { Query query = Ebean.find(Customer.class); - EqlParser.parse(raw, query); + EqlParser.parse(raw, (SpiQuery)query); return query; }