This is an automated email from the ASF dual-hosted git repository.
andy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/jena.git
The following commit(s) were added to refs/heads/main by this push:
new cd556795d9 Further path flattening test coverage
new 6cb004690f Merge pull request #1640 from rvesse/path-flatten-tests
cd556795d9 is described below
commit cd556795d9fd604059de511e2fd49e14839a1795
Author: Rob Vesse <[email protected]>
AuthorDate: Tue Nov 29 10:47:06 2022 +0000
Further path flattening test coverage
Expands the path flattening test coverage so that all tests cover both
the default and the algebra based transforms. Also makes some
additional fixes to both transforms for parity in a couple of areas:
- Sequence transforms place the grounded portion of the path first in
the sequence
- Default transform now explicitly errors on invalid :p{N,M} forms
instead of generating the wrong algebra
---
.../optimize/TransformPathFlattenAlgebra.java | 22 +-
.../org/apache/jena/sparql/path/PathCompiler.java | 4 +
.../algebra/optimize/TestTransformPathFlatten.java | 268 ++++++++++++++++++---
3 files changed, 258 insertions(+), 36 deletions(-)
diff --git
a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformPathFlattenAlgebra.java
b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformPathFlattenAlgebra.java
index bd04ef0018..5501d0e2e8 100644
---
a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformPathFlattenAlgebra.java
+++
b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/optimize/TransformPathFlattenAlgebra.java
@@ -172,11 +172,16 @@ public class TransformPathFlattenAlgebra extends
TransformCopy {
@Override
public void visit(P_Mod pathMod) {
- if (pathMod.isFixedLength() && pathMod.getFixedLength() > 0) {
- // Treat as a fixed length path and convert that way instead
- Path p = PathFactory.pathFixedLength(pathMod.getSubPath(),
pathMod.getFixedLength());
- Op op = transformPath(null, subject, p, object);
- result = op;
+ if (pathMod.isFixedLength()) {
+ if (pathMod.getFixedLength() > 0) {
+ // Treat as a fixed length path and convert that way
instead
+ Path p = PathFactory.pathFixedLength(pathMod.getSubPath(),
pathMod.getFixedLength());
+ Op op = transformPath(null, subject, p, object);
+ result = op;
+ } else {
+ // Don't transform :p{0,0}
+ result = null;
+ }
return;
}
@@ -250,7 +255,12 @@ public class TransformPathFlattenAlgebra extends
TransformCopy {
Var v = varAlloc.allocVar();
Op op1 = transformPath(null, subject, pathSeq.getLeft(), v);
Op op2 = transformPath(null, v, pathSeq.getRight(), object);
- result = join(op1, op2);
+ // Want to evaluate the grounded portion of the path (if any) first
+ if (!subject.isVariable() || object.isVariable()) {
+ result = join(op1, op2);
+ } else {
+ result = join(op2, op1);
+ }
}
}
}
diff --git
a/jena-arq/src/main/java/org/apache/jena/sparql/path/PathCompiler.java
b/jena-arq/src/main/java/org/apache/jena/sparql/path/PathCompiler.java
index 33becd908b..868e527463 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/path/PathCompiler.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/path/PathCompiler.java
@@ -21,6 +21,7 @@ package org.apache.jena.sparql.path;
import org.apache.jena.graph.Node ;
import org.apache.jena.graph.Triple ;
import org.apache.jena.sparql.ARQConstants ;
+import org.apache.jena.sparql.ARQException;
import org.apache.jena.sparql.algebra.optimize.TransformPathFlattenAlgebra;
import org.apache.jena.sparql.algebra.optimize.TransformPathFlattern;
import org.apache.jena.sparql.core.PathBlock ;
@@ -181,6 +182,9 @@ public class PathCompiler
p2 = PathFactory.pathZeroOrMoreN(pMod.getSubPath()) ;
else
{
+ if ( pMod.getMin() > pMod.getMax() )
+ throw new ARQException("Bad path: " + pMod);
+
long len2 = pMod.getMax()-pMod.getMin() ;
if ( len2 < 0 ) len2 = 0 ;
p2 = PathFactory.pathMod(pMod.getSubPath(),0, len2) ;
diff --git
a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformPathFlatten.java
b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformPathFlatten.java
index 9e3f699053..3515e916d5 100644
---
a/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformPathFlatten.java
+++
b/jena-arq/src/test/java/org/apache/jena/sparql/algebra/optimize/TestTransformPathFlatten.java
@@ -22,6 +22,7 @@ import static org.apache.jena.atlas.lib.StrUtils.strjoinNL;
import static org.junit.Assert.assertEquals;
import org.apache.jena.query.ARQ;
+import org.apache.jena.sparql.ARQException;
import org.apache.jena.sparql.algebra.Op;
import org.apache.jena.sparql.algebra.Transform;
import org.apache.jena.sparql.algebra.Transformer;
@@ -43,6 +44,12 @@ public class TestTransformPathFlatten {
private static Prologue prologue;
+ /**
+ * Debug mode for developers extending these tests, when enabled the
transformation output is always printed to
+ * standard out for inspection
+ */
+ private static boolean debug = false;
+
@BeforeClass public static void beforeClass() {
prologue = new Prologue();
prologue.getPrefixMapping().setNsPrefix("", "http://example/";);
@@ -50,7 +57,7 @@ public class TestTransformPathFlatten {
@Before public void before() {
- // Reset the variable allocator.
+ // Reset the variable allocators before each test, otherwise the
expected test outputs won't match
PathCompiler.resetForTest();
TransformPathFlattenAlgebra.resetForTest();
}
@@ -58,7 +65,7 @@ public class TestTransformPathFlatten {
@Test public void pathFlatten_00() {
Op op1 = path(":x0", ":p0", ":T0");
Op op2 = op("(bgp (triple :x0 :p0 :T0))");
- test(op1, op2);
+ testDefaultTransform(op1, op2);
}
@Test public void pathFlatten_01() {
@@ -67,28 +74,70 @@ public class TestTransformPathFlatten {
," (bgp (triple :x1 :q1 ??P0))"
," (path ??P0 (path* :p1) :T1))"
);
- test(op1, op2);
+ testDefaultTransform(op1, op2);
+ }
+
+ @Test public void pathFlatten_01_algebra() {
+ Op op1 = path(":x1", ":q1/:p1*", ":T1");
+ Op op2 = op("(join"
+ ," (triple :x1 :q1 ??Q0)"
+ ," (path ??Q0 (path* :p1) :T1))"
+ );
+ testAlgebraTransform(op1, op2);
+ }
+
+ @Test public void pathFlatten_01b_algebra() {
+ Op op1 = path(":x1", ":q1/:p1*", ":T1");
+ Op op2 = op("(sequence"
+ ," (triple :x1 :q1 ??Q0)"
+ ," (path ??Q0 (path* :p1) :T1))"
+ );
+ Context context = new Context();
+ context.set(ARQ.optPathFlattenAlgebra, true);
+ testOptimise(op1, op2, context);
}
- // JENA-1918 : order of sequence is grounded first.
- @Test public void pathFlatten_02() {
+ @Test public void pathFlatten_02() {
Op op1 = path("?x", ":q1/:p1*", ":T1");
+ // JENA-1918 : order of sequence is grounded first.
Op op2 = op("(sequence"
," (path ??P0 (path* :p1) :T1)"
," (bgp (triple ?x :q1 ??P0)) )"
);
- test(op1, op2);
+ testDefaultTransform(op1, op2);
+ }
+
+ @Test public void pathFlatten_02_algebra() {
+ Op op1 = path("?x", ":q1/:p1*", ":T1");
+ // JENA-1918 : order of sequence is grounded first.
+ Op op2 = op("(join"
+ ," (path ??Q0 (path* :p1) :T1)"
+ ," (triple ?x :q1 ??Q0))"
+ );
+ testAlgebraTransform(op1, op2);
+ }
+
+ @Test public void pathFlatten_02b_algebra() {
+ Op op1 = path("?x", ":q1/:p1*", ":T1");
+ // JENA-1918 : order of sequence is grounded first.
+ Op op2 = op("(sequence"
+ ," (path ??Q0 (path* :p1) :T1)"
+ ," (triple ?x :q1 ??Q0))"
+ );
+ Context context = new Context();
+ context.set(ARQ.optPathFlattenAlgebra, true);
+ testOptimise(op1, op2, context);
}
- // JENA-1918 : order of sequence is grounded first.
@Test public void pathFlatten_10() {
Op op1 = path("?x", ":p1{2}", ":T1");
+ // JENA-1918 : order of sequence is grounded first.
Op op2 = op("(bgp"
," (triple ?x :p1 ??P0)"
," (triple ??P0 :p1 :T1)"
,")"
);
- test(op1, op2);
+ testDefaultTransform(op1, op2);
}
@Test public void pathFlatten_11() {
@@ -100,13 +149,13 @@ public class TestTransformPathFlatten {
," (triple ?x :p1 ??P1)"
," (triple ??P1 :p1 ??P0)"
," ))");
- test(op1, op2);
+ testDefaultTransform(op1, op2);
}
@Test public void pathFlatten_alt_01() {
Op op1 = path("?x", ":p1|:p2", ":T1");
// Basic flatten does not flatten alternative paths
- test(op1, op1);
+ testDefaultTransform(op1, op1);
}
@Test public void pathFlatten_alt_02() {
@@ -117,10 +166,10 @@ public class TestTransformPathFlatten {
" (triple ?x :p1 :T1)",
" (triple ?x :p2 :T1)",
")");
- testAlgebra(op1, expected);
+ testAlgebraTransform(op1, expected);
}
- @Test public void pathFlatten_alt_03() {
+ @Test public void pathFlatten_alt_03_algebra() {
Op op1 = path("?x", ":p1|^:p2", ":T1");
// Extended flatten does flatten alternative paths
Op expected = op(
@@ -128,12 +177,12 @@ public class TestTransformPathFlatten {
" (triple ?x :p1 :T1)",
" (triple :T1 :p2 ?x)",
")");
- testAlgebra(op1, expected);
+ testAlgebraTransform(op1, expected);
}
- @Test public void pathFlatten_alt_04() {
+ @Test public void pathFlatten_alt_04_algebra() {
Op op1 = path("?x", ":p1|:p2|(:p3*)", ":T1");
- // Extended flatten does flatten alternative paths
+ // Algebra flatten does flatten alternative paths
Op expected = op(
"(union",
" (union",
@@ -141,12 +190,12 @@ public class TestTransformPathFlatten {
" (triple ?x :p2 :T1))",
" (path ?x (path* :p3) :T1)",
")");
- testAlgebra(op1, expected);
+ testAlgebraTransform(op1, expected);
}
- @Test public void pathFlatten_alt_05() {
+ @Test public void pathFlatten_alt_05_algebra() {
Op op1 = path("?x", ":p1|:p2|(:p3{2})", ":T1");
- // Extended flatten does flatten alternative paths
+ // Algebra flatten does flatten alternative paths
Op expected = op(
"(union",
" (union",
@@ -156,10 +205,10 @@ public class TestTransformPathFlatten {
" (triple ?x :p3 ??Q0)",
" (triple ??Q0 :p3 :T1))",
")");
- testAlgebra(op1, expected);
+ testAlgebraTransform(op1, expected);
}
- @Test public void pathFlatten_alt_05b() {
+ @Test public void pathFlatten_alt_05b_algebra() {
Op op1 = path("?x", ":p1|:p2|(:p3{2})", ":T1");
Op expected = op(
"(union",
@@ -182,15 +231,15 @@ public class TestTransformPathFlatten {
"(path :T1 (path+ :p1) ?x)"
);
- test(op1, expected);
+ testDefaultTransform(op1, expected);
}
- @Test public void pathFlatten_reverse_algebra_01() {
+ @Test public void pathFlatten_reverse_01_algebra() {
Op op1 = path("?x", "^:p1+", ":T1");
Op expected = op(
"(path :T1 (path+ :p1) ?x)"
);
- testAlgebra(op1, expected);
+ testAlgebraTransform(op1, expected);
}
@Test public void pathFlatten_n_to_m_01() {
@@ -203,7 +252,7 @@ public class TestTransformPathFlatten {
" (triple ??P1 :p ??P0)",
"))"
);
- test(op1, expected);
+ testDefaultTransform(op1, expected);
}
@Test public void pathFlatten_n_to_m_01_algebra() {
@@ -216,7 +265,7 @@ public class TestTransformPathFlatten {
" (path ?x (pathN* :p) ??Q0)",
")"
);
- testAlgebra(op1, expected);
+ testAlgebraTransform(op1, expected);
}
@Test public void pathFlatten_n_to_m_01b_algebra() {
@@ -244,7 +293,7 @@ public class TestTransformPathFlatten {
" (path ??P0 (pathN* :p) ?x)",
")"
);
- test(op1, expected);
+ testDefaultTransform(op1, expected);
}
@Test public void pathFlatten_n_to_m_02_algebra() {
@@ -257,7 +306,143 @@ public class TestTransformPathFlatten {
" (path ??Q0 (pathN* :p) ?x)",
")"
);
- testAlgebra(op1, expected);
+ testAlgebraTransform(op1, expected);
+ }
+
+ @Test public void pathFlatten_n_to_m_03() {
+ Op op1 = path(":T1", ":p{2,4}", "?x");
+ Op expected = op(
+ "(sequence",
+ " (bgp",
+ " (triple :T1 :p ??P1)",
+ " (triple ??P1 :p ??P0)",
+ " )",
+ " (path ??P0 (mod 0 2 :p) ?x)",
+ ")"
+ );
+ Context context = new Context();
+ context.set(ARQ.optPathFlattenAlgebra, false);
+ testOptimise(op1, expected, context);
+ }
+
+ @Test public void pathFlatten_n_to_m_03_algebra() {
+ Op op1 = path(":T1", ":p{2,4}", "?x");
+ Op expected = op(
+ "(union",
+ " (union",
+ " (bgp",
+ " (triple :T1 :p ??Q0)",
+ " (triple ??Q0 :p ?x))",
+ " (bgp",
+ " (triple :T1 :p ??Q2)",
+ " (triple ??Q2 :p ??Q3)",
+ " (triple ??Q3 :p ?x)",
+ " ))",
+ " (bgp",
+ " (triple :T1 :p ??Q5)",
+ " (triple ??Q5 :p ??Q6)",
+ " (triple ??Q6 :p ??Q7)",
+ " (triple ??Q7 :p ?x)",
+ "))"
+ );
+ Context context = new Context();
+ context.set(ARQ.optPathFlattenAlgebra, true);
+ testOptimise(op1, expected, context);
+ }
+
+ @Test public void pathFlatten_n_to_m_04() {
+ Op op1 = path(":T1", ":p{2,2}", "?x");
+ Op expected = op(
+ "(bgp",
+ " (triple :T1 :p ??P0)",
+ " (triple ??P0 :p ?x)",
+ ")"
+ );
+ Context context = new Context();
+ context.set(ARQ.optPathFlattenAlgebra, false);
+ testOptimise(op1, expected, context);
+ }
+
+ @Test public void pathFlatten_n_to_m_04_algebra() {
+ Op op1 = path(":T1", ":p{2,2}", "?x");
+ Op expected = op(
+ "(bgp",
+ " (triple :T1 :p ??Q0)",
+ " (triple ??Q0 :p ?x)",
+ ")"
+ );
+ Context context = new Context();
+ context.set(ARQ.optPathFlattenAlgebra, true);
+ testOptimise(op1, expected, context);
+ }
+
+ @Test public void pathFlatten_n_to_m_05() {
+ // Not currently transformed
+ Op op1 = path(":T1", ":p{,2}", "?x");
+ testDefaultTransform(op1, null);
+ }
+
+ @Test public void pathFlatten_n_to_m_05_algebra() {
+ // Not currently transformed
+ Op op1 = path(":T1", ":p{,2}", "?x");
+ testAlgebraTransform(op1, null);
+ }
+
+ @Test public void pathFlatten_n_to_m_06() {
+ // Not currently transformed
+ Op op1 = path(":T1", ":p{0,0}", "?x");
+ testDefaultTransform(op1, null);
+ }
+
+ @Test public void pathFlatten_n_to_m_06_algebra() {
+ // Not currently transformed
+ Op op1 = path(":T1", ":p{0,0}", "?x");
+ testAlgebraTransform(op1, null);
+ }
+
+ @Test public void pathFlatten_n_to_m_07() {
+ Op op1 = path(":T1", ":p{1,1}", "?x");
+ Op expected = op(
+ "(bgp",
+ " (triple :T1 :p ?x))"
+ );
+ testDefaultTransform(op1, expected);
+ }
+
+ @Test public void pathFlatten_n_to_m_07_algebra() {
+ Op op1 = path(":T1", ":p{1,1}", "?x");
+ Op expected = op(
+ " (triple :T1 :p ?x)"
+ );
+ testAlgebraTransform(op1, expected);
+ }
+
+ @Test(expected = ARQException.class) public void pathFlatten_n_to_m_08() {
+ // Illegal path, should be caught long before it reaches the algebra
stage but
+ // can be constructed by using lower level APIs
+ Op op1 = path(":T1", ":p{3,2}", "?x");
+ testDefaultTransform(op1, null);
+ }
+
+ @Test(expected = ARQException.class) public void
pathFlatten_n_to_m_08_algebra() {
+ // Illegal path, should be caught long before it reaches the algebra
stage but
+ // can be constructed by using lower level APIs
+ Op op1 = path(":T1", ":p{3,2}", "?x");
+ testAlgebraTransform(op1, null);
+ }
+
+ @Test(expected = ARQException.class) public void pathFlatten_n_to_m_09() {
+ // Illegal path, should be caught long before it reaches the algebra
stage but
+ // can be constructed by using lower level APIs
+ Op op1 = path(":T1", ":p{3,0}", "?x");
+ testDefaultTransform(op1, null);
+ }
+
+ @Test(expected = ARQException.class) public void
pathFlatten_n_to_m_09_algebra() {
+ // Illegal path, should be caught long before it reaches the algebra
stage but
+ // can be constructed by using lower level APIs
+ Op op1 = path(":T1", ":p{3,0}", "?x");
+ testAlgebraTransform(op1, null);
}
private static Op path(String s, String pathStr, String o) {
@@ -272,17 +457,32 @@ public class TestTransformPathFlatten {
return SSE.parseOp(input);
}
- private static void test(Op opInput, Op opExpected) {
+ private static void testDefaultTransform(Op opInput, Op opExpected) {
testPathTransform(opInput, opExpected, new TransformPathFlattern());
}
+ /**
+ * Tests the behaviour of a specific path transform
+ *
+ * @param opInput Input algebra
+ * @param opExpected Expected output algebra
+ * @param transform Algebra transformation to apply
+ */
private static void testPathTransform(Op opInput, Op opExpected, Transform
transform) {
Op op = Transformer.transform(transform, opInput);
verifyTransforms(opInput, opExpected, op);
}
+ /**
+ * Verifies that a given transformation had the expected output
+ * @param opInput Input algebra
+ * @param opExpected Expected output algebra, if {@code null} then
expect the input to not be modified
+ * @param opTransformed Actual transformation output algebra
+ */
private static void verifyTransforms(Op opInput, Op opExpected, Op
opTransformed) {
-
System.out.println(opTransformed.toString(prologue.getPrefixMapping()));
+ if (debug) {
+
System.out.println(opTransformed.toString(prologue.getPrefixMapping()));
+ }
if ( opExpected == null ) {
// Expect no transformation to be applied so input should be same
as transformation output
assertEquals(opInput, opTransformed);
@@ -292,10 +492,18 @@ public class TestTransformPathFlatten {
}
}
- private static void testAlgebra(Op opInput, Op opExpected) {
+ private static void testAlgebraTransform(Op opInput, Op opExpected) {
testPathTransform(opInput, opExpected, new
TransformPathFlattenAlgebra());
}
+ /**
+ * Tests with the full {@link OptimizerStd} applied, this helps simplify
some of the resulting algebra structures
+ * by merging BGPs and makes the expected output definition simpler for
the more complex path cases
+ *
+ * @param opInput Input algebra
+ * @param opExpected Expected output algebra
+ * @param context Context, used to control which optimisations are
applied
+ */
private static void testOptimise(Op opInput, Op opExpected, Context
context) {
OptimizerStd optimizer = new OptimizerStd(context);
Op op = optimizer.rewrite(opInput);
