#743 - Fix issue with ElasticSearch nested path query with multiple nested path expressions

This commit is contained in:
Robin Bygrave
2016-06-20 20:57:45 +12:00
parent 6bbf93aba8
commit 3477f590dd
23 changed files with 742 additions and 6 deletions
@@ -18,8 +18,13 @@ public interface SpiExpression extends Expression {
*/
void writeDocQuery(DocQueryContext context) throws IOException;
/**
* Process "Many" properties populating ManyWhereJoins.
/**
* Return the nested path for this expression.
*/
String nestedPath(BeanDescriptor<?> desc);
/**
* Process "Many" properties populating ManyWhereJoins.
* <p>
* Predicates on Many properties require an extra independent join clause.
* </p>
@@ -301,6 +301,11 @@ public interface SpiQuery<T> extends Query<T> {
*/
NaturalKeyBindParam getNaturalKeyBindParam();
/**
* Prepare the query for docstore execution with nested paths.
*/
void prepareDocNested();
/**
* Set the query to be a delete query.
*/
@@ -8,6 +8,7 @@ import com.avaje.ebeaninternal.api.SpiExpressionValidation;
import com.avaje.ebeaninternal.server.deploy.BeanDescriptor;
import com.avaje.ebeaninternal.server.el.ElPropertyDeploy;
import com.avaje.ebeaninternal.server.el.ElPropertyValue;
import com.avaje.ebeaninternal.server.query.SplitName;
/**
* Base class for simple expressions.
@@ -25,6 +26,21 @@ public abstract class AbstractExpression implements SpiExpression {
return this;
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
return propertyNestedPath(propName, desc);
}
protected String propertyNestedPath(String propertyName, BeanDescriptor<?> desc) {
if (propertyName != null) {
ElPropertyDeploy elProp = desc.getElPropertyDeploy(propertyName);
if (elProp != null && elProp.containsMany()) {
return SplitName.begin(propName);
}
}
return null;
}
@Override
public void containsMany(BeanDescriptor<?> desc, ManyWhereJoins manyWhereJoin) {
@@ -30,6 +30,11 @@ class AllEqualsExpression extends NonPrepareExpression {
context.writeAllEquals(propMap);
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
return null;
}
@Override
public void containsMany(BeanDescriptor<?> desc, ManyWhereJoins manyWhereJoin) {
if (propMap != null) {
@@ -7,6 +7,7 @@ import com.avaje.ebeaninternal.api.SpiExpressionRequest;
import com.avaje.ebeaninternal.api.SpiExpressionValidation;
import com.avaje.ebeaninternal.server.deploy.BeanDescriptor;
import com.avaje.ebeaninternal.server.el.ElPropertyDeploy;
import com.avaje.ebeaninternal.server.query.SplitName;
import java.io.IOException;
@@ -39,6 +40,16 @@ class BetweenPropertyExpression extends NonPrepareExpression {
context.endBool();
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
ElPropertyDeploy elProp = desc.getElPropertyDeploy(name(lowProperty));
if (elProp != null && elProp.containsMany()) {
// assumes highProperty is also nested property which seems reasonable
return SplitName.begin(lowProperty);
}
return null;
}
@Override
public void containsMany(BeanDescriptor<?> desc, ManyWhereJoins manyWhereJoin) {
@@ -109,6 +109,11 @@ public class DefaultExampleExpression implements SpiExpression, ExampleExpressio
return new DefaultExampleExpression(list);
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
return null;
}
@Override
public void containsMany(BeanDescriptor<?> desc, ManyWhereJoins whereManyJoins) {
list = buildExpressions(desc);
@@ -31,7 +31,7 @@ public class DefaultExpressionList<T> implements SpiExpressionList<T> {
private static final String AND = " and ";
protected final List<SpiExpression> list;
protected List<SpiExpression> list;
protected final Query<T> query;
@@ -39,6 +39,8 @@ public class DefaultExpressionList<T> implements SpiExpressionList<T> {
protected transient ExpressionFactory expr;
protected String allDocNestedPath;
/**
* Set to true for the "Text" root expression list.
*/
@@ -75,6 +77,26 @@ public class DefaultExpressionList<T> implements SpiExpressionList<T> {
this(null, null, null, new ArrayList<SpiExpression>());
}
/**
* Wrap the expression list as a Junction or top level DefaultExpressionList.
*
* @param list The list of expressions grouped by nested path
* @param nestedPath The doc store nested path
* @param type The junction type (or null for top level expression list).
* @return A single SpiExpression that has the nestedPath set
*/
SpiExpression wrap(List<SpiExpression> list, String nestedPath, Junction.Type type) {
DefaultExpressionList<T> wrapper = new DefaultExpressionList<T>(query, expr, null, list, false);
wrapper.setAllDocNested(nestedPath);
if (type != null) {
return new JunctionExpression<T>(type, wrapper);
} else {
return wrapper;
}
}
/**
* Write being aware if it is the Top level "text" expressions.
* <p>
@@ -94,6 +116,7 @@ public class DefaultExpressionList<T> implements SpiExpressionList<T> {
// this is a Top level "text" expressions so we may need to wrap in Bool SHOULD etc.
if (list.isEmpty()) throw new IllegalStateException("empty expression list?");
if (allDocNestedPath!=null) context.startNested(allDocNestedPath);
int size = list.size();
SpiExpression first = list.get(0);
@@ -124,11 +147,13 @@ public class DefaultExpressionList<T> implements SpiExpressionList<T> {
if (implicitBool || explicitBool) {
context.endBoolGroup();
}
if (allDocNestedPath!=null) context.endNested();
}
}
public void writeDocQuery(DocQueryContext context, SpiExpression idEquals) throws IOException {
if (allDocNestedPath!=null) context.startNested(allDocNestedPath);
int size = list.size();
if (size == 1 && idEquals == null) {
// only 1 expression - skip bool
@@ -147,6 +172,7 @@ public class DefaultExpressionList<T> implements SpiExpressionList<T> {
}
context.endBool();
}
if (allDocNestedPath!=null) context.endNested();
}
@Override
@@ -975,4 +1001,30 @@ public class DefaultExpressionList<T> implements SpiExpressionList<T> {
return junction(Junction.Type.MUST_NOT);
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
// effectively handled by JunctionExpression
return null;
}
/**
* Set the nested path that all contained expressions share.
*/
public void setAllDocNested(String allDocNestedPath) {
this.allDocNestedPath = allDocNestedPath;
}
/**
* Replace the underlying expression list with one organised by nested path.
*/
public void setUnderlying(List<SpiExpression> groupedByNesting) {
this.list = groupedByNesting;
}
/**
* Prepare expressions for document store nested path handling.
*/
public void prepareDocNested(BeanDescriptor<T> beanDescriptor) {
PrepareDocNested.prepare(this, beanDescriptor);
}
}
@@ -147,4 +147,14 @@ public interface DocQueryContext {
* Return the expression path for the given property path.
*/
ExpressionPath getExpressionPath(String propName);
/**
* Start nested path expressions.
*/
void startNested(String nestedPath) throws IOException;
/**
* End nested path expressions.
*/
void endNested() throws IOException;
}
@@ -118,6 +118,11 @@ class ExistsQueryExpression implements SpiExpression, UnsupportedDocStoreExpress
return true;
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
return null;
}
@Override
public void containsMany(BeanDescriptor<?> desc, ManyWhereJoins whereManyJoins) {
// Nothing to do for exists expression
@@ -25,6 +25,11 @@ class IdExpression extends NonPrepareExpression implements SpiExpression {
context.writeId(value);
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
return null;
}
/**
* Always returns false.
*/
@@ -22,6 +22,11 @@ public class IdInExpression extends NonPrepareExpression {
this.idList = idList;
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
return null;
}
@Override
public void containsMany(BeanDescriptor<?> desc, ManyWhereJoins manyWhereJoin) {
}
@@ -39,7 +39,7 @@ import java.util.Set;
*/
class JunctionExpression<T> implements SpiJunction<T>, SpiExpression, ExpressionList<T> {
private final DefaultExpressionList<T> exprList;
protected final DefaultExpressionList<T> exprList;
protected final Junction.Type type;
@@ -790,4 +790,17 @@ class JunctionExpression<T> implements SpiJunction<T>, SpiExpression, Expression
public ExpressionList<T> endNot() {
return endJunction();
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
PrepareDocNested.prepare(exprList, desc, type);
String nestedPath = exprList.allDocNestedPath;
if (nestedPath != null) {
// push the nestedPath up to parent
exprList.setAllDocNested(null);
return nestedPath;
}
return null;
}
}
@@ -45,9 +45,9 @@ abstract class LogicExpression implements SpiExpression {
}
}
protected final SpiExpression expOne;
protected SpiExpression expOne;
protected final SpiExpression expTwo;
protected SpiExpression expTwo;
private final String joinType;
@@ -67,6 +67,27 @@ abstract class LogicExpression implements SpiExpression {
context.endBool();
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
String pathOne = expOne.nestedPath(desc);
String pathTwo = expTwo.nestedPath(desc);
if (pathOne == null && pathTwo == null) {
return null;
}
if (pathOne != null && pathOne.equals(pathTwo)) {
return pathOne;
}
if (pathOne != null) {
expOne = new NestedPathWrapperExpression(pathOne, expOne);
}
if (pathTwo != null) {
expTwo = new NestedPathWrapperExpression(pathTwo, expTwo);
}
return null;
}
@Override
public void containsMany(BeanDescriptor<?> desc, ManyWhereJoins manyWhereJoin) {
expOne.containsMany(desc, manyWhereJoin);
@@ -0,0 +1,93 @@
package com.avaje.ebeaninternal.server.expression;
import com.avaje.ebean.event.BeanQueryRequest;
import com.avaje.ebeaninternal.api.HashQueryPlanBuilder;
import com.avaje.ebeaninternal.api.ManyWhereJoins;
import com.avaje.ebeaninternal.api.SpiExpression;
import com.avaje.ebeaninternal.api.SpiExpressionRequest;
import com.avaje.ebeaninternal.api.SpiExpressionValidation;
import com.avaje.ebeaninternal.server.deploy.BeanDescriptor;
import java.io.IOException;
/**
* Wraps a single expression with nestedPath for document queries.
*/
class NestedPathWrapperExpression implements SpiExpression {
protected final String nestedPath;
protected final SpiExpression delegate;
NestedPathWrapperExpression(String nestedPath, SpiExpression delegate) {
this.nestedPath = nestedPath;
this.delegate = delegate;
}
@Override
public void writeDocQuery(DocQueryContext context) throws IOException {
context.startNested(nestedPath);
delegate.writeDocQuery(context);
context.endNested();
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
return nestedPath;
}
@Override
public void containsMany(BeanDescriptor<?> desc, ManyWhereJoins whereManyJoins) {
delegate.containsMany(desc, whereManyJoins);
}
@Override
public void prepareExpression(BeanQueryRequest<?> request) {
delegate.prepareExpression(request);
}
@Override
public void queryPlanHash(HashQueryPlanBuilder builder) {
delegate.queryPlanHash(builder);
}
@Override
public int queryBindHash() {
return delegate.queryBindHash();
}
@Override
public boolean isSameByPlan(SpiExpression other) {
if (other instanceof NestedPathWrapperExpression) {
NestedPathWrapperExpression that = (NestedPathWrapperExpression)other;
return nestedPath.equals(that.nestedPath)
&& delegate.isSameByPlan(that.delegate);
}
return false;
}
@Override
public boolean isSameByBind(SpiExpression other) {
return delegate.isSameByBind(other);
}
@Override
public void addSql(SpiExpressionRequest request) {
delegate.addSql(request);
}
@Override
public void addBindValues(SpiExpressionRequest request) {
delegate.addBindValues(request);
}
@Override
public void validate(SpiExpressionValidation validation) {
delegate.validate(validation);
}
@Override
public SpiExpression copyForPlanKey() {
return new NestedPathWrapperExpression(nestedPath, delegate.copyForPlanKey());
}
}
@@ -26,6 +26,11 @@ class NoopExpression implements SpiExpression {
public void writeDocQuery(DocQueryContext context) throws IOException {
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
return null;
}
@Override
public void containsMany(BeanDescriptor<?> desc, ManyWhereJoins whereManyJoins) {
// nothing to do
@@ -34,6 +34,11 @@ final class NotExpression implements SpiExpression {
return new NotExpression(exp.copyForPlanKey());
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
return exp.nestedPath(desc);
}
@Override
public void containsMany(BeanDescriptor<?> desc, ManyWhereJoins manyWhereJoin) {
exp.containsMany(desc, manyWhereJoin);
@@ -0,0 +1,163 @@
package com.avaje.ebeaninternal.server.expression;
import com.avaje.ebean.Junction;
import com.avaje.ebeaninternal.api.SpiExpression;
import com.avaje.ebeaninternal.server.deploy.BeanDescriptor;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
/**
* Prepare nested path expressions for
*/
class PrepareDocNested {
/**
* Prepare the top level expressions for nested path handling.
*/
static void prepare(DefaultExpressionList<?> expressions, BeanDescriptor<?> beanDescriptor) {
new PrepareDocNested(expressions, beanDescriptor, null).process();
}
/**
* Prepare the Junction expressions for nested path handling.
*/
static void prepare(DefaultExpressionList<?> expressions, BeanDescriptor<?> beanDescriptor, Junction.Type type) {
new PrepareDocNested(expressions, beanDescriptor, type).process();
}
enum Mode {
NONE,
SINGLE,
MIXED
}
private final Junction.Type type;
private final DefaultExpressionList<?> original;
private final BeanDescriptor<?> beanDescriptor;
private final List<SpiExpression> origUnderlying;
private final int origSize;
private boolean hasNesting;
private boolean hasMixedNesting;
private String firstNestedPath;
PrepareDocNested(DefaultExpressionList<?> original, BeanDescriptor<?> beanDescriptor, Junction.Type type) {
this.type = type;
this.beanDescriptor = beanDescriptor;
this.original = original;
this.origUnderlying = original.getUnderlyingList();
this.origSize = origUnderlying.size();
}
void process() {
PrepareDocNested.Mode mode = determineMode();
if (mode == PrepareDocNested.Mode.SINGLE) {
original.setAllDocNested(firstNestedPath);
} else if (mode == PrepareDocNested.Mode.MIXED) {
original.setUnderlying(group());
}
}
/**
* Reorganise the flat list of expressions into a tree grouping expressions by nested path.
*
* Returns the new top level list of expressions.
*/
private List<SpiExpression> group() {
Map<String,Group> groups = new LinkedHashMap<String,Group>();
// organise expressions by nestedPath
for (int i = 0; i < origSize; i++) {
SpiExpression expr = origUnderlying.get(i);
String nestedPath = expr.nestedPath(beanDescriptor);
Group group = groups.get(nestedPath);
if (group == null) {
group = new Group(nestedPath);
groups.put(nestedPath, group);
}
group.list.add(expr);
}
List<SpiExpression> newList = new ArrayList<SpiExpression>();
Collection<Group> values = groups.values();
for (Group group : values) {
group.addTo(newList);
}
return newList;
}
/**
* Determined the nested path mode.
*/
private Mode determineMode() {
if (!hasNesting()) {
// no nested paths at all
return Mode.NONE;
}
if (!hasMixedNesting) {
// single nested path for all expressions
return Mode.SINGLE;
}
// mixed nested paths to underlying expression list needs re-organising by nested path
return Mode.MIXED;
}
/**
* Return true if the expressions have nested paths.
*/
private boolean hasNesting() {
for (int i = 0; i < origSize; i++) {
SpiExpression expr = origUnderlying.get(i);
String nestedPath = expr.nestedPath(beanDescriptor);
if (nestedPath == null) {
hasMixedNesting = true;
} if (nestedPath != null) {
hasNesting = true;
if (firstNestedPath == null) {
firstNestedPath = nestedPath;
} else if (hasMixedNesting || !firstNestedPath.equals(nestedPath)) {
hasMixedNesting = true;
return true;
}
}
}
return hasNesting;
}
/**
* List of SpiExpression grouped by nested path.
*/
class Group {
final String nestedPath;
final List<SpiExpression> list = new ArrayList<SpiExpression>();
Group(String nestedPath) {
this.nestedPath = nestedPath;
}
void addTo(List<SpiExpression> newList) {
if (nestedPath == null) {
newList.addAll(list);
} else {
newList.add(original.wrap(list, nestedPath, type));
}
}
}
}
@@ -25,6 +25,11 @@ class RawExpression extends NonPrepareExpression {
context.writeRaw(sql, values);
}
@Override
public String nestedPath(BeanDescriptor<?> desc) {
return null;
}
@Override
public void containsMany(BeanDescriptor<?> desc, ManyWhereJoins manyWhereJoin) {
@@ -1,9 +1,15 @@
package com.avaje.ebeaninternal.server.query;
/**
* Helper for dot notation property paths.
*/
public class SplitName {
private static final char PERIOD = '.';
/**
* Add the two name sections together in dot notation.
*/
public static String add(String prefix, String name) {
if (prefix != null) {
return prefix + "." + name;
@@ -38,10 +44,20 @@ public class SplitName {
}
}
/**
* Return the name split by last.
*/
public static String[] split(String name) {
return split(name, true);
}
/**
* Return the first part of the name.
*/
public static String begin(String name) {
return splitBegin(name)[0];
}
public static String[] splitBegin(String name) {
return split(name, false);
}
@@ -484,6 +484,16 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
}
}
@Override
public void prepareDocNested() {
if (textExpressions != null) {
textExpressions.prepareDocNested(beanDescriptor);
}
if (whereExpressions != null) {
whereExpressions.prepareDocNested(beanDescriptor);
}
}
/**
* Setup to be a delete query.
*/