Skip to content

Commit

Permalink
GH-1405 fixed scope handling in join iterator (#1616)
Browse files Browse the repository at this point in the history
* GH-1405 fixed scope handling in join iterator

use merge-join for group patterns to handle scope, with special case
exception for FILTER (NOT) EXISTS

* GH-1405 only add scoping braces if arg is a group graph pattern

* GH-1405 fix NosuchElementException in MergeIteration and skip for all
filters
  • Loading branch information
abrokenjester authored Oct 22, 2019
1 parent c6baf63 commit 91cb1cd
Show file tree
Hide file tree
Showing 8 changed files with 264 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,19 @@

import org.eclipse.rdf4j.common.iteration.CloseableIteration;
import org.eclipse.rdf4j.common.iteration.EmptyIteration;
import org.eclipse.rdf4j.common.iteration.FilterIteration;
import org.eclipse.rdf4j.common.iteration.LookAheadIteration;
import org.eclipse.rdf4j.model.Value;
import org.eclipse.rdf4j.query.Binding;
import org.eclipse.rdf4j.query.BindingSet;
import org.eclipse.rdf4j.query.QueryEvaluationException;
import org.eclipse.rdf4j.query.algebra.Filter;
import org.eclipse.rdf4j.query.algebra.Join;
import org.eclipse.rdf4j.query.algebra.TupleExpr;
import org.eclipse.rdf4j.query.algebra.evaluation.EvaluationStrategy;
import org.eclipse.rdf4j.query.algebra.evaluation.QueryBindingSet;
import org.eclipse.rdf4j.query.algebra.helpers.TupleExprs;
import org.eclipse.rdf4j.query.impl.EmptyBindingSet;

public class JoinIterator extends LookAheadIteration<BindingSet, QueryEvaluationException> {

Expand Down Expand Up @@ -61,7 +69,15 @@ protected BindingSet getNextElement() throws QueryEvaluationException {
rightIter.close();

if (leftIter.hasNext()) {
rightIter = strategy.evaluate(join.getRightArg(), leftIter.next());
TupleExpr rightArg = join.getRightArg();
if (TupleExprs.isGraphPatternGroup(rightArg) && !(rightArg instanceof Filter)) {
// leftiter bindings are out of scope for the right arg, so we merge afterward.
BindingSet next = leftIter.next();
rightIter = new MergeIteration(next, new BindingSetFilterIteration(next,
strategy.evaluate(rightArg, new EmptyBindingSet())));
} else {
rightIter = strategy.evaluate(rightArg, leftIter.next());
}
}
}
} catch (NoSuchElementException ignore) {
Expand All @@ -84,4 +100,70 @@ protected void handleClose() throws QueryEvaluationException {
}
}
}

private class MergeIteration extends LookAheadIteration<BindingSet, QueryEvaluationException> {

private BindingSet bindingSet;
private CloseableIteration<BindingSet, QueryEvaluationException> iter;

public MergeIteration(BindingSet mergeBS, CloseableIteration<BindingSet, QueryEvaluationException> iter) {
this.bindingSet = mergeBS;
this.iter = iter;

}

/**
* Merge each sequence from the wrapped iterator with the provided BindingSet
*/
@Override
protected BindingSet getNextElement() throws QueryEvaluationException {
if (!iter.hasNext()) {
return null;
}

BindingSet bs = iter.next();
QueryBindingSet result = new QueryBindingSet(bs);
for (Binding b : bindingSet) {
if (!result.hasBinding(b.getName())) {
result.addBinding(b);
}
}
return result;
}

@Override
protected void handleClose() {
super.handleClose();
iter.close();
}

}

private class BindingSetFilterIteration extends FilterIteration<BindingSet, QueryEvaluationException> {

private BindingSet bindingSet;

public BindingSetFilterIteration(BindingSet bindingSet,
CloseableIteration<BindingSet, QueryEvaluationException> iteration) {
super(iteration);
this.bindingSet = bindingSet;
}

/**
* Filter out sequences where any bindings conflict with bindings in the provided bindingSet
*/
@Override
protected boolean accept(BindingSet toBeFiltered) throws QueryEvaluationException {
for (Binding b : bindingSet) {
Value v = b.getValue();
String name = b.getName();
if (toBeFiltered.hasBinding(name)) {
if (!toBeFiltered.getValue(name).equals(v)) {
return false;
}
}
}
return true;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
@Deprecated
@InternalUseOnly
/**
* Implementations of {@link Iteration} relevant to query evaluation.
*
* @deprecated since 3.0. This feature is for internal use only: its existence, signature or behavior may change without
* warning from one release to the next.
*/
package org.eclipse.rdf4j.query.algebra.evaluation.iterator;

import org.eclipse.rdf4j.common.annotation.InternalUseOnly;
27 changes: 20 additions & 7 deletions core/queryalgebra/model/pom.xml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">

<modelVersion>4.0.0</modelVersion>

Expand Down Expand Up @@ -29,13 +31,24 @@
<artifactId>rdf4j-util</artifactId>
<version>${project.version}</version>
</dependency>

<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
<build>
<plugins>
<plugin>
<groupId>com.github.siom79.japicmp</groupId>
<artifactId>japicmp-maven-plugin</artifactId>
</plugin>
<build>
<plugins>
<plugin>
<groupId>com.github.siom79.japicmp</groupId>
<artifactId>japicmp-maven-plugin</artifactId>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ public abstract class UnaryTupleOperator extends AbstractQueryModelNode implemen
*/
protected TupleExpr arg;

private boolean isGraphPatternGroup;

/*--------------*
* Constructors *
*--------------*/
Expand All @@ -45,16 +43,6 @@ protected UnaryTupleOperator(TupleExpr arg) {
* Methods *
*---------*/

@Override
public boolean isGraphPatternGroup() {
return isGraphPatternGroup;
}

@Override
public void setGraphPatternGroup(boolean isGraphPatternGroup) {
this.isGraphPatternGroup = isGraphPatternGroup;
}

/**
* Gets the argument of this unary tuple operator.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@
import org.eclipse.rdf4j.model.BNode;
import org.eclipse.rdf4j.model.Literal;
import org.eclipse.rdf4j.model.Value;
import org.eclipse.rdf4j.query.algebra.Exists;
import org.eclipse.rdf4j.query.algebra.Filter;
import org.eclipse.rdf4j.query.algebra.GraphPatternGroupable;
import org.eclipse.rdf4j.query.algebra.Join;
import org.eclipse.rdf4j.query.algebra.Not;
import org.eclipse.rdf4j.query.algebra.Projection;
import org.eclipse.rdf4j.query.algebra.QueryModelNode;
import org.eclipse.rdf4j.query.algebra.TupleExpr;
Expand Down Expand Up @@ -162,4 +165,23 @@ public static String getConstVarName(Value value) {

return "_const_" + uniqueStringForValue;
}

/**
* Verifies if the supplied expression is a FILTER (NOT) EXISTS operation
*
* @param rightArg a tuple expression
* @return true if the supplied expression is a FILTER (NOT) EXISTS operation, false otherwise.
*/
public static boolean isFilterExistsFunction(TupleExpr expr) {
if (expr instanceof Filter) {
Filter filter = (Filter) expr;
if (filter.getCondition() instanceof Exists) {
return true;
} else if (filter.getCondition() instanceof Not) {
Not n = (Not) filter.getCondition();
return (n.getArg() instanceof Exists);
}
}
return false;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*******************************************************************************
* Copyright (c) 2019 Eclipse RDF4J contributors.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Distribution License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/org/documents/edl-v10.php.
*******************************************************************************/
package org.eclipse.rdf4j.query.algebra.helpers;

import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.rdf4j.query.algebra.helpers.TupleExprs.isFilterExistsFunction;

import org.eclipse.rdf4j.model.ValueFactory;
import org.eclipse.rdf4j.model.impl.SimpleValueFactory;
import org.eclipse.rdf4j.query.algebra.Compare;
import org.eclipse.rdf4j.query.algebra.Exists;
import org.eclipse.rdf4j.query.algebra.Filter;
import org.eclipse.rdf4j.query.algebra.Not;
import org.eclipse.rdf4j.query.algebra.StatementPattern;
import org.eclipse.rdf4j.query.algebra.TupleExpr;
import org.eclipse.rdf4j.query.algebra.Var;
import org.junit.Test;

public class TupleExprsTest {

private final ValueFactory f = SimpleValueFactory.getInstance();

@Test
public void isFilterExistsFunctionOnEmptyFilter() {
TupleExpr expr = new Filter();

assertThat(isFilterExistsFunction(expr)).isFalse();
}

@Test
public void isFilterExistsFunctionOnNormalFilter() {
Filter expr = new Filter();
expr.setArg(new StatementPattern());
expr.setCondition(new Compare(new Var("x", f.createBNode()), new Var("y", f.createBNode())));

assertThat(isFilterExistsFunction(expr)).isFalse();
}

@Test
public void isFilterExistsFunctionOnNormalNot() {
Filter expr = new Filter();
expr.setArg(new StatementPattern());
expr.setCondition(new Not(new Compare(new Var("x", f.createBNode()), new Var("y", f.createBNode()))));

assertThat(isFilterExistsFunction(expr)).isFalse();
}

@Test
public void isFilterExistsFunctionOnExists() {
Filter expr = new Filter();
expr.setArg(new StatementPattern());
expr.setCondition(new Exists(new StatementPattern()));

assertThat(isFilterExistsFunction(expr)).isTrue();

}

@Test
public void isFilterExistsFunctionOnNotExist() {
Filter expr = new Filter();
expr.setArg(new StatementPattern());
expr.setCondition(new Not(new Exists(new StatementPattern())));

assertThat(isFilterExistsFunction(expr)).isTrue();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.eclipse.rdf4j.query.algebra.Var;
import org.eclipse.rdf4j.query.algebra.evaluation.QueryOptimizer;
import org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor;
import org.eclipse.rdf4j.query.algebra.helpers.TupleExprs;
import org.eclipse.rdf4j.repository.RepositoryException;
import org.eclipse.rdf4j.rio.turtle.TurtleUtil;
import org.eclipse.rdf4j.sail.federation.algebra.NaryJoin;
Expand All @@ -47,8 +48,6 @@
*/
public class PrepareOwnedTupleExpr extends AbstractQueryModelVisitor<RepositoryException> implements QueryOptimizer {

private static final String END_BLOCK = "}\n";

private OwnedTupleExpr owner;

private String pattern;
Expand Down Expand Up @@ -238,7 +237,7 @@ public void meet(LeftJoin node) throws RepositoryException {
vars.putAll(variables);
node.getRightArg().visit(this);
if (patternNode != null) {
builder.append("OPTIONAL {").append(pattern).append(END_BLOCK);
builder.append("OPTIONAL {").append(pattern).append("}\n");
vars.putAll(variables);
this.variables = vars;
this.pattern = builder.toString();
Expand All @@ -261,7 +260,14 @@ public void meetMultiJoin(NaryJoin node) throws RepositoryException {
// no owner
builder = null; // NOPMD
} else if (builder != null) {
builder.append("{").append(pattern).append(END_BLOCK);
if (TupleExprs.isGraphPatternGroup(arg)) {
builder.append("{");
}
builder.append(pattern);
if (TupleExprs.isGraphPatternGroup(arg)) {
builder.append("}");
}
builder.append("\n");
vars.putAll(variables);
}
}
Expand All @@ -276,13 +282,33 @@ public void meetMultiJoin(NaryJoin node) throws RepositoryException {
public void meet(Join node) throws RepositoryException {
Map<String, String> vars = new HashMap<>();
StringBuilder builder = new StringBuilder();
node.getLeftArg().visit(this);

TupleExpr leftArg = node.getLeftArg();
TupleExpr rightArg = node.getRightArg();

leftArg.visit(this);
if (patternNode != null) {
builder.append("{").append(pattern).append(END_BLOCK);
if (TupleExprs.isGraphPatternGroup(leftArg)) {
builder.append("{");
}
builder.append(pattern);
if (TupleExprs.isGraphPatternGroup(leftArg)) {
builder.append("}");
}
builder.append("\n");

vars.putAll(variables);
node.getRightArg().visit(this);
rightArg.visit(this);
if (patternNode != null) {
builder.append("{").append(pattern).append(END_BLOCK);
if (TupleExprs.isGraphPatternGroup(rightArg)) {
builder.append("{");
}
builder.append(pattern);
if (TupleExprs.isGraphPatternGroup(rightArg)) {
builder.append("}");
}
builder.append("\n");

vars.putAll(variables);
this.variables = vars;
this.pattern = builder.toString();
Expand Down
Loading

0 comments on commit 91cb1cd

Please sign in to comment.