Dmitry Lychagin has posted comments on this change. Change subject: [NO ISSUE][FUN] Implement array_insert() function ......................................................................
Patch Set 3: (4 comments) https://asterix-gerrit.ics.uci.edu/#/c/2738/3/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/array_fun/array_insert/array_insert.3.query.sqlpp File asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/array_fun/array_insert/array_insert.3.query.sqlpp: Line 22: { add a testcase where you're inserting 'null' into an array. also add testcase inserting multiple items into an array. https://asterix-gerrit.ics.uci.edu/#/c/2738/3/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/AbstractAListOrNullTypeComputer.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/AbstractAListOrNullTypeComputer.java: Line 34: public abstract class AbstractAListOrNullTypeComputer extends AbstractResultTypeComputer { you're only using subclasses to set minNumArgs, they don't override any methods, so really not necessary. Instead make this class non-abstract and create two static final instances: INSTANCE_2 with minNumArgs =2 and INSTANCE_3 with minNumArgs = 3. Then use these instances for array-insert and array-append (and possibly in the future for other array functions) https://asterix-gerrit.ics.uci.edu/#/c/2738/3/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/ArrayInsertDescriptor.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/ArrayInsertDescriptor.java: Line 91: argTypes = Arrays.copyOf(states, states.length, IAType[].class); array copy is not necessary here because FunctionTypeInferers.SET_ARGUMENTS_TYPE creates a new array anyway https://asterix-gerrit.ics.uci.edu/#/c/2738/3/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 385: fc.addGenerated(ArrayInsertDescriptor.FACTORY); you cannot use code-gened runtime for this function because the value to be inserted can be null. -- To view, visit https://asterix-gerrit.ics.uci.edu/2738 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I57d53f63086da7e9b3412bb12f9901f5f2088c54 Gerrit-PatchSet: 3 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: 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
