Skip to content

Commit 2f0767c

Browse files
hmottestadhmottestad
committed
closes GH-5115 fix for bug where sh:class could cause missing validation results when nested sh:property and sh:node
1 parent eae9e0a commit 2f0767c

6 files changed

Lines changed: 85 additions & 14 deletions

File tree

‎core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/iterator/PeekMarkIterator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public class PeekMarkIterator<E> implements CloseableIteration<E> {
3939

4040
private boolean closed;
4141

42-
PeekMarkIterator(CloseableIteration<E> iterator) {
42+
public PeekMarkIterator(CloseableIteration<E> iterator) {
4343
this.iterator = iterator;
4444
}
4545

‎core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/ClassConstraintComponent.java‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,10 @@
2929
import org.eclipse.rdf4j.sail.shacl.ast.ValidationApproach;
3030
import org.eclipse.rdf4j.sail.shacl.ast.ValidationQuery;
3131
import org.eclipse.rdf4j.sail.shacl.ast.paths.Path;
32+
import org.eclipse.rdf4j.sail.shacl.ast.planNodes.AllTargetsPlanNode;
3233
import org.eclipse.rdf4j.sail.shacl.ast.planNodes.BufferedSplitter;
3334
import org.eclipse.rdf4j.sail.shacl.ast.planNodes.BulkedExternalInnerJoin;
35+
import org.eclipse.rdf4j.sail.shacl.ast.planNodes.DebugPlanNode;
3436
import org.eclipse.rdf4j.sail.shacl.ast.planNodes.EmptyNode;
3537
import org.eclipse.rdf4j.sail.shacl.ast.planNodes.FilterByPredicateObject;
3638
import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PlanNode;
@@ -85,10 +87,30 @@ public PlanNode generateTransactionalValidationPlan(ConnectionsGroup connections
8587
PlanNode addedTargets;
8688

8789
if (overrideTargetNode != null) {
88-
addedTargets = effectiveTarget.extend(overrideTargetNode.getPlanNode(), connectionsGroup,
89-
validationSettings.getDataGraph(), scope,
90-
EffectiveTarget.Extend.right,
91-
false, null);
90+
PlanNode planNode = overrideTargetNode.getPlanNode();
91+
if (planNode instanceof AllTargetsPlanNode) {
92+
// We are cheating a bit here by retrieving all the targets and values at the same time by
93+
// pretending to be in node shape scope and then shifting the results back to property shape scope
94+
PlanNode allTargets = getTargetChain()
95+
.getEffectiveTarget(Scope.nodeShape,
96+
connectionsGroup.getRdfsSubClassOfReasoner(), stableRandomVariableProvider)
97+
.getAllTargets(connectionsGroup, validationSettings.getDataGraph(), Scope.nodeShape);
98+
allTargets = new ShiftToPropertyShape(allTargets);
99+
100+
// filter by type against the base sail
101+
allTargets = new FilterByPredicateObject(
102+
connectionsGroup.getBaseConnection(),
103+
validationSettings.getDataGraph(), RDF.TYPE, clazzSet,
104+
allTargets, false, FilterByPredicateObject.FilterOn.value, true);
105+
106+
return allTargets;
107+
108+
} else {
109+
addedTargets = effectiveTarget.extend(planNode, connectionsGroup,
110+
validationSettings.getDataGraph(), scope,
111+
EffectiveTarget.Extend.right,
112+
false, null);
113+
}
92114

93115
} else {
94116
BufferedSplitter addedTargetsBufferedSplitter = new BufferedSplitter(
@@ -98,7 +120,7 @@ public PlanNode generateTransactionalValidationPlan(ConnectionsGroup connections
98120
PlanNode addedByPath = path.getAllAdded(connectionsGroup, validationSettings.getDataGraph(), null);
99121

100122
addedByPath = effectiveTarget.getTargetFilter(connectionsGroup,
101-
validationSettings.getDataGraph(), Unique.getInstance(new TrimToTarget(addedByPath), false));
123+
validationSettings.getDataGraph(), Unique.getInstance(new TrimToTarget(addedByPath), true));
102124

103125
addedByPath = new ReduceTargets(addedByPath, addedTargetsBufferedSplitter.getPlanNode());
104126

@@ -132,6 +154,12 @@ public PlanNode generateTransactionalValidationPlan(ConnectionsGroup connections
132154
addedTargets = Unique.getInstance(addedTargets, false);
133155
}
134156

157+
int size = effectiveTarget.size();
158+
159+
if (size > 1) {
160+
addedTargets = Unique.getInstance(addedTargets, true);
161+
}
162+
135163
PlanNode falseNode = new BulkedExternalInnerJoin(
136164
addedTargets,
137165
connectionsGroup.getBaseConnection(),

‎core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/planNodes/BulkedExternalInnerJoin.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.eclipse.rdf4j.query.BindingSet;
2222
import org.eclipse.rdf4j.query.Dataset;
2323
import org.eclipse.rdf4j.query.algebra.TupleExpr;
24+
import org.eclipse.rdf4j.query.algebra.evaluation.iterator.PeekMarkIterator;
2425
import org.eclipse.rdf4j.sail.SailConnection;
2526
import org.eclipse.rdf4j.sail.memory.MemoryStoreConnection;
2627
import org.eclipse.rdf4j.sail.shacl.ast.SparqlFragment;
@@ -93,14 +94,14 @@ public CloseableIteration<? extends ValidationTuple> iterator() {
9394
ArrayDeque<ValidationTuple> left;
9495
ArrayDeque<ValidationTuple> right;
9596
ArrayDeque<ValidationTuple> joined;
96-
private CloseableIteration<? extends ValidationTuple> leftNodeIterator;
97+
private PeekMarkIterator<? extends ValidationTuple> leftNodeIterator;
9798

9899
@Override
99100
protected void init() {
100101
left = new ArrayDeque<>(BULK_SIZE);
101102
right = new ArrayDeque<>(BULK_SIZE);
102103
joined = new ArrayDeque<>(BULK_SIZE);
103-
leftNodeIterator = leftNode.iterator();
104+
leftNodeIterator = new PeekMarkIterator<>(leftNode.iterator());
104105
}
105106

106107
private void calculateNext() {
@@ -112,6 +113,16 @@ private void calculateNext() {
112113
while (joined.isEmpty() && leftNodeIterator.hasNext()) {
113114

114115
while (left.size() < BULK_SIZE && leftNodeIterator.hasNext()) {
116+
if (!left.isEmpty()) {
117+
ValidationTuple peek = leftNodeIterator.peek();
118+
if (peek.sameTargetAs(left.getFirst())) {
119+
// stop if we detect a duplicate target since we only support distinct targets on the
120+
// left side of the join
121+
assert false
122+
: "Current and next left target is the same: " + peek + " " + left.getFirst();
123+
break;
124+
}
125+
}
115126
left.addFirst(leftNodeIterator.next());
116127
}
117128

‎core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/planNodes/BulkedExternalLeftOuterJoin.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.eclipse.rdf4j.query.BindingSet;
2222
import org.eclipse.rdf4j.query.Dataset;
2323
import org.eclipse.rdf4j.query.algebra.TupleExpr;
24+
import org.eclipse.rdf4j.query.algebra.evaluation.iterator.PeekMarkIterator;
2425
import org.eclipse.rdf4j.sail.SailConnection;
2526
import org.eclipse.rdf4j.sail.memory.MemoryStoreConnection;
2627
import org.eclipse.rdf4j.sail.shacl.ast.SparqlFragment;
@@ -64,13 +65,13 @@ public CloseableIteration<? extends ValidationTuple> iterator() {
6465
ArrayDeque<ValidationTuple> left;
6566
ArrayDeque<ValidationTuple> right;
6667

67-
private CloseableIteration<? extends ValidationTuple> leftNodeIterator;
68+
private PeekMarkIterator<? extends ValidationTuple> leftNodeIterator;
6869

6970
@Override
7071
protected void init() {
7172
left = new ArrayDeque<>(BULK_SIZE);
7273
right = new ArrayDeque<>(BULK_SIZE);
73-
leftNodeIterator = leftNode.iterator();
74+
leftNodeIterator = new PeekMarkIterator<>(leftNode.iterator());
7475
}
7576

7677
private void calculateNext() {
@@ -80,6 +81,15 @@ private void calculateNext() {
8081
}
8182

8283
while (left.size() < BULK_SIZE && leftNodeIterator.hasNext()) {
84+
if (!left.isEmpty()) {
85+
ValidationTuple peek = leftNodeIterator.peek();
86+
if (peek.sameTargetAs(left.getFirst())) {
87+
// stop if we detect a duplicate target since we only support distinct targets on the left
88+
// side of the join
89+
assert false : "Current and next left target is the same: " + peek + " " + left.getFirst();
90+
break;
91+
}
92+
}
8393
left.addFirst(leftNodeIterator.next());
8494
}
8595

‎core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/planNodes/FilterByPredicateObject.java‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import java.util.Arrays;
1515
import java.util.Objects;
1616
import java.util.Set;
17+
import java.util.concurrent.ExecutionException;
1718

1819
import org.apache.commons.text.StringEscapeUtils;
1920
import org.eclipse.rdf4j.common.iteration.CloseableIteration;
@@ -22,8 +23,12 @@
2223
import org.eclipse.rdf4j.model.Statement;
2324
import org.eclipse.rdf4j.model.Value;
2425
import org.eclipse.rdf4j.sail.SailConnection;
26+
import org.eclipse.rdf4j.sail.SailException;
2527
import org.eclipse.rdf4j.sail.memory.MemoryStoreConnection;
2628

29+
import com.google.common.cache.Cache;
30+
import com.google.common.cache.CacheBuilder;
31+
2732
/**
2833
* @author Håvard Ottestad
2934
*/
@@ -41,6 +46,8 @@ public class FilterByPredicateObject implements PlanNode {
4146
private ValidationExecutionLogger validationExecutionLogger;
4247
private final Resource[] dataGraph;
4348

49+
private final Cache<Resource, Boolean> cache = CacheBuilder.newBuilder().maximumSize(1000).build();
50+
4451
public FilterByPredicateObject(SailConnection connection, Resource[] dataGraph, IRI filterOnPredicate,
4552
Set<Resource> filterOnObject, PlanNode parent, boolean returnMatching, FilterOn filterOn,
4653
boolean includeInferred) {
@@ -195,12 +202,26 @@ private boolean matches(Value subject, IRI filterOnPredicate, Resource[] filterO
195202
}
196203

197204
if (subject.isResource()) {
198-
for (Resource object : filterOnObject) {
199-
if (connection.hasStatement(((Resource) subject), filterOnPredicate, object, includeInferred,
200-
dataGraph)) {
201-
return true;
205+
try {
206+
return cache.get(((Resource) subject), () -> {
207+
for (Resource object : filterOnObject) {
208+
if (connection.hasStatement(((Resource) subject), filterOnPredicate, object, includeInferred,
209+
dataGraph)) {
210+
return true;
211+
}
212+
}
213+
return false;
214+
});
215+
} catch (ExecutionException e) {
216+
if (e.getCause() != null) {
217+
if (e.getCause() instanceof RuntimeException) {
218+
throw ((RuntimeException) e.getCause());
219+
}
220+
throw new SailException(e.getCause());
202221
}
222+
throw new SailException(e);
203223
}
224+
204225
}
205226
return false;
206227
}

‎core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/targets/TargetClass.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ private PlanNode getAddedRemovedInner(SailConnection connection, Resource[] data
6666
Resource clazz = targetClass.stream().findAny().get();
6767
planNode = new UnorderedSelect(connection, null, RDF.TYPE, clazz,
6868
dataGraph, UnorderedSelect.Mapper.SubjectScopedMapper.getFunction(scope), null);
69+
return planNode;
6970
} else {
7071
planNode = new Select(connection,
7172
SparqlFragment.bgp(Set.of(),

0 commit comments

Comments
 (0)