Dmitry Lychagin has posted comments on this change. Change subject: [NO ISSUE][COMP] Disable constant folding; forward recordType ......................................................................
Patch Set 5: (5 comments) Please fix the commit message. Other comments are minor (can be done in a follow up change) https://asterix-gerrit.ics.uci.edu/#/c/3218/5//COMMIT_MSG Commit Message: Line 7: [NO ISSUE][COMP] Disable constant folding; forward recordType This message is confusing: 1. We're not disabling constant folding completely, only providing a mechanism to disallow it for certain functions. 2. "forward recordType" is not in this change. We can just say "Customize constant folding" or something like that and describe it in "Details" section Line 13: Details: Changes to forward recordType from scalar descriptor to This change doesn't do any forwarding of recordType. Line 15: rules on aggregate functions. on any function, not just aggregate functions https://asterix-gerrit.ics.uci.edu/#/c/3218/5/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java: Line 100: private static final ImmutableSet<FunctionIdentifier> BUILTIN_FUNC_ID_SET_THAT_SHOULD_NOT_BE_APPLIED = minor. we don't need to keep this immutable set in memory after we copy its contents into FUNC_ID_SET_THAT_SHOULD_NOT_BE_APPLIED . we can create FUNC_ID_SET_THAT_SHOULD_NOT_BE_APPLIED with builtin functions and let it be augmented by addNonFoldableFunction(): FUNC_ID_SET_THAT_SHOULD_NOT_BE_APPLIED = new HashSet<>(Arrays.asList( .... )) https://asterix-gerrit.ics.uci.edu/#/c/3218/5/asterixdb/asterix-om/src/main/java/org/apache/asterix/runtime/evaluators/base/AbstractScalarFunctionDynamicDescriptor.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/runtime/evaluators/base/AbstractScalarFunctionDynamicDescriptor.java: Line 34: public void setImmutableStates(Object... states) { minor. this method is not needed because it overrides setImmutableStates() in AbstractFunctionDescriptor which is already empty. -- To view, visit https://asterix-gerrit.ics.uci.edu/3218 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic3c19e316e1ebdbab8a38048c1b64b087d95e146 Gerrit-PatchSet: 5 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: [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-HasComments: Yes
