>From <[email protected]>: Attention is currently required from: Murtadha Hubail, Peeyush Gupta, Ali Alsuliman. [email protected] 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 20: (8 comments) File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/db90f37d_ef7e6c44 PS19, Line 270: if (!config.containsKey(CompilerProperties.COMPILER_ORDERFIELDS_KEY)) { > Can we move this up and place it right after: […] Done File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/ARecordVisitablePointable.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/b77bc1f1_0f5cd913 PS13, Line 240: String openFieldName = UTF8StringUtil : .toString(new StringBuilder(), dataBos.getByteArray(), fnstart + 1).toString(); > This will become expensive since it will be done for every open field for > every tuple of the result. […] Ack File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/ARecordVisitablePointable.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/7fbbbe79_71a82812 PS19, Line 60: private final List<Integer> reverseLookupClosedFields = new ArrayList<>(); > We can use IntArrayList from fastutil. […] Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/34780e6d_a115a2e7 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 […] For non order queries or queries which contains closed fields allOrderedFields will be null and we won't do any extra allocation. Issue with openfields and select order where we will first arrange the fields in their order in fieldNames. Now when we move next we need to know where all the closed fields are placed in the fieldNames so that we can reverse lookup it. For that we need to compare the fields so we are creating it the 2nd time. Not sure how to avoid the comparison. Other way is to use the string and convert the closed fieldNames to string and compare. But this again we have to do it for open fields in the Set call. Any thoughts? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/520e1d28_4fec61d5 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 w […] Ack File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/printer/ARecordPrinter.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/ccdbf1ef_35737b64 PS14, Line 65: if (orderedFields == null) { > Changed in ARecordVisitablePointables to set this field conditionally. Done 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/e50e2fe4_bac1f089 PS19, Line 69: metadataProvider != null > I believe metadataProvider should never be null. […] When i tried "select 1;" metadataProvider is null; https://github.com/couchbase/asterixdb/blob/d93b80e859cb5a68eba154daad308584ee758cd0/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/visitor/ConstantFoldingVisitor.java#L136 here null is passed for metadataprovider. 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/dab5182e_fe8c015d PS19, Line 87: protected boolean selectOrderFields; > Let's remove this (and the methods). I don't think we will need it. […] Done -- 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: 20 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-Reviewer: Peeyush Gupta <[email protected]> Gerrit-Attention: Murtadha Hubail <[email protected]> Gerrit-Attention: Peeyush Gupta <[email protected]> Gerrit-Attention: Ali Alsuliman <[email protected]> Gerrit-Comment-Date: Wed, 25 Sep 2024 08:50:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Ali Alsuliman <[email protected]> Comment-In-Reply-To: [email protected] Gerrit-MessageType: comment
