Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12985 )
Change subject: IMPALA-8381: WIP - Optimize ParquetPlainEncoder::DecodeBatch() for simple types ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/12985/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12985/2//COMMIT_MSG@37 PS2, Line 37: in nit: long line, wrap at 72 when possible http://gerrit.cloudera.org:8080/#/c/12985/1/be/src/exec/parquet/parquet-common.h File be/src/exec/parquet/parquet-common.h: http://gerrit.cloudera.org:8080/#/c/12985/1/be/src/exec/parquet/parquet-common.h@237 PS1, Line 237: int fixed_len_size, int64_t num_values, int64_t stride, InternalType* v); > Is it a special case? A 96 bit value takes 12 bytes. sizeof(TimestampValue) == 16 due to padding (to ensure the 8 byte alignment of one of its members), but we only overwrite the first 12 byte of it, that is why 12 is used instead of sizeof(TimestampValue). The padding doesn't make sense in reality, as the slots are not aligned, but we couldn't #pragma pack TimestampValue due to its boost struct members. IMPALA-7367 contains some details about this. http://gerrit.cloudera.org:8080/#/c/12985/1/be/src/exec/parquet/parquet-common.h@477 PS1, Line 477: > We never bounds check the output buffer, only the input buffer, and FIXED_L Sorry, another idea for the name: has_variable_length I think that this would make the reason behind's BYTE_ARRAY's special handling even clearer. -- To view, visit http://gerrit.cloudera.org:8080/12985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I57b7d2573bb6dfd038e581acb3bd8ea1565aa20d Gerrit-Change-Number: 12985 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Thu, 11 Apr 2019 15:21:45 +0000 Gerrit-HasComments: Yes
