>From Peeyush Gupta <[email protected]>:

Attention is currently required from: Ali Alsuliman, Michael Blow.
Peeyush Gupta has posted comments on this change. ( 
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20135 )

Change subject: [ASTERIXDB-3633][EXT] Support file splits while reading delta 
tables
......................................................................


Patch Set 8:

(4 comments)

File 
asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/aws/delta/DeltaFileRecordReader.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20135/comment/844a6843_9abe2ce4
PS7, Line 112: DeltaFileRecordReader
> Minor; could we refactor common parts and call these constructors to a single 
> private constructor?
Done


File 
asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/aws/delta/DeltaReaderFactory.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20135/comment/dddd3495_5e261d3a
PS7, Line 192: FileSplit
> can we just extend FileSplit with a serializable implementation here?
Did not quite understand your comment. Do you want me to extend the FileSplit 
here using anonymous class instead of creating a new file?


File 
asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/aws/delta/SerializableFileSplit.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20135/comment/30fab41c_526025db
PS7, Line 49: dos.close();
> Minor; could we use try-with resources? or? same below. […]
updated to use try-with resources


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20135/comment/0d29737e_0f3b52cf
PS7, Line 57:         try {
            :             Class splitClass = 
Class.forName(FileSplit.class.getName());
            :             Constructor[] constructors = 
splitClass.getDeclaredConstructors();
            :             Constructor defaultConstructor = null;
            :             for (Constructor constructor : constructors) {
            :                 if (constructor.getParameterTypes().length == 0) {
            :                     constructor.setAccessible(true);
            :                     defaultConstructor = constructor;
            :                 }
            :             }
> this should be done once in the `<clinit>`
Done



--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20135
To unsubscribe, or for help writing mail filters, visit 
https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: ionic
Gerrit-Change-Id: Ie6daf3846064326bfe749ad15b508fe27d1721ca
Gerrit-Change-Number: 20135
Gerrit-PatchSet: 8
Gerrit-Owner: Peeyush Gupta <[email protected]>
Gerrit-Reviewer: Ali Alsuliman <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Peeyush Gupta <[email protected]>
Gerrit-CC: Michael Blow <[email protected]>
Gerrit-Attention: Ali Alsuliman <[email protected]>
Gerrit-Attention: Michael Blow <[email protected]>
Gerrit-Comment-Date: Tue, 26 Aug 2025 17:19:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Ali Alsuliman <[email protected]>
Comment-In-Reply-To: Michael Blow <[email protected]>
Gerrit-MessageType: comment

Reply via email to