>From Ali Alsuliman <[email protected]>:

Attention is currently required from: [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 14:

(5 comments)

File 
asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/ARecordVisitablePointable.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/6a708eca_aee7461f
PS14, Line 74: private final List<IVisitablePointable> openFieldLocator = new 
ArrayList<>();
             :     private final List<IVisitablePointable> closedFieldLocator = 
new ArrayList<>();
             :     private final List<IVisitablePointable> orderedFields = new 
ArrayList<>();
It seems that all of these can be created conditionally (i.e. only when there 
is order). Of course, you will have to remove the "final" modifier.


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/5fd24ba7_c21e1a01
PS14, Line 271: Integer
Let's return primitive int instead of Integer.


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/e3c14c1d_c723b327
PS14, Line 65: if (orderedFields == null) {
Isn't this always going to be false since you always create orderedFields in 
ARecordVisitablePointable? Either add || orderedFields.isEmpty() or better yet 
only create orderedFields conditionally in ARecordVisitablePointable if there 
is order which should apply to only ARecordVisitablePointables used by the 
SELECT (and not all ARecordVisitablePointable used)


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/06a8a5e5_96fc5839
PS14, Line 71: boolean[] printed = new boolean[size];
Let's get rid of this to avoid creating (garbage) objects with every printed 
record.
Define this instance field up: private BitSet remainingFields;
Then, here:
else {
boolean remainingFieldsSet = false;
...
if (index == -1) {
  if (!remainingFieldsSet) {
    initRemainingFields();
  }
  remainingFieldsSet = true;
  remainingFields.set(index);
  continue;
}
...
}

private void initRemainingFields() {
    if (remainingFields == null) {
      remainingFields = new BitSet();
    } else {
      remainingFields.clear();
    }
}


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18401/comment/f9d53fa9_bf062658
PS14, Line 81:             for (int i = 0; i < size; ++i) {
This now can be replaced with:
if (remainingFieldsSet) {
  for (int i = remainingFields.nextSetBit(0); i >= 0; i = 
remainingFields.nextSetBit(i+1)) {
     first = printRecord(ps, visitor, first, fieldNames, fieldValues, i);
 }
}



--
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: 14
Gerrit-Owner: [email protected]
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-CC: Ali Alsuliman <[email protected]>
Gerrit-Attention: [email protected]
Gerrit-Comment-Date: Tue, 27 Aug 2024 21:56:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to