You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@jena.apache.org by GitBox <gi...@apache.org> on 2022/06/06 14:33:53 UTC

[GitHub] [jena] Aklakan opened a new pull request, #1370: Fix gh-1369; handling assignments in OpAsQuery

Aklakan opened a new pull request, #1370:
URL: https://github.com/apache/jena/pull/1370

   This PR improves handling of assignents w.r.t. projections by taking the order into account.
   Assignments will only be converted into projections if the projection references them in the same order.
   
   There is one class cast to ElementGroup which I would like to get rid of and I would need input on what might be the cleanest way to do so.
   ```
   // The following cast may be critical:
   // The problem is that here its too late to add to currentGroup()
   ElementGroup activeGroup = ((ElementGroup)query.getQueryPattern());
   ```
   
   
   It adds two more test cases testBind08 and testBind09 that would fail with the prior implementation.
   All other tests continue to work.
   
   ```
   test_roundTripQuery("SELECT (?x AS ?y) { BIND ('x' AS ?x) }");
   test_roundTripQuery("SELECT ('y' AS ?y) ?x { BIND ('x' AS ?x) }"); } // Note that ?x is assigned before ?y, but projected only afterwards
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan commented on pull request #1370: GH-1369 Fix handling of assignments in OpAsQuery

Posted by GitBox <gi...@apache.org>.
Aklakan commented on PR #1370:
URL: https://github.com/apache/jena/pull/1370#issuecomment-1148443040

   I added a few more tests for somewhat corner cases that came to my mind today; it's still green - so from my side its ready for review/feedback.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan commented on pull request #1370: GH-1369 Fix handling of assignments in OpAsQuery

Posted by GitBox <gi...@apache.org>.
Aklakan commented on PR #1370:
URL: https://github.com/apache/jena/pull/1370#issuecomment-1151439147

   I added the test case.
   For robustness I replaced the naive class cast with a check to ensure that there exists an ElementGroup to which to append the ElementBinds.
   
   ```java
                       if (activeElement instanceof ElementGroup) {
                           activeGroup = (ElementGroup)activeElement;
                       } else {
                           // Not sure whether it's possible here for BINDs to exist with the
                           // activeElement NOT being a group pattern - but better safe than sorry
                           activeGroup = new ElementGroup();
                           activeGroup.addElement(activeElement);
                           query.setQueryPattern(activeGroup);
                       }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs commented on a diff in pull request #1370: GH-1369 Fix handling of assignments in OpAsQuery

Posted by GitBox <gi...@apache.org>.
afs commented on code in PR #1370:
URL: https://github.com/apache/jena/pull/1370#discussion_r893664146


##########
jena-arq/src/test/java/org/apache/jena/sparql/algebra/TestOpAsQuery.java:
##########
@@ -42,206 +42,213 @@ public class TestOpAsQuery {
     @Test public void testBasic02() { test_roundTripQuery("SELECT * { ?s ?p ?o }") ; }
     @Test public void testBasic03() { test_roundTripQuery("SELECT * { ?s ?p ?o FILTER(?o > 5) }") ; }
     @Test public void testBasic04() { test_roundTripQuery("SELECT ?s { ?s ?p ?o FILTER(?o > 5) }") ; }
-    
-    // 01, 02: Same algebra.  
+
+    // 01, 02: Same algebra.
     @Test public void testBind01() { test_roundTripQuery("SELECT ?s (?o + 5 AS ?B) { ?s ?p ?o }") ; }
     @Test public void testBind02() { test_roundTripAlegbra("SELECT ?o ?B  { ?s ?p ?o BIND (?o + 5 AS ?B) }") ; }
     // No project
     @Test public void testBind03() { test_roundTripQuery("SELECT * { ?s ?p ?o BIND (?o + 5 AS ?B)  }") ; }
-    
+
     // Over nested.
-    @Test public void testBind04() { 
+    @Test public void testBind04() {
         test_roundTripQuery("SELECT * { ?s ?p ?o BIND(?o+1 AS ?a1) ?x ?q ?v BIND(?v+2 AS ?a2) }",
-                            "SELECT * { { ?s ?p ?o BIND(( ?o + 1 ) AS ?a1) } ?x ?q ?v BIND(( ?v + 2 ) AS ?a2) } "); 
+                            "SELECT * { { ?s ?p ?o BIND(( ?o + 1 ) AS ?a1) } ?x ?q ?v BIND(( ?v + 2 ) AS ?a2) } ");
     }
-    
+
     // Over nested.
-    @Test public void testBind05() { 
+    @Test public void testBind05() {
         test_roundTripQuery("SELECT * { ?s ?p ?o BIND(?o+1 AS ?a1) ?x ?q ?v BIND(2 AS ?a2) } ORDER BY ?s",
-                            "SELECT * { { { ?s ?p ?o BIND(( ?o + 1 ) AS ?a1) } ?x ?q ?v } BIND(2 AS ?a2) } ORDER BY ?s"); 
+                            "SELECT * { { { ?s ?p ?o BIND(( ?o + 1 ) AS ?a1) } ?x ?q ?v } BIND(2 AS ?a2) } ORDER BY ?s");
     }
-    
+
     // https://issues.apache.org/jira/browse/JENA-1843
     @Test public void testBind06() { test_roundTripQuery("SELECT * { ?s ?p ?o BIND(?o + 1 AS ?a1) BIND(?v+2 as ?a2) }"); }
     @Test public void testBind07() { test_roundTripQuery("SELECT * { BIND(?o + 1 AS ?a1) BIND(?v+2 as ?a2) }"); }
-    
-    @Test public void testOptional01() 
+
+    // https://github.com/apache/jena/issues/1369
+    @Test public void testBind08() { test_roundTripQuery("SELECT (?x AS ?y) { BIND ('x' AS ?x) }"); }
+    @Test public void testBind09() { test_roundTripQuery("SELECT ('y' AS ?y) ?x { BIND ('x' AS ?x) }"); }
+    @Test public void testBind10() { test_roundTripQuery("SELECT ('y' AS ?y) ?s ('x' AS ?x) { ?s ?p ?o }"); }
+    @Test public void testBind11() { test_roundTripQuery("SELECT ?s ('y' AS ?y) ?p ?x ?o { ?s ?p ?o BIND ('x' AS ?x) }"); }
+    @Test public void testBind12() { test_roundTripQuery("SELECT ?w ('y' AS ?y) ?x { BIND('w' AS ?w) ?s ?p ?o BIND ('x' AS ?x) }"); }
+

Review Comment:
   Please add:
   ```
       @Test public void testBind13() { test_roundTripQuery("SELECT ('x' AS ?x) (str(?x) AS ?y) (str(?x) AS ?z) {}"); }
   ```
   which passes.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan commented on a diff in pull request #1370: GH-1369 Fix handling of assignments in OpAsQuery

Posted by GitBox <gi...@apache.org>.
Aklakan commented on code in PR #1370:
URL: https://github.com/apache/jena/pull/1370#discussion_r893814315


##########
jena-arq/src/test/java/org/apache/jena/sparql/algebra/TestOpAsQuery.java:
##########
@@ -42,206 +42,213 @@ public class TestOpAsQuery {
     @Test public void testBasic02() { test_roundTripQuery("SELECT * { ?s ?p ?o }") ; }
     @Test public void testBasic03() { test_roundTripQuery("SELECT * { ?s ?p ?o FILTER(?o > 5) }") ; }
     @Test public void testBasic04() { test_roundTripQuery("SELECT ?s { ?s ?p ?o FILTER(?o > 5) }") ; }
-    
-    // 01, 02: Same algebra.  
+
+    // 01, 02: Same algebra.
     @Test public void testBind01() { test_roundTripQuery("SELECT ?s (?o + 5 AS ?B) { ?s ?p ?o }") ; }
     @Test public void testBind02() { test_roundTripAlegbra("SELECT ?o ?B  { ?s ?p ?o BIND (?o + 5 AS ?B) }") ; }
     // No project
     @Test public void testBind03() { test_roundTripQuery("SELECT * { ?s ?p ?o BIND (?o + 5 AS ?B)  }") ; }
-    
+
     // Over nested.
-    @Test public void testBind04() { 
+    @Test public void testBind04() {
         test_roundTripQuery("SELECT * { ?s ?p ?o BIND(?o+1 AS ?a1) ?x ?q ?v BIND(?v+2 AS ?a2) }",
-                            "SELECT * { { ?s ?p ?o BIND(( ?o + 1 ) AS ?a1) } ?x ?q ?v BIND(( ?v + 2 ) AS ?a2) } "); 
+                            "SELECT * { { ?s ?p ?o BIND(( ?o + 1 ) AS ?a1) } ?x ?q ?v BIND(( ?v + 2 ) AS ?a2) } ");
     }
-    
+
     // Over nested.
-    @Test public void testBind05() { 
+    @Test public void testBind05() {
         test_roundTripQuery("SELECT * { ?s ?p ?o BIND(?o+1 AS ?a1) ?x ?q ?v BIND(2 AS ?a2) } ORDER BY ?s",
-                            "SELECT * { { { ?s ?p ?o BIND(( ?o + 1 ) AS ?a1) } ?x ?q ?v } BIND(2 AS ?a2) } ORDER BY ?s"); 
+                            "SELECT * { { { ?s ?p ?o BIND(( ?o + 1 ) AS ?a1) } ?x ?q ?v } BIND(2 AS ?a2) } ORDER BY ?s");
     }
-    
+
     // https://issues.apache.org/jira/browse/JENA-1843
     @Test public void testBind06() { test_roundTripQuery("SELECT * { ?s ?p ?o BIND(?o + 1 AS ?a1) BIND(?v+2 as ?a2) }"); }
     @Test public void testBind07() { test_roundTripQuery("SELECT * { BIND(?o + 1 AS ?a1) BIND(?v+2 as ?a2) }"); }
-    
-    @Test public void testOptional01() 
+
+    // https://github.com/apache/jena/issues/1369
+    @Test public void testBind08() { test_roundTripQuery("SELECT (?x AS ?y) { BIND ('x' AS ?x) }"); }
+    @Test public void testBind09() { test_roundTripQuery("SELECT ('y' AS ?y) ?x { BIND ('x' AS ?x) }"); }
+    @Test public void testBind10() { test_roundTripQuery("SELECT ('y' AS ?y) ?s ('x' AS ?x) { ?s ?p ?o }"); }
+    @Test public void testBind11() { test_roundTripQuery("SELECT ?s ('y' AS ?y) ?p ?x ?o { ?s ?p ?o BIND ('x' AS ?x) }"); }
+    @Test public void testBind12() { test_roundTripQuery("SELECT ?w ('y' AS ?y) ?x { BIND('w' AS ?w) ?s ?p ?o BIND ('x' AS ?x) }"); }
+

Review Comment:
   Done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs merged pull request #1370: GH-1369 Fix handling of assignments in OpAsQuery

Posted by GitBox <gi...@apache.org>.
afs merged PR #1370:
URL: https://github.com/apache/jena/pull/1370


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org