Hussain Towaileb has posted comments on this change. ( 
https://asterix-gerrit.ics.uci.edu/3043 )

Change subject: [NO ISSUE][FUN] Code generator alternative
......................................................................


Patch Set 12:

(4 comments)

https://asterix-gerrit.ics.uci.edu/#/c/3043/12/asterixdb/asterix-fuzzyjoin/src/main/java/org/apache/asterix/runtime/evaluators/common/EditDistanceCheckEvaluator.java
File 
asterixdb/asterix-fuzzyjoin/src/main/java/org/apache/asterix/runtime/evaluators/common/EditDistanceCheckEvaluator.java:

https://asterix-gerrit.ics.uci.edu/#/c/3043/12/asterixdb/asterix-fuzzyjoin/src/main/java/org/apache/asterix/runtime/evaluators/common/EditDistanceCheckEvaluator.java@73
PS12, Line 73:         // MISSING and NULL checks
> i'd like to remove these from all these classes- i think it's clear without
Done


https://asterix-gerrit.ics.uci.edu/#/c/3043/12/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/ArrayIntersectDescriptor.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/ArrayIntersectDescriptor.java:

https://asterix-gerrit.ics.uci.edu/#/c/3043/12/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/ArrayIntersectDescriptor.java@231
PS12, Line 231:                         // If it's not missing, then it's null, 
but don't return until all arguments are checked for missing
> +1
Done


https://asterix-gerrit.ics.uci.edu/#/c/3043/12/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/PointableHelper.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/PointableHelper.java:

https://asterix-gerrit.ics.uci.edu/#/c/3043/12/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/PointableHelper.java@215
PS12, Line 215:        // Pointable2
              :         switch (checkMissingOrNull(pointable2)) {
              :             case MISSING:
              :                 setMissing(result);
              :                 return true;
              :             case NULL:
              :                 isMeetNull = true;
              :                 break;
              :         }
              :
> would prefer to move the null check for pointable outside of checkMissingOr
Done.


But just to confirm, what you're recommending is having something like this:

if (pointable2 != null) {
    switch (checkMissingOrNull(pointable2)) {
        ...
    }
}


And this should be the change in checkMissingOrNull(..):
(I should probably rename this method to getPointableValueState(...))

checkMissingOrNull(@NotNull Pointable pointable) {
    if (pointable.getLength() > 0) {
        ...
    }
}


https://asterix-gerrit.ics.uci.edu/#/c/3043/12/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/PointableHelper.java@264
PS12, Line 264: pointable != null
> see above comment
Done, see the above comment.



--
To view, visit https://asterix-gerrit.ics.uci.edu/3043
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icca2e2128c4b0f2bfd8675655cf5296cbbaeba88
Gerrit-Change-Number: 3043
Gerrit-PatchSet: 12
Gerrit-Owner: Hussain Towaileb <[email protected]>
Gerrit-Reviewer: Ali Alsuliman <[email protected]>
Gerrit-Reviewer: Anon. E. Moose (1000171)
Gerrit-Reviewer: Dmitry Lychagin <[email protected]>
Gerrit-Reviewer: Hussain Towaileb <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Michael Blow <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Comment-Date: Thu, 18 Apr 2019 05:17:34 +0000
Gerrit-HasComments: Yes

Reply via email to