Aklakan commented on code in PR #2925:
URL: https://github.com/apache/jena/pull/2925#discussion_r1918597769


##########
jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterLateral.java:
##########
@@ -292,17 +295,33 @@ public Op transform(OpTable opTable) {
             // By the assignment restriction, the binding only needs to be 
added to each row of the table.
             Table table = opTable.getTable();
             // Table vars.
-            List<Var> vars = new ArrayList<>(table.getVars());
-            binding.vars().forEachRemaining(vars::add);
-            TableN table2 = new TableN(vars);
+            List<Var> tableVars = table.getVars();
+            List<Var> vars = new ArrayList<>(tableVars);
+
+            // Track variables that appear both in the table and the binding.
+            List<Var> commonVars = new ArrayList<>();
+
+            // Index variables in a set if there are more than a few of them.
+            Collection<Var> tableVarsIndex = tableVars.size() > 4 ? new 
HashSet<>(tableVars) : tableVars;
+            binding.vars().forEachRemaining(v -> {
+                if (tableVarsIndex.contains(v)) {
+                    commonVars.add(v);
+                } else {
+                    vars.add(v);
+                }
+            });
+
+            List<Binding> bindings = new ArrayList<>(table.size());
             BindingBuilder builder = BindingFactory.builder();
             table.iterator(null).forEachRemaining(row->{
-                builder.reset();
-                builder.addAll(row);
-                builder.addAll(binding);
-                table2.addBinding(builder.build());
+                if (Algebra.compatible(row, binding, commonVars.iterator())) {

Review Comment:
   > In general, some system have optimizations that on the static analysis of 
the query.
   
   Well, the tradeoff to what extend existing implementations need to drive 
restrictions of new features in a standard.
   
   For the sake of closing the issue, should I modify/simplify the logic in 
QueryIterLateral or should I leave the slightly more general approach as it is 
for now? ~Essentially, the var scope check should prevent non-empty sets of 
commonVars.~ No, commonVars is non-empty in the case of nested laterals - 
because the outer lateral already injects its binding into the unit tables - so 
subsequent inner laterals attempt to do the same injection. Perhaps the way it 
is is safe - the alternative is to remove the compatibility check on commonVars 
(and the commonVars set itself) - because the injections should be compatible 
in the first place.



-- 
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-unsubscr...@jena.apache.org

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


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

Reply via email to