>From Ian Maxon <[email protected]>:

Ian Maxon has posted comments on this change. ( 
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13324 )

Change subject: [NO ISSUE][API] Introduce Lossless-ADM-JSON encoding
......................................................................


Patch Set 4: Code-Review+2

(4 comments)

few questions/nits but loos good

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13324/4/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/parser/AbstractDataParser.java
File 
asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/parser/AbstractDataParser.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13324/4/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/parser/AbstractDataParser.java@393
PS4, Line 393:         } catch (NumberFormatException e) {
wonder why we were using the boxed one to begin with...


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13324/4/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/parser/LosslessADMJSONDataParser.java
File 
asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/parser/LosslessADMJSONDataParser.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13324/4/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/parser/LosslessADMJSONDataParser.java@273
PS4, Line 273:     return null;
is there anything of interest in this exception that could be carried up?


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13324/4/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/parser/test/LosslessADMJsonDataParserTest.java
File 
asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/parser/test/LosslessADMJsonDataParserTest.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13324/4/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/parser/test/LosslessADMJsonDataParserTest.java@182
PS4, Line 182: qlppRQGJoinsIT
shouldn't this be LosslessADMJsonDataParserTest?


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13324/4/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/base/AOrderedList.java
File 
asterixdb/asterix-om/src/main/java/org/apache/asterix/om/base/AOrderedList.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13324/4/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/base/AOrderedList.java@81
PS4, Line 81:         AOrderedList y = (AOrderedList) o;
if i understand this right we're now saying the type of the list has to be the 
same as well as all the items. what if i had an any-typed list of ints and a 
typed list of ints though?



--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13324
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: I94902632cce647a7b40ebfd7e5e9c514d43e388f
Gerrit-Change-Number: 13324
Gerrit-PatchSet: 4
Gerrit-Owner: Dmitry Lychagin <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <[email protected]>
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Comment-Date: Wed, 22 Sep 2021 23:54:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Reply via email to