You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Jim Apple (Code Review)" <ge...@cloudera.org> on 2019/07/27 19:08:05 UTC
[Impala-ASF-CR] IMPALA-5031: widen Thrift enum to placate UBSAN
Jim Apple has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13940
Change subject: IMPALA-5031: widen Thrift enum to placate UBSAN
......................................................................
IMPALA-5031: widen Thrift enum to placate UBSAN
This fixes an instance of undefined behavior in the end-to-end tests
in which an enum value is outside of the allowable values for that
enum according to the C++14 standard.
Representative backtrace:
exec/parquet/parquet-metadata-utils.cc:293:26: runtime error: load of
value 49, which is not a valid value for type 'Type::type'
#0 ParquetMetadataUtils::ValidateRowGroupColumn(
parquet::FileMetaData const&, char const*, int, int,
parquet::SchemaElement const&, RuntimeState*)
exec/parquet/parquet-metadata-utils.cc:293:26
#1 BaseScalarColumnReader::Reset(HdfsFileDesc const&,
parquet::ColumnChunk const&, int)
exec/parquet/parquet-column-readers.cc:1077:43
#2 HdfsParquetScanner::InitScalarColumns()
exec/parquet/hdfs-parquet-scanner.cc:1679:60
#3 HdfsParquetScanner::NextRowGroup()
exec/parquet/hdfs-parquet-scanner.cc:648:45
#4 HdfsParquetScanner::GetNextInternal(RowBatch*)
exec/parquet/hdfs-parquet-scanner.cc:437:45
#5 HdfsParquetScanner::ProcessSplit()
exec/parquet/hdfs-parquet-scanner.cc:353:21
#6 HdfsScanNode::ProcessSplit(vector<FilterContext> const&,
MemPool*, io::ScanRange*, long*) exec/hdfs-scan-node.cc:514:21
#7 HdfsScanNode::ScannerThread(bool, long)
exec/hdfs-scan-node.cc:415:7
#8 HdfsScanNode::ThreadTokenAvailableCb(ThreadResourcePool*)::
$_0::operator()() const exec/hdfs-scan-node.cc:337:13
Change-Id: I48090e8e0c6c6f18bb1ad3c32c1f5fbffc908844
---
M common/thrift/parquet.thrift
1 file changed, 22 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/13940/1
--
To view, visit http://gerrit.cloudera.org:8080/13940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I48090e8e0c6c6f18bb1ad3c32c1f5fbffc908844
Gerrit-Change-Number: 13940
Gerrit-PatchSet: 1
Gerrit-Owner: Jim Apple <jb...@apache.org>
[Impala-ASF-CR] IMPALA-5031: widen Thrift enum to placate UBSAN
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13940 )
Change subject: IMPALA-5031: widen Thrift enum to placate UBSAN
......................................................................
IMPALA-5031: widen Thrift enum to placate UBSAN
This fixes an instance of undefined behavior in the end-to-end tests
in which an enum value is outside of the allowable values for that
enum according to the C++14 standard.
Representative backtrace:
exec/parquet/parquet-metadata-utils.cc:293:26: runtime error: load of
value 49, which is not a valid value for type 'Type::type'
#0 ParquetMetadataUtils::ValidateRowGroupColumn(
parquet::FileMetaData const&, char const*, int, int,
parquet::SchemaElement const&, RuntimeState*)
exec/parquet/parquet-metadata-utils.cc:293:26
#1 BaseScalarColumnReader::Reset(HdfsFileDesc const&,
parquet::ColumnChunk const&, int)
exec/parquet/parquet-column-readers.cc:1077:43
#2 HdfsParquetScanner::InitScalarColumns()
exec/parquet/hdfs-parquet-scanner.cc:1679:60
#3 HdfsParquetScanner::NextRowGroup()
exec/parquet/hdfs-parquet-scanner.cc:648:45
#4 HdfsParquetScanner::GetNextInternal(RowBatch*)
exec/parquet/hdfs-parquet-scanner.cc:437:45
#5 HdfsParquetScanner::ProcessSplit()
exec/parquet/hdfs-parquet-scanner.cc:353:21
#6 HdfsScanNode::ProcessSplit(vector<FilterContext> const&,
MemPool*, io::ScanRange*, long*) exec/hdfs-scan-node.cc:514:21
#7 HdfsScanNode::ScannerThread(bool, long)
exec/hdfs-scan-node.cc:415:7
#8 HdfsScanNode::ThreadTokenAvailableCb(ThreadResourcePool*)::
$_0::operator()() const exec/hdfs-scan-node.cc:337:13
Change-Id: I48090e8e0c6c6f18bb1ad3c32c1f5fbffc908844
Reviewed-on: http://gerrit.cloudera.org:8080/13940
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M common/thrift/parquet.thrift
1 file changed, 22 insertions(+), 0 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/13940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I48090e8e0c6c6f18bb1ad3c32c1f5fbffc908844
Gerrit-Change-Number: 13940
Gerrit-PatchSet: 3
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-5031: widen Thrift enum to placate UBSAN
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13940 )
Change subject: IMPALA-5031: widen Thrift enum to placate UBSAN
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4905/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/13940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48090e8e0c6c6f18bb1ad3c32c1f5fbffc908844
Gerrit-Change-Number: 13940
Gerrit-PatchSet: 2
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Sep 2019 13:46:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: widen Thrift enum to placate UBSAN
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13940 )
Change subject: IMPALA-5031: widen Thrift enum to placate UBSAN
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48090e8e0c6c6f18bb1ad3c32c1f5fbffc908844
Gerrit-Change-Number: 13940
Gerrit-PatchSet: 2
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Sep 2019 17:49:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: widen Thrift enum to placate UBSAN
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/13940 )
Change subject: IMPALA-5031: widen Thrift enum to placate UBSAN
......................................................................
Patch Set 1:
> I'm a bit reluctant about diverging parquet.thrift from the
> upstream Apache Parquet thrift. Need to think about whether this is
> the right way.
We actually already deviated in the encoding. That kind of deviation or this one are the two ways that occurred to me to resolve issues like this.
Did you have any other particular method in mind?
--
To view, visit http://gerrit.cloudera.org:8080/13940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48090e8e0c6c6f18bb1ad3c32c1f5fbffc908844
Gerrit-Change-Number: 13940
Gerrit-PatchSet: 1
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 08 Sep 2019 18:32:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: widen Thrift enum to placate UBSAN
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13940 )
Change subject: IMPALA-5031: widen Thrift enum to placate UBSAN
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4049/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/13940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48090e8e0c6c6f18bb1ad3c32c1f5fbffc908844
Gerrit-Change-Number: 13940
Gerrit-PatchSet: 1
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 27 Jul 2019 19:49:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: widen Thrift enum to placate UBSAN
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13940 )
Change subject: IMPALA-5031: widen Thrift enum to placate UBSAN
......................................................................
Patch Set 1:
I'm a bit reluctant about diverging parquet.thrift from the upstream Apache Parquet thrift. Need to think about whether this is the right way.
--
To view, visit http://gerrit.cloudera.org:8080/13940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48090e8e0c6c6f18bb1ad3c32c1f5fbffc908844
Gerrit-Change-Number: 13940
Gerrit-PatchSet: 1
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jul 2019 04:11:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: widen Thrift enum to placate UBSAN
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13940 )
Change subject: IMPALA-5031: widen Thrift enum to placate UBSAN
......................................................................
Patch Set 1: Code-Review+2
Thanks for explaining, this seems fine then.
--
To view, visit http://gerrit.cloudera.org:8080/13940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48090e8e0c6c6f18bb1ad3c32c1f5fbffc908844
Gerrit-Change-Number: 13940
Gerrit-PatchSet: 1
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Sep 2019 13:44:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: widen Thrift enum to placate UBSAN
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13940 )
Change subject: IMPALA-5031: widen Thrift enum to placate UBSAN
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48090e8e0c6c6f18bb1ad3c32c1f5fbffc908844
Gerrit-Change-Number: 13940
Gerrit-PatchSet: 2
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Sep 2019 13:46:52 +0000
Gerrit-HasComments: No