Skip to content

Commit

Permalink
GH-4769 Variables that are not in scope should be hidden from filters.
Browse files Browse the repository at this point in the history
  • Loading branch information
JervenBolleman committed Nov 15, 2023
1 parent 978cde1 commit ec12d20
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
package org.eclipse.rdf4j.query.algebra.evaluation.impl;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import java.util.function.BiFunction;
Expand Down Expand Up @@ -165,7 +167,9 @@
import org.eclipse.rdf4j.query.algebra.evaluation.util.QueryEvaluationUtility;
import org.eclipse.rdf4j.query.algebra.evaluation.util.ValueComparator;
import org.eclipse.rdf4j.query.algebra.evaluation.util.XMLDatatypeMathUtil;
import org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor;
import org.eclipse.rdf4j.query.impl.EmptyBindingSet;
import org.eclipse.rdf4j.query.parser.sparql.ast.VisitorException;
import org.eclipse.rdf4j.util.UUIDable;

import com.google.common.base.Stopwatch;
Expand Down Expand Up @@ -621,20 +625,25 @@ protected QueryEvaluationStep prepare(Service service, QueryEvaluationContext co

protected QueryEvaluationStep prepare(Filter node, QueryEvaluationContext context) throws QueryEvaluationException {

QueryEvaluationStep arg = precompile(node.getArg(), context);
final FilterIterator.RetainedVariableFilteredQueryEvaluationContext context2 = new FilterIterator.RetainedVariableFilteredQueryEvaluationContext(
node, context);
QueryEvaluationStep arg = precompile(node.getArg(), context2);
QueryValueEvaluationStep ves;
try {
ves = precompile(node.getCondition(), context);
} catch (QueryEvaluationException e) {
// If we have a failed compilation we always return false.
// Which means empty. so let's short circuit that.
// ves = new QueryValueEvaluationStep.ConstantQueryValueEvaluationStep(BooleanLiteral.FALSE);
return new QueryEvaluationStep() {
var ves2 = precompile(node.getCondition(), context2);
ves = new QueryValueEvaluationStep() {

@Override
public CloseableIteration<BindingSet, QueryEvaluationException> evaluate(BindingSet bs) {
return new EmptyIteration<>();
public Value evaluate(BindingSet bindings) throws QueryEvaluationException {
assert (node != null);
return ves2.evaluate(bindings);
}

};
} catch (QueryEvaluationException e) {
// If we have a failed compilation we always return false.
// Which means empty. so let's short circuit that.
return QueryEvaluationStep.EMPTY;
}
return new QueryEvaluationStep() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
*******************************************************************************/
package org.eclipse.rdf4j.query.algebra.evaluation.iterator;

import java.util.HashSet;
import java.util.Set;
import java.util.function.BiConsumer;
import java.util.function.Predicate;

Expand All @@ -22,13 +24,17 @@
import org.eclipse.rdf4j.query.Dataset;
import org.eclipse.rdf4j.query.MutableBindingSet;
import org.eclipse.rdf4j.query.QueryEvaluationException;
import org.eclipse.rdf4j.query.algebra.BindingSetAssignment;
import org.eclipse.rdf4j.query.algebra.Filter;
import org.eclipse.rdf4j.query.algebra.QueryModelNode;
import org.eclipse.rdf4j.query.algebra.SubQueryValueOperator;
import org.eclipse.rdf4j.query.algebra.Var;
import org.eclipse.rdf4j.query.algebra.evaluation.EvaluationStrategy;
import org.eclipse.rdf4j.query.algebra.evaluation.QueryValueEvaluationStep;
import org.eclipse.rdf4j.query.algebra.evaluation.ValueExprEvaluationException;
import org.eclipse.rdf4j.query.algebra.evaluation.impl.QueryEvaluationContext;
import org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor;
import org.eclipse.rdf4j.query.algebra.helpers.collectors.VarNameCollector;

@Deprecated(since = "4.1.0")
public class FilterIterator extends FilterIteration<BindingSet, QueryEvaluationException> {
Expand Down Expand Up @@ -72,17 +78,49 @@ public static boolean isPartOfSubQuery(QueryModelNode node) {
}
}

protected static class VarCollector extends AbstractQueryModelVisitor<QueryEvaluationException> {

private final Set<String> collectedVars = new HashSet<>();

@Override
public void meet(Var var) {
if (!var.isAnonymous()) {
collectedVars.add(var.getName());
}
}

@Override
public void meet(BindingSetAssignment node) throws QueryEvaluationException {
for (BindingSet bs : node.getBindingSets()) {
for (String name : bs.getBindingNames()) {
collectedVars.add(name);
}
}
super.meet(node);
}

/**
* @return Returns the collectedVars.
*/
public Set<String> getCollectedVars() {
return collectedVars;
}

}

/**
* This is used to make sure that no variable is seen by the filter that are not in scope. Historically important in
* subquery cases.
* subquery cases. See also GH-4769
*/
public static final class RetainedVariableFilteredQueryEvaluationContext implements QueryEvaluationContext {
private final Filter node;
private final QueryEvaluationContext context;
private final Set<String> inScopeVariableNames;

public RetainedVariableFilteredQueryEvaluationContext(Filter node, QueryEvaluationContext contextToFilter) {
this.node = node;
this.context = contextToFilter;
final VarCollector varCollector = new VarCollector();
node.getArg().visit(varCollector);
this.inScopeVariableNames = varCollector.getCollectedVars();
}

@Override
Expand All @@ -105,7 +143,7 @@ public Predicate<BindingSet> hasBinding(String variableName) {
}

boolean isVariableInScope(String variableName) {
return node.getBindingNames().contains(variableName);
return inScopeVariableNames.contains(variableName);
}

@Override
Expand Down

0 comments on commit ec12d20

Please sign in to comment.