GH-5112 fix for sh:pattern difference between SPARQL and transactional validation

hmottestad · hmottestad · commit 302d4eface8d · 2024-08-26T09:58:56.000+02:00
diff --git a/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/evaluationsteps/values/QueryValueEvaluationStepSupplier.java b/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/evaluationsteps/values/QueryValueEvaluationStepSupplier.java
@@ -50,7 +50,7 @@ private QueryValueEvaluationStepSupplier() {
 	}
 
 	public static QueryValueEvaluationStep prepareStr(QueryValueEvaluationStep arg, ValueFactory valueFactory) {
-		return make(arg, "Unkown constant argument for STR()", bs -> str(arg, valueFactory, bs));
+		return make(arg, "Unknown constant argument for STR()", bs -> str(arg, valueFactory, bs));
 	}
 
 	private static Value str(QueryValueEvaluationStep arg, ValueFactory valueFactory, BindingSet bindings) {
@@ -67,7 +67,7 @@ private static Value str(QueryValueEvaluationStep arg, ValueFactory valueFactory
 				return valueFactory.createLiteral(literal.getLabel());
 			}
 		} else {
-			throw new ValueExprEvaluationException("Unkown constant argument for STR()");
+			throw new ValueExprEvaluationException("Unknown constant argument for STR()");
 		}
 	}
 
diff --git a/core/queryresultio/binary/src/main/java/org/eclipse/rdf4j/query/resultio/binary/BinaryQueryResultParser.java b/core/queryresultio/binary/src/main/java/org/eclipse/rdf4j/query/resultio/binary/BinaryQueryResultParser.java
@@ -227,7 +227,7 @@ private void processError() throws IOException, QueryResultParseException {
 		} else if (errTypeFlag == QUERY_EVALUATION_ERROR) {
 			errType = QueryErrorType.QUERY_EVALUATION_ERROR;
 		} else {
-			throw new QueryResultParseException("Unkown error type: " + errTypeFlag);
+			throw new QueryResultParseException("Unknown error type: " + errTypeFlag);
 		}
 
 		String msg = readString();
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ShaclSailBaseConfiguration.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ShaclSailBaseConfiguration.java
@@ -27,7 +27,7 @@ abstract class ShaclSailBaseConfiguration extends NotifyingSailWrapper {
 	private static final Logger logger = LoggerFactory.getLogger(ShaclSailBaseConfiguration.class);
 
 	// Field used to control if the new SPARQL based validation should be enabled or disabled. Enabled by default.
-	final boolean sparqlValidation;
+	boolean sparqlValidation;
 
 	private boolean parallelValidation = ShaclSailConfig.PARALLEL_VALIDATION_DEFAULT;
 	private boolean logValidationPlans = ShaclSailConfig.LOG_VALIDATION_PLANS_DEFAULT;
@@ -375,4 +375,5 @@ public Set<IRI> getShapesGraphs() {
 	public void setShapesGraphs(Set<IRI> shapesGraphs) {
 		this.shapesGraphs = shapesGraphs;
 	}
+
 }
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/AbstractSimpleConstraintComponent.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/AbstractSimpleConstraintComponent.java
@@ -47,18 +47,18 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public abstract class SimpleAbstractConstraintComponent extends AbstractConstraintComponent {
+public abstract class AbstractSimpleConstraintComponent extends AbstractConstraintComponent {
 
-	private static final Logger logger = LoggerFactory.getLogger(SimpleAbstractConstraintComponent.class);
+	private static final Logger logger = LoggerFactory.getLogger(AbstractSimpleConstraintComponent.class);
 
 	private Resource id;
 	TargetChain targetChain;
 
-	public SimpleAbstractConstraintComponent(Resource id) {
+	public AbstractSimpleConstraintComponent(Resource id) {
 		this.id = id;
 	}
 
-	public SimpleAbstractConstraintComponent() {
+	public AbstractSimpleConstraintComponent() {
 
 	}
 
@@ -159,15 +159,24 @@ private PlanNode getPlanNodeForOverrideTargetNode(ConnectionsGroup connectionsGr
 						validationSettings.getDataGraph(), scope);
 				allTargets = getFilterAttacherWithNegation(negatePlan, allTargets);
 
-				return Unique.getInstance(allTargets, true);
+				if (effectiveTarget.size() > 1) {
+					allTargets = Unique.getInstance(allTargets, true);
+				}
+				return allTargets;
 			} else {
-				return effectiveTarget.extend(overrideTargetPlanNode, connectionsGroup,
+				PlanNode extend = effectiveTarget.extend(overrideTargetPlanNode, connectionsGroup,
 						validationSettings.getDataGraph(), scope,
 						EffectiveTarget.Extend.right,
 						false,
 						p -> getFilterAttacherWithNegation(negatePlan, p)
 				);
 
+				if (effectiveTarget.size() > 1) {
+					extend = Unique.getInstance(extend, true);
+				}
+
+				return extend;
+
 			}
 
 		} else {
@@ -184,14 +193,22 @@ private PlanNode getPlanNodeForOverrideTargetNode(ConnectionsGroup connectionsGr
 
 				allTargets = getFilterAttacherWithNegation(negatePlan, allTargets);
 
-				return Unique.getInstance(allTargets, true);
+				if (effectiveTarget.size() > 1) {
+					allTargets = Unique.getInstance(allTargets, true);
+				}
+
+				return allTargets;
 
 			} else {
 
 				overrideTargetPlanNode = effectiveTarget.extend(overrideTargetPlanNode, connectionsGroup,
 						validationSettings.getDataGraph(), scope,
 						EffectiveTarget.Extend.right, false, null);
 
+				if (effectiveTarget.size() > 1) {
+					overrideTargetPlanNode = Unique.getInstance(overrideTargetPlanNode, true);
+				}
+
 				planNode = new BulkedExternalInnerJoin(overrideTargetPlanNode,
 						connectionsGroup.getBaseConnection(),
 						validationSettings.getDataGraph(), path.get()
@@ -314,13 +331,25 @@ String literalToString(Literal literal) {
 	@Override
 	public PlanNode getAllTargetsPlan(ConnectionsGroup connectionsGroup, Resource[] dataGraph, Scope scope,
 			StatementMatcher.StableRandomVariableProvider stableRandomVariableProvider) {
+
 		if (scope == Scope.propertyShape) {
-			PlanNode allTargetsPlan = getTargetChain()
-					.getEffectiveTarget(Scope.nodeShape, connectionsGroup.getRdfsSubClassOfReasoner(),
-							stableRandomVariableProvider)
-					.getPlanNode(connectionsGroup, dataGraph, Scope.nodeShape, true, null);
 
-			return Unique.getInstance(new ShiftToPropertyShape(allTargetsPlan), true);
+			EffectiveTarget effectiveTarget = getTargetChain()
+					.getEffectiveTarget(
+							Scope.nodeShape,
+							connectionsGroup.getRdfsSubClassOfReasoner(),
+							stableRandomVariableProvider
+					);
+
+			PlanNode allTargetsPlan = effectiveTarget
+					.getPlanNode(
+							connectionsGroup,
+							dataGraph, Scope.nodeShape,
+							true,
+							null
+					);
+
+			return Unique.getInstance(new ShiftToPropertyShape(allTargetsPlan), effectiveTarget.size() > 1);
 		}
 		return EmptyNode.getInstance();
 	}
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/DatatypeConstraintComponent.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/DatatypeConstraintComponent.java
@@ -29,7 +29,7 @@
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.FilterPlanNode;
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PlanNode;
 
-public class DatatypeConstraintComponent extends SimpleAbstractConstraintComponent {
+public class DatatypeConstraintComponent extends AbstractSimpleConstraintComponent {
 
 	private final CoreDatatype coreDatatype;
 	private final IRI datatype;
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/InConstraintComponent.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/InConstraintComponent.java
@@ -31,7 +31,7 @@
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.ValueInFilter;
 import org.eclipse.rdf4j.sail.shacl.wrapper.shape.ShapeSource;
 
-public class InConstraintComponent extends SimpleAbstractConstraintComponent {
+public class InConstraintComponent extends AbstractSimpleConstraintComponent {
 
 	private final Set<Value> in;
 
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/LanguageInConstraintComponent.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/LanguageInConstraintComponent.java
@@ -34,7 +34,7 @@
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PlanNode;
 import org.eclipse.rdf4j.sail.shacl.wrapper.shape.ShapeSource;
 
-public class LanguageInConstraintComponent extends SimpleAbstractConstraintComponent {
+public class LanguageInConstraintComponent extends AbstractSimpleConstraintComponent {
 
 	private final List<String> languageIn;
 	private final ArrayList<String> languageRanges;
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/MaxExclusiveConstraintComponent.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/MaxExclusiveConstraintComponent.java
@@ -28,7 +28,7 @@
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.LiteralComparatorFilter;
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PlanNode;
 
-public class MaxExclusiveConstraintComponent extends SimpleAbstractConstraintComponent {
+public class MaxExclusiveConstraintComponent extends AbstractSimpleConstraintComponent {
 
 	Literal maxExclusive;
 
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/MaxInclusiveConstraintComponent.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/MaxInclusiveConstraintComponent.java
@@ -28,7 +28,7 @@
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.LiteralComparatorFilter;
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PlanNode;
 
-public class MaxInclusiveConstraintComponent extends SimpleAbstractConstraintComponent {
+public class MaxInclusiveConstraintComponent extends AbstractSimpleConstraintComponent {
 
 	Literal maxInclusive;
 
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/MaxLengthConstraintComponent.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/MaxLengthConstraintComponent.java
@@ -30,7 +30,7 @@
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.MaxLengthFilter;
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PlanNode;
 
-public class MaxLengthConstraintComponent extends SimpleAbstractConstraintComponent {
+public class MaxLengthConstraintComponent extends AbstractSimpleConstraintComponent {
 
 	long maxLength;
 
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/MinExclusiveConstraintComponent.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/MinExclusiveConstraintComponent.java
@@ -28,7 +28,7 @@
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.LiteralComparatorFilter;
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PlanNode;
 
-public class MinExclusiveConstraintComponent extends SimpleAbstractConstraintComponent {
+public class MinExclusiveConstraintComponent extends AbstractSimpleConstraintComponent {
 
 	Literal minExclusive;
 
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/MinInclusiveConstraintComponent.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/MinInclusiveConstraintComponent.java
@@ -28,7 +28,7 @@
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.LiteralComparatorFilter;
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PlanNode;
 
-public class MinInclusiveConstraintComponent extends SimpleAbstractConstraintComponent {
+public class MinInclusiveConstraintComponent extends AbstractSimpleConstraintComponent {
 
 	Literal minInclusive;
 
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/MinLengthConstraintComponent.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/MinLengthConstraintComponent.java
@@ -30,7 +30,7 @@
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.MinLengthFilter;
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PlanNode;
 
-public class MinLengthConstraintComponent extends SimpleAbstractConstraintComponent {
+public class MinLengthConstraintComponent extends AbstractSimpleConstraintComponent {
 
 	long minLength;
 
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/NodeKindConstraintComponent.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/NodeKindConstraintComponent.java
@@ -23,11 +23,12 @@
 import org.eclipse.rdf4j.model.vocabulary.SHACL;
 import org.eclipse.rdf4j.sail.shacl.SourceConstraintComponent;
 import org.eclipse.rdf4j.sail.shacl.ast.StatementMatcher.Variable;
+import org.eclipse.rdf4j.sail.shacl.ast.ValidationApproach;
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.FilterPlanNode;
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.NodeKindFilter;
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PlanNode;
 
-public class NodeKindConstraintComponent extends SimpleAbstractConstraintComponent {
+public class NodeKindConstraintComponent extends AbstractSimpleConstraintComponent {
 
 	NodeKind nodeKind;
 
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/PatternConstraintComponent.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/PatternConstraintComponent.java
@@ -30,7 +30,7 @@
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PatternFilter;
 import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PlanNode;
 
-public class PatternConstraintComponent extends SimpleAbstractConstraintComponent {
+public class PatternConstraintComponent extends AbstractSimpleConstraintComponent {
 
 	String pattern;
 	String flags;
@@ -67,7 +67,10 @@ String getSparqlFilterExpression(Variable<Value> variable, boolean negated) {
 	}
 
 	private static String escapeRegexForSparql(String pattern) {
-		return pattern.replace("\\", "\\\\").replace("\"", "\\\"").replace("\n", "\\n");
+		pattern = pattern.replace("\\", "\\\\");
+		pattern = pattern.replace("\"", "\\\"");
+		pattern = pattern.replace("\n", "\\n");
+		return pattern;
 	}
 
 	@Override
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/planNodes/PatternFilter.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/planNodes/PatternFilter.java
@@ -14,7 +14,13 @@
 import java.util.Objects;
 import java.util.regex.Pattern;
 
+import org.eclipse.rdf4j.model.IRI;
+import org.eclipse.rdf4j.model.Literal;
+import org.eclipse.rdf4j.model.Triple;
 import org.eclipse.rdf4j.model.Value;
+import org.eclipse.rdf4j.model.ValueFactory;
+import org.eclipse.rdf4j.model.impl.SimpleValueFactory;
+import org.eclipse.rdf4j.query.algebra.evaluation.util.QueryEvaluationUtility;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -71,16 +77,49 @@ public PatternFilter(PlanNode parent, String pattern, String flags) {
 			logger.trace("PatternFilter constructed with pattern: {} and flags: {}", pattern, flags);
 
 		} else {
-			this.pattern = Pattern.compile(pattern);
+			this.pattern = Pattern.compile(pattern, 0b0);
 			logger.trace("PatternFilter constructed with pattern: {} and no flags", pattern);
 		}
 	}
 
+	private static Literal str(Value argValue, ValueFactory valueFactory) {
+		if (argValue instanceof IRI || argValue instanceof Triple) {
+			return valueFactory.createLiteral(argValue.toString());
+		} else if (argValue instanceof Literal) {
+			Literal literal = (Literal) argValue;
+
+			if (QueryEvaluationUtility.isSimpleLiteral(literal)) {
+				return literal;
+			} else {
+				return valueFactory.createLiteral(literal.getLabel());
+			}
+		} else {
+			return null;
+		}
+	}
+
 	@Override
 	boolean checkTuple(Reference t) {
 		Value literal = t.get().getValue();
+		literal = str(literal, SimpleValueFactory.getInstance());
 
-		return pattern.matcher(literal.stringValue()).matches();
+		if (literal == null)
+			return false;
+
+		if (QueryEvaluationUtility.isStringLiteral(literal)) {
+			boolean result = pattern.matcher(((Literal) literal).getLabel()).find();
+			if (logger.isTraceEnabled()) {
+				logger.trace("PatternFilter value: \"{}\" with pattern: \"{}\" and result: {}",
+						((Literal) literal).getLabel().replace("\n", "\\n").replace("\"", "\\\""),
+						pattern.toString().replace("\n", "\\n").replace("\"", "\\\""), result);
+			}
+			return result;
+		}
+
+		if (logger.isTraceEnabled()) {
+			logger.trace("PatternFilter did not match value because value is not a string literal: {}", literal);
+		}
+		return false;
 	}
 
 	@Override
diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/targets/EffectiveTarget.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/targets/EffectiveTarget.java
@@ -401,6 +401,10 @@ public Variable<Value> getOptionalVar() {
 		return Objects.requireNonNull(optional, "Optional was null").var;
 	}
 
+	public int size() {
+		return chain.size();
+	}
+
 	public enum Extend {
 		left,
 		right

Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ private QueryValueEvaluationStepSupplier() {`
`50`	`50`	`}`
`51`	`51`
`52`	`52`	`public static QueryValueEvaluationStep prepareStr(QueryValueEvaluationStep arg, ValueFactory valueFactory) {`
`53`		`- return make(arg, "Unkown constant argument for STR()", bs -> str(arg, valueFactory, bs));`
	`53`	`+ return make(arg, "Unknown constant argument for STR()", bs -> str(arg, valueFactory, bs));`
`54`	`54`	`}`
`55`	`55`
`56`	`56`	`private static Value str(QueryValueEvaluationStep arg, ValueFactory valueFactory, BindingSet bindings) {`
`@@ -67,7 +67,7 @@ private static Value str(QueryValueEvaluationStep arg, ValueFactory valueFactory`
`67`	`67`	`return valueFactory.createLiteral(literal.getLabel());`
`68`	`68`	`}`
`69`	`69`	`} else {`
`70`		`- throw new ValueExprEvaluationException("Unkown constant argument for STR()");`
	`70`	`+ throw new ValueExprEvaluationException("Unknown constant argument for STR()");`
`71`	`71`	`}`
`72`	`72`	`}`
`73`	`73`
Original file line number	Diff line number	Diff line change
`@@ -227,7 +227,7 @@ private void processError() throws IOException, QueryResultParseException {`
`227`	`227`	`} else if (errTypeFlag == QUERY_EVALUATION_ERROR) {`
`228`	`228`	`errType = QueryErrorType.QUERY_EVALUATION_ERROR;`
`229`	`229`	`} else {`
`230`		`- throw new QueryResultParseException("Unkown error type: " + errTypeFlag);`
	`230`	`+ throw new QueryResultParseException("Unknown error type: " + errTypeFlag);`
`231`	`231`	`}`
`232`	`232`
`233`	`233`	`String msg = readString();`
Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ abstract class ShaclSailBaseConfiguration extends NotifyingSailWrapper {`
`27`	`27`	`private static final Logger logger = LoggerFactory.getLogger(ShaclSailBaseConfiguration.class);`
`28`	`28`
`29`	`29`	`// Field used to control if the new SPARQL based validation should be enabled or disabled. Enabled by default.`
`30`		`- final boolean sparqlValidation;`
	`30`	`+ boolean sparqlValidation;`
`31`	`31`
`32`	`32`	`private boolean parallelValidation = ShaclSailConfig.PARALLEL_VALIDATION_DEFAULT;`
`33`	`33`	`private boolean logValidationPlans = ShaclSailConfig.LOG_VALIDATION_PLANS_DEFAULT;`
`@@ -375,4 +375,5 @@ public Set<IRI> getShapesGraphs() {`
`375`	`375`	`public void setShapesGraphs(Set<IRI> shapesGraphs) {`
`376`	`376`	`this.shapesGraphs = shapesGraphs;`
`377`	`377`	`}`
	`378`	`+`
`378`	`379`	`}`