divijvaidya commented on a change in pull request #1553:
URL: https://github.com/apache/tinkerpop/pull/1553#discussion_r794549554
##########
File path:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/AndStep.java
##########
@@ -33,10 +34,18 @@ public AndStep(final Traversal.Admin traversal, final
Traversal<S, ?>... travers
@Override
protected boolean filter(final Traverser.Admin<S> traverser) {
+ GremlinTypeErrorException typeError = null;
for (final Traversal.Admin<S, ?> traversal : this.traversals) {
- if (!TraversalUtil.test(traverser, traversal))
- return false;
+ try {
+ if (!TraversalUtil.test(traverser, traversal))
+ return false;
+ } catch (GremlinTypeErrorException ex) {
+ // hold onto it until the end in case any other arguments
evaluate to FALSE
Review comment:
Understood. Your comment made me think about this change in the
semantics itself.
My understanding is that we are changing the semantics to say "illegal
comparisons are treated as false i.e. not comparable" unlike before where we
used to throw an exception on illegal comparisons. I also understand that the
new change in preferred since it removes "null" as a result for a comparison
evaluation and beings it closer to equality/equivalence semantics.
But why do we need ternary logic? Why can't Illegal comparison simply be
false (instead of error)? (please feel free to point me to a document or
discussion thread if this has already been discussed before).
I am advocating for illegal comparison to be treated as `false` at predicate
evaluation itself to keep things simpler for customers. It's easy to understand
"illegal comparisons are treated as false" but it is difficult to navigate the
granular semantics defined in the table in this PR (the one involving XOR).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
