Preston Carman has posted comments on this change. Change subject: ASTERIXDB-1281 - Interval format update to AQL and ADM ......................................................................
Patch Set 1: (4 comments) Responses to your first look. https://asterix-gerrit.ics.uci.edu/#/c/602/1/asterix-external-data/src/main/resources/adm.grammar File asterix-external-data/src/main/resources/adm.grammar: Line 52: INTERVAL_DATETIME_CONS = string(interval-datetime) > Can't we remove the last 3 ones now? Yes, I will upload a new patch. I was not sure if we wanted to be backwards compatible. https://asterix-gerrit.ics.uci.edu/#/c/602/1/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/adm/AIntervalPrinter.java File asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/adm/AIntervalPrinter.java: Line 54: AIntervalSerializerDeserializer.getStartSize(b, s + 1), ps); > It is feasible to pull these out into constants? These should definitely come from the AIntervalSerializerDeserializer (or other definitive source). The issues is now with the new interval format, an interval is variable length. A date and time interval only uses 9 (1,4,4) bytes where a datetime interval uses 17 (1,8,8) bytes. Previously we had used a long to save the date and time attribute to keep as a fixed length field. So I am not sure if we can remove these due to the dynamic size of the data. https://asterix-gerrit.ics.uci.edu/#/c/602/1/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/json/clean/AIntervalPrinter.java File asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/json/clean/AIntervalPrinter.java: Line 54: AIntervalSerializerDeserializer.getStartSize(b, s + 1), ps); > It is feasible to pull these out into constants? see previous comment https://asterix-gerrit.ics.uci.edu/#/c/602/1/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/json/lossless/AIntervalPrinter.java File asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/json/lossless/AIntervalPrinter.java: Line 54: AIntervalSerializerDeserializer.getStartSize(b, s + 1), ps); > It is feasible to pull these out into constants? see previous comment -- To view, visit https://asterix-gerrit.ics.uci.edu/602 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I009c71b7a445d141e228ba15d56d0b6cf3c8a3f5 Gerrit-PatchSet: 1 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Preston Carman <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Preston Carman <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-HasComments: Yes
