>From Wail Alkowaileet <[email protected]>:

Attention is currently required from: [email protected].
Wail Alkowaileet has posted comments on this change. ( 
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18200 )

Change subject: [WIP] Copy to S3 in parquet format
......................................................................


Patch Set 12:

(8 comments)

File 
asterixdb/asterix-om/src/main/java/org/apache/asterix/om/pointables/printer/parquet/ParquetRecordVisitorUtils.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18200/comment/60865838_3e51b3fd
PS12, Line 49: Stringify
That's expensive. You're allocating byte[] for each call to Stringify. Is there 
a way to write a string without converting the byte[] of valueReference? (e.g., 
CharBuffer?)


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18200/comment/49aea65d_329b5fef
PS12, Line 62: AppendValueToParquetGroup
appendValueToParquetGroup. Java convention: methods and members should start 
with a small letter. Only classes and interfaces names start with a capital 
letter.


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18200/comment/3c65740f_7f14d700
 
PS12, Line 62: IValueReference
Pass LazyPointable here instead to avoid the multiple casts below


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18200/comment/9402d9ec_98877761
PS12, Line 71: pointable instanceof FlatLazyVisitablePointable
Check the typetag instead. We have only three nested types


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18200/comment/432ff621_47ed980f
PS12, Line 85:
             :         IValueReference iValueReference = new IValueReference() {
             :             @Override
             :             public byte[] getByteArray() {
             :                 return b;
             :             }
             :
             :             @Override
             :             public int getStartOffset() {
             :                 return s;
             :             }
             :
             :             @Override
             :             public int getLength() {
             :                 return l;
             :             }
             :         };
Use VoidPointable instead


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18200/comment/baf406a6_89e1ae50
PS12, Line 104: TINYINT
This should simply append a long. tintyint cannot be a boolean. Hence,  
group.append(fieldName, (long) tinyIntValue) should suffice.

My hope here is that we will infer the schema and we should not get into those 
conversions.


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18200/comment/214d5d7a_b1580c2a
PS12, Line 130: SMALLINT
Same as above


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18200/comment/94c0f2cd_2ad2c951
PS12, Line 208: FLOAT
roup.append(fieldName, (double) floatValue);



--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18200
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: I108be151cadbb7989d22f126296280964c4b838f
Gerrit-Change-Number: 18200
Gerrit-PatchSet: 12
Gerrit-Owner: [email protected]
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-CC: Wail Alkowaileet <[email protected]>
Gerrit-Attention: [email protected]
Gerrit-Comment-Date: Tue, 19 Mar 2024 15:45:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to