Dmitry Lychagin has posted comments on this change. Change subject: [NO ISSUE][FUN] Implement array functions p2 ......................................................................
Patch Set 12: (10 comments) https://asterix-gerrit.ics.uci.edu/#/c/2756/12/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/array_fun/array_distinct/array_distinct.4.query.sqlpp File asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/array_fun/array_distinct/array_distinct.4.query.sqlpp: Line 20: use TinySocial; can you add a comment to this and following query to say that the error is expected? https://asterix-gerrit.ics.uci.edu/#/c/2756/12/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/array_fun/array_sort/array_sort.4.query.sqlpp File asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/array_fun/array_sort/array_sort.4.query.sqlpp: Line 22: select array_sort([19, 3, [5]]); add a comment to this and following query to say that the error is expected https://asterix-gerrit.ics.uci.edu/#/c/2756/12/asterixdb/asterix-app/src/test/resources/runtimets/results/array_fun/array_append/array_append.3.adm File asterixdb/asterix-app/src/test/resources/runtimets/results/array_fun/array_append/array_append.3.adm: Line 1: { "t1": [ { "$1": {{ "t-mobile", "customization", "sth", 5 }} }, { "$1": {{ "verizon", "voice-clarity", "sth", 5 }} }, { "$1": {{ "iphone", "platform", "sth", 5 }} }, { "$1": {{ "samsung", "voice-command", "sth", 5 }} }, { "$1": {{ "verizon", "shortcut-menu", "sth", 5 }} }, { "$1": {{ "motorola", "speed", "sth", 5 }} }, { "$1": {{ "sprint", "voice-command", "sth", 5 }} }, { "$1": {{ "motorola", "speed", "sth", 5 }} }, { "$1": {{ "iphone", "voice-clarity", "sth", 5 }} }, { "$1": {{ "samsung", "platform", "sth", 5 }} }, { "$1": {{ "t-mobile", "shortcut-menu", "sth", 5 }} }, { "$1": {{ "verizon", "voicemail-service", "sth", 5 }} } ], "t2": [ { "$2": [ 3, "John", [ { "sth": 33 }, { "sth": 44 } ] ] } ], "t4": null, "t5": null, "t7": [ 5, 10, 12.0, "sth" ], "t9": [ 3, 3, [ 9 ], null, "sth" ] } why this change? is the order non-determistic? https://asterix-gerrit.ics.uci.edu/#/c/2756/12/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/AListFirstTypeComputer.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/AListFirstTypeComputer.java: Line 45: return AUnionType.createUnknownableType(argType); why change to unknownable here? If it's only used for reverse, sort and distinct then can we just return argType here? Non-list cases will return ANY and null/missing list cases are handled by AbstractResultTypeComputer https://asterix-gerrit.ics.uci.edu/#/c/2756/12/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/ArrayDistinctDescriptor.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/ArrayDistinctDescriptor.java: Line 99: private final HashMap<Integer, List<AbstractPointable>> hashes; use it.unimi.dsi.fastutil.ints.Int2ObjectMap to avoid Integer object construction. Line 136: sameHashes = hashes.getOrDefault(hash, new ArrayList<>()); can we reuse this ArrayList instead of creating it each time? Line 141: item = new VoidPointable(); can we use IObjectPool to reuse pointable references instead of allocating them each time? Line 164: storage = new ArrayBackedValueStorage(); use IObectPool to pool storage objects? https://asterix-gerrit.ics.uci.edu/#/c/2756/12/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/ArraySortDescriptor.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/ArraySortDescriptor.java: Line 117: storage = new ArrayBackedValueStorage(); can we use IObjectPool to reuse storage and pointable references instead of allocating them each time? https://asterix-gerrit.ics.uci.edu/#/c/2756/12/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/functions/FunctionCollection.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/functions/FunctionCollection.java: Line 393: fc.add(ArrayReverseDescriptor.FACTORY); why are we not using code-gen versions here? Seems like all these functions are following that behavior. -- To view, visit https://asterix-gerrit.ics.uci.edu/2756 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5e9c4ff6400b7fe93ca2aab0234750dc78f50659 Gerrit-PatchSet: 12 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
