Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/11336 )
Change subject: IMPALA-6442: Misleading file offset reporting in error messages. ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11336/1/be/src/exec/hdfs-parquet-scanner.cc File be/src/exec/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/11336/1/be/src/exec/hdfs-parquet-scanner.cc@1266 PS1, Line 1266: stream_->file_desc()->file_length, offset, status.GetDetail())); The end of a Parquet file is like this: ... <Column 2 Chunk M + Column Metadata> ... <Column N Chunk M + Column Metadata> File Metadata 4-byte length in bytes of file metadata 4-byte magic number "PAR1" Before we start reading the file, we don't know the exact size of the metadata. We need the "4-byte length in bytes of file metadata" that is at the end of the file (and we want to verify the magic number). It is wasteful to read that alone, because we are going to need the rest of the footer anyway. So, we guess and read the last FOOTER_SIZE of the file (or the file size, whichever is smaller) and expect that to include the whole footer, then get the exact size by reading those last few bytes. By the time we are here, we have the exact metadata size, which is how we have the appropriate offset in the buffer and appropriate size to pass into DeserializeThriftMsg(). I think we should use file length - (metadata_size + sizeof(PARQUET_VERSION_NUMBER) + sizeof(uint32_t)) rather than the initial file offset. It's worth going through the code and verifying that what I'm saying is true. -- To view, visit http://gerrit.cloudera.org:8080/11336 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie78ef9b0e11b8505ab7bc965aafdb2729b0027c9 Gerrit-Change-Number: 11336 Gerrit-PatchSet: 1 Gerrit-Owner: Yongjun Zhang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Yongjun Zhang <[email protected]> Gerrit-Comment-Date: Mon, 27 Aug 2018 21:38:09 +0000 Gerrit-HasComments: Yes
