Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11984 )
Change subject: IMPALA-7853: Add support to read int64 NANO timestamps from Parquet ...................................................................... Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11984/2/be/src/exec/parquet-common.cc File be/src/exec/parquet-common.cc: http://gerrit.cloudera.org:8080/#/c/11984/2/be/src/exec/parquet-common.cc@148 PS2, Line 148: if (e.type == parquet::Type::INT96 && convert_int96_timestamps) needs_conversion = true; > I see your problem now as this looks suboptimal in my opinion too. I would vote for returning to the last solution. My problem with saving it in a member is that another member would be needed to store the physical type, because currently it is not stored whether the timestamp is int64 or int96, as this information is always available in the context when ParquetTimestampDecoder is used (often as template parameter, so without runtime cost). -- To view, visit http://gerrit.cloudera.org:8080/11984 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I932396d8646f43c0b9ca4a6359f164c4d8349d8f Gerrit-Change-Number: 11984 Gerrit-PatchSet: 2 Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Thu, 06 Dec 2018 14:47:06 +0000 Gerrit-HasComments: Yes
