inject/where/choose combination causes ClassCastException #415

allenhadden · 2021-06-26T14:17:52Z

The following query causes a ClassCastException:

g.inject('test').
where(
  choose(
      __.V().hasLabel('DoesntMatter'),
      __.constant(true),
      __.constant(false)
  )
)

Note that while this gremlin is a little nonsensical, it represents a reduction of a much more complicated gremlin which also fails.

The exception trace is:

gremlin> g.inject('test').
......1> where(
......2>   choose(
......3>       __.V().hasLabel('DoesntMatter'),
......4>       __.constant(true),
......5>       __.constant(false)
......6>   )
......7> )
class java.lang.String cannot be cast to class org.umlg.sqlg.structure.SqlgElement (java.lang.String is in module java.base of loader 'bootstrap'; org.umlg.sqlg.structure.SqlgElement is in unnamed module of loader 'app')
Type ':help' or ':h' for help.
Display stack trace? [yN]y
java.lang.ClassCastException: class java.lang.String cannot be cast to class org.umlg.sqlg.structure.SqlgElement (java.lang.String is in module java.base of loader 'bootstrap'; org.umlg.sqlg.structure.SqlgElement is in unnamed module of loader 'app')
	at org.umlg.sqlg.step.barrier.SqlgBranchStepBarrier.processNextStart(SqlgBranchStepBarrier.java:80)
	at org.umlg.sqlg.step.SqlgAbstractStep.hasNext(SqlgAbstractStep.java:121)
	at org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversal.hasNext(DefaultTraversal.java:196)
	at org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil.test(TraversalUtil.java:96)
	at org.apache.tinkerpop.gremlin.process.traversal.step.filter.TraversalFilterStep.filter(TraversalFilterStep.java:59)
	at org.apache.tinkerpop.gremlin.process.traversal.step.filter.FilterStep.processNextStart(FilterStep.java:38)
	at org.apache.tinkerpop.gremlin.process.traversal.step.util.AbstractStep.hasNext(AbstractStep.java:144)
	at org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversal.hasNext(DefaultTraversal.java:196)
	at java_util_Iterator$hasNext.call(Unknown Source)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:47)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:115)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:119)
	at org.apache.tinkerpop.gremlin.console.Console$_closure3.doCall(Console.groovy:257)

FWIW, this seems to work around the problem:

g.V().limit(1).constant("test")
where(
  choose(
      __.V().hasLabel('DoesntMatter'),
      __.constant(true),
      __.constant(false)
  )
)

The text was updated successfully, but these errors were encountered:

allenhadden · 2021-06-26T14:19:10Z

Here's a simple test case that currently fails:

diff --git a/sqlg-test/src/main/java/org/umlg/sqlg/test/inject/InjectTest.java b/sqlg-test/src/main/java/org/umlg/sqlg/test/inject/InjectTest.java
new file mode 100644
index 00000000..cb3f69b2
--- /dev/null
+++ b/sqlg-test/src/main/java/org/umlg/sqlg/test/inject/InjectTest.java
@@ -0,0 +1,20 @@
+package org.umlg.sqlg.test.inject;
+
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
+import org.junit.Test;
+import org.umlg.sqlg.test.BaseTest;
+
+public class InjectTest extends BaseTest {
+    @Test
+    public void testInjectWhereChoose() {
+        this.sqlgGraph.traversal()
+                .inject("test")
+                .where(
+                        __.choose(
+                                __.V().hasLabel("DoesntMatter"),
+                                __.constant(true),
+                                __.constant(false)
+                        )
+                ).next();
+    }
+}

pietermartin · 2021-06-27T19:55:19Z

Some feedback,
I have made progress, at least the trimmed down test you provided, thanks for that, passes. Your usecase bypasses Sqlg's bypassing of inject. Sqlg does not optimize traversals that contain InjectStep. However your gremlin's nested traversals did not quite realize the input came via an inject and attempted to optimize it...

allenhadden · 2021-06-28T19:07:32Z

FWIW, for us this case is specifically to address the fact that a "union" step cannot start a traversal. For example:

g.inject('  cypher.start')  <-- this is a dummy step to get the traversal started
  .union(
    __.V().hasLabel(...),
    __.V().hasLabel(...)
  )

So at least in that case, the inject is completely unused. I'm not sure how common that is.

pietermartin · 2021-06-30T09:01:24Z

@allenhadden can you please test your more complex query? I added a bunch of InjectStep optimizations.

allenhadden · 2021-06-30T19:03:55Z

@pietermartin Awesome - your fix for inject along with the fix for #416 does resolve the more complex query! Thanks!

One thing I noticed about the fix is that there are some other places that do the same sort that you fixed. Does it make sense to do something like this? Happy to do a PR if you'd like.

diff --git a/sqlg-core/src/main/java/org/umlg/sqlg/step/SqlgAbstractStep.java b/sqlg-core/src/main/java/org/umlg/sqlg/step/SqlgAbstractStep.java
index 4fef5890..f32a66b6 100644
--- a/sqlg-core/src/main/java/org/umlg/sqlg/step/SqlgAbstractStep.java
+++ b/sqlg-core/src/main/java/org/umlg/sqlg/step/SqlgAbstractStep.java
@@ -7,6 +7,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.util.EmptyStep;
 import org.apache.tinkerpop.gremlin.process.traversal.util.EmptyTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalInterruptedException;
 import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
+import org.umlg.sqlg.structure.traverser.SqlgTraverser;
 
 import java.util.*;
 
@@ -188,4 +189,18 @@ public abstract class SqlgAbstractStep<S, E> implements Step<S, E> {
         return traverser;
     }
 
+    protected <T> void sortResults(final List<Traverser.Admin<T>> results) {
+        results.sort((o1, o2) -> {
+            if (o1 instanceof SqlgTraverser && o2 instanceof SqlgTraverser) {
+                SqlgTraverser x = (SqlgTraverser) o1;
+                SqlgTraverser y = (SqlgTraverser) o2;
+                return Long.compare(x.getStartElementIndex(), y.getStartElementIndex());
+            } else {
+                return 0;
+            }
+        });
+    }
 }
diff --git a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgAndStepBarrier.java b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgAndStepBarrier.java
index 03ce1fbf..e87b2d30 100644
--- a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgAndStepBarrier.java
+++ b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgAndStepBarrier.java
@@ -94,11 +94,9 @@ public class SqlgAndStepBarrier<S> extends SqlgConnectiveStep<S> {
                 }
                 first = false;
             }
-            this.results.sort((o1, o2) -> {
-                SqlgTraverser x = (SqlgTraverser) o1;
-                SqlgTraverser y = (SqlgTraverser) o2;
-                return Long.compare(x.getStartElementIndex(), y.getStartElementIndex());
-            });
+
+            sortResults(this.results);
+
             this.resultIterator = this.results.iterator();
         }
         if (this.resultIterator.hasNext()) {
diff --git a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgBranchStepBarrier.java b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgBranchStepBarrier.java
index b047b419..3f160d9f 100644
--- a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgBranchStepBarrier.java
+++ b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgBranchStepBarrier.java
@@ -164,16 +164,8 @@ public abstract class SqlgBranchStepBarrier<S, E, M> extends SqlgAbstractStep<S,
                 }
             }
 
-            //Sort the results, this is to ensure the the incoming start order is not lost.
-            this.results.sort((o1, o2) -> {
-                if (o1 instanceof SqlgTraverser && o2 instanceof SqlgTraverser) {
-                    SqlgTraverser x = (SqlgTraverser) o1;
-                    SqlgTraverser y = (SqlgTraverser) o2;
-                    return Long.compare(x.getStartElementIndex(), y.getStartElementIndex());
-                } else {
-                    return 0;
-                }
-            });
+            sortResults(this.results);
+
             this.resultIterator = this.results.iterator();
         }
         //noinspection LoopStatementThatDoesntLoop
diff --git a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgLocalStepBarrier.java b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgLocalStepBarrier.java
index 4857b6f9..ae0ebdb3 100644
--- a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgLocalStepBarrier.java
+++ b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgLocalStepBarrier.java
@@ -50,11 +50,9 @@ public class SqlgLocalStepBarrier<S, E> extends SqlgAbstractStep<S, E> implement
             while (this.localTraversal.hasNext()) {
                 this.results.add(this.localTraversal.nextTraverser());
             }
-            this.results.sort((o1, o2) -> {
-                ISqlgTraverser x = (ISqlgTraverser) o1;
-                ISqlgTraverser y = (ISqlgTraverser) o2;
-                return Long.compare(x.getStartElementIndex(), y.getStartElementIndex());
-            });
+
+            sortResults(this.results);
+
             this.resultIterator = this.results.iterator();
         }
         if (this.resultIterator.hasNext()) {
diff --git a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgNotStepBarrier.java b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgNotStepBarrier.java
index a65d39ac..6ae5b620 100644
--- a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgNotStepBarrier.java
+++ b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgNotStepBarrier.java
@@ -74,11 +74,9 @@ public class SqlgNotStepBarrier<S> extends SqlgFilterStep<S> implements Traversa
                 }
             }
             this.results.addAll(startRecordIds.values());
-            this.results.sort((o1, o2) -> {
-                SqlgTraverser x = (SqlgTraverser) o1;
-                SqlgTraverser y = (SqlgTraverser) o2;
-                return Long.compare(x.getStartElementIndex(), y.getStartElementIndex());
-            });
+
+            sortResults(this.results);
+
             this.resultIterator = this.results.iterator();
         }
         if (this.resultIterator.hasNext()) {
diff --git a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgOptionalStepBarrier.java b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgOptionalStepBarrier.java
index f4127581..d83768a3 100644
--- a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgOptionalStepBarrier.java
+++ b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgOptionalStepBarrier.java
@@ -72,12 +72,10 @@ public class SqlgOptionalStepBarrier<S> extends SqlgAbstractStep<S, S> implement
                 //Bulking logic interferes here, addStart calls DefaultTraversal.merge which has bulking logic
                 start.setBulk(1L);
             }
+
             //Sort the results, this is to ensure the the incoming start order is not lost.
-            this.results.sort((o1, o2) -> {
-                SqlgTraverser x = (SqlgTraverser) o1;
-                SqlgTraverser y = (SqlgTraverser) o2;
-                return Long.compare(x.getStartElementIndex(), y.getStartElementIndex());
-            });
+            sortResults(this.results);
+
             this.resultIterator = this.results.iterator();
         }
         if (this.resultIterator.hasNext()) {
diff --git a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgOrStepBarrier.java b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgOrStepBarrier.java
index 2fe77b83..44cae5e9 100644
--- a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgOrStepBarrier.java
+++ b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgOrStepBarrier.java
@@ -82,11 +82,7 @@ public class SqlgOrStepBarrier<S> extends SqlgConnectiveStep<S> {
                     }
                 }
             }
-            this.results.sort((o1, o2) -> {
-                SqlgTraverser x = (SqlgTraverser) o1;
-                SqlgTraverser y = (SqlgTraverser) o2;
-                return Long.compare(x.getStartElementIndex(), y.getStartElementIndex());
-            });
+            sortResults(this.results);
             this.resultIterator = this.results.iterator();
         }
         if (this.resultIterator.hasNext()) {
diff --git a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgTraversalFilterStepBarrier.java b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgTraversalFilterStepBarrier.java
index 52f9d532..914b47cd 100644
--- a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgTraversalFilterStepBarrier.java
+++ b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgTraversalFilterStepBarrier.java
@@ -82,11 +82,9 @@ public class SqlgTraversalFilterStepBarrier<S> extends SqlgAbstractStep<S, S> im
                 }
 
             }
-            this.results.sort((o1, o2) -> {
-                SqlgTraverser x = (SqlgTraverser) o1;
-                SqlgTraverser y = (SqlgTraverser) o2;
-                return Long.compare(x.getStartElementIndex(), y.getStartElementIndex());
-            });
+
+            sortResults(this.results);
+
             this.resultIterator = this.results.iterator();
         }
         if (this.resultIterator.hasNext()) {
diff --git a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgUnionStepBarrier.java b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgUnionStepBarrier.java
index 155eeb2f..f51ad815 100644
--- a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgUnionStepBarrier.java
+++ b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgUnionStepBarrier.java
@@ -70,11 +70,7 @@ public class SqlgUnionStepBarrier<S, E> extends SqlgAbstractStep<S, E> implement
                     this.results.add(globalTraversal.nextTraverser());
                 }
             }
-            this.results.sort((o1, o2) -> {
-                SqlgTraverser x = (SqlgTraverser) o1;
-                SqlgTraverser y = (SqlgTraverser) o2;
-                return Long.compare(x.getStartElementIndex(), y.getStartElementIndex());
-            });
+            sortResults(this.results);
             this.resultIterator = this.results.iterator();
         }
         if (this.resultIterator.hasNext()) {
diff --git a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgWhereTraversalStepBarrier.java b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgWhereTraversalStepBarrier.java
index 96c74f78..c488bc48 100644
--- a/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgWhereTraversalStepBarrier.java
+++ b/sqlg-core/src/main/java/org/umlg/sqlg/step/barrier/SqlgWhereTraversalStepBarrier.java
@@ -145,11 +145,7 @@ public class SqlgWhereTraversalStepBarrier<S> extends SqlgAbstractStep<S, S> imp
                 }
 
             }
-            this.results.sort((o1, o2) -> {
-                SqlgTraverser x = (SqlgTraverser) o1;
-                SqlgTraverser y = (SqlgTraverser) o2;
-                return Long.compare(x.getStartElementIndex(), y.getStartElementIndex());
-            });
+            sortResults(this.results);
             this.resultIterator = this.results.iterator();
         }
         if (this.resultIterator.hasNext()) {

pietermartin · 2021-07-01T08:03:40Z

Thanks for this but that and the test cases. However that if (o1 instanceof SqlgTraverser && o2 instanceof SqlgTraverser) { should not be there. It slipped through with trying to understand what was going on.

The fix is more in SqlgInjectStep and SqlgStartStepBarrier. In particular SqlgStartStepBarrier creates the SqlgTraverser which makes the if check redundant.

I removed that if, just running all the tests to see all is well.

pietermartin added the bug label Jun 27, 2021

pietermartin self-assigned this Jun 27, 2021

pietermartin closed this as completed in ea1119d Jun 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

inject/where/choose combination causes ClassCastException #415

inject/where/choose combination causes ClassCastException #415

allenhadden commented Jun 26, 2021

allenhadden commented Jun 26, 2021

pietermartin commented Jun 27, 2021

allenhadden commented Jun 28, 2021

pietermartin commented Jun 30, 2021

allenhadden commented Jun 30, 2021

pietermartin commented Jul 1, 2021

inject/where/choose combination causes ClassCastException #415

inject/where/choose combination causes ClassCastException #415

Comments

allenhadden commented Jun 26, 2021

allenhadden commented Jun 26, 2021

pietermartin commented Jun 27, 2021

allenhadden commented Jun 28, 2021

pietermartin commented Jun 30, 2021

allenhadden commented Jun 30, 2021

pietermartin commented Jul 1, 2021