Dmitry Lychagin has posted comments on this change.

Change subject: [NO ISSUE][FUN] Implement array functions 1
......................................................................


Patch Set 5:

(5 comments)

https://asterix-gerrit.ics.uci.edu/#/c/2751/5/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/AbstractArrayAddRemoveEval.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/AbstractArrayAddRemoveEval.java:

Line 83:      * @return -1 if position value is missing, -2 if null, otherwise 
should return the adjusted position value, >= 0
minor. let's create constants for -1 and -2. (RETURN_NULL, RETURN_MISSING)


Line 110:         CastTypeEvaluator caster = new CastTypeEvaluator();
create CastTypeEvaluator in the constructor and reuse instance


Line 147:             listBuilder = new OrderedListBuilder();
we need to reuse list builder instances. let's create 2 fields: 
listBuilderOrdered and listBuilderUnordered. They'll initially be set to null, 
then here if it was null the create new instance.


Line 163:         ListAccessor listAccessor = new ListAccessor();
create ListAccessor in the constructor.


https://asterix-gerrit.ics.uci.edu/#/c/2751/5/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/ArrayAppendDescriptor.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/ArrayAppendDescriptor.java:

Line 87:             ATypeTag listTag =
it's a bit confusing that we might get AListPointable here that is not a list. 
We usually evaluate into VoidPointable first then check type tag and then 
assign to another typed pointable (AListPointable, etc) to access the data. Can 
we pass ATypeTag as an argument to this method and non-null AListPointable only 
when we know that the value is a list.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d9cb80325138daf99fb039793446d109481c94b
Gerrit-PatchSet: 5
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ali Alsuliman <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Michael Blow <[email protected]>
Gerrit-Reviewer: Murtadha Hubail <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to