>From Ali Alsuliman <[email protected]>: Attention is currently required from: Murtadha Hubail, [email protected]. Ali Alsuliman has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401 )
Change subject: [NO ISSUE][COMP] Honor select-list field order ...................................................................... Patch Set 19: (7 comments) Patchset: PS19: Let's fix the 3 test failures and include the new property in the expected results: APIExecutionTest.test[APIExecutionTest 0: api: cluster_state_1] ClusterStateExecutionLessParallelismTest.test[ClusterStateExecutionLessParallelismTest 0: api: cluster_state_1] ClusterStateExecutionFullParallelismTest.test[ClusterStateExecutionFullParallelismTest 0: api: cluster_state_1] File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/2a278c6c_624ce7e2 PS19, Line 270: if (!config.containsKey(CompilerProperties.COMPILER_ORDERFIELDS_KEY)) { Can we move this up and place it right after: final PhysicalOptimizationConfig physOptConf = OptimizationConfUtil.createPhysicalOptimizationConf(compilerProperties, querySpecificConfig, configurableParameterNames, sourceLoc); This is just to keep the context together since we will need to re-visit this. File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/ARecordVisitablePointable.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/a66ff4d9_0f90fb44 PS19, Line 60: private final List<Integer> reverseLookupClosedFields = new ArrayList<>(); We can use IntArrayList from fastutil. If you search for 'IntArrayList' in the code base, you will see some examples. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/e77fabe0_0a62f6cf PS19, Line 98: LinkedHashSet<String> allOrderedFields = inputType.getAllOrderedFields(); In the case of ordered fields, this increases the typeDos stream by 2x since we will write the field names here and also below (along with creating the AFlatValuePointable 2x). We can change the code organization around so we don't have to do that. It might be simpler to just separate the two cases: ordered vs non-ordered. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/c7052200_e2f21fb0 PS19, Line 169: for (int i = fieldValues.size() - 1; i >= numFields; i--) { : fieldValues.remove(i); : } It might be faster to just clear() and then add() to avoid the extra logic/index checks associated with remove() and set(), but keep it like you have it for now since it will depend, I guess, and it may or may not be faster. File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/OpenRecordConstructorResultType.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/9b122e41_abd8b013 PS19, Line 69: metadataProvider != null I believe metadataProvider should never be null. Did you find something else? File hyracks-fullstack/algebricks/algebricks-compiler/src/main/java/org/apache/hyracks/algebricks/compiler/api/AbstractCompilerFactoryBuilder.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/c1535262_24d93218 PS19, Line 87: protected boolean selectOrderFields; Let's remove this (and the methods). I don't think we will need it. We can introduce it later if we have to. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: Ia6e37080e581b92744ddd9090b291936513c75af Gerrit-Change-Number: 18401 Gerrit-PatchSet: 19 Gerrit-Owner: [email protected] Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Attention: Murtadha Hubail <[email protected]> Gerrit-Attention: [email protected] Gerrit-Comment-Date: Tue, 24 Sep 2024 17:46:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
