You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org> on 2021/07/22 16:59:10 UTC
[Impala-ASF-CR] IMPALA-10814: Fix crash on illegal Parquet file
Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17714
Change subject: IMPALA-10814: Fix crash on illegal Parquet file
......................................................................
IMPALA-10814: Fix crash on illegal Parquet file
In IMPALA-7087/IMPALA-8131 we allowed reading decimals with
different precision/scale than table metadata. To allow this
we relaxed some checks against the Parquet file schema.
However we should still add some sanity checks, e.g. ignoring
negative values, require that scale is not greater than precision.
This patch adds these sanity checks. Without these checks we might
hit DCHECKS in Parquet fuzz testing.
Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
---
M be/src/exec/parquet/parquet-metadata-utils.cc
1 file changed, 8 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/17714/1
--
To view, visit http://gerrit.cloudera.org:8080/17714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
Gerrit-Change-Number: 17714
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10814: Fix crash on illegal Parquet file
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17714 )
Change subject: IMPALA-10814: Fix crash on illegal Parquet file
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17714/1/be/src/exec/parquet/parquet-metadata-utils.cc
File be/src/exec/parquet/parquet-metadata-utils.cc:
http://gerrit.cloudera.org:8080/#/c/17714/1/be/src/exec/parquet/parquet-metadata-utils.cc@349
PS1, Line 349: schema_element.precision
Can we add the precision in the message? User might be confused if (schema_element.precision < schema_element.scale <= slot_desc->type().precision), since they can't see the file precision directly, and the scale seems valid.
--
To view, visit http://gerrit.cloudera.org:8080/17714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
Gerrit-Change-Number: 17714
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 23 Jul 2021 02:18:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10814: Fix crash on illegal Parquet file
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17714 )
Change subject: IMPALA-10814: Fix crash on illegal Parquet file
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9151/ : 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/17714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
Gerrit-Change-Number: 17714
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 23 Jul 2021 08:41:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10814: Fix crash on illegal Parquet file
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17714 )
Change subject: IMPALA-10814: Fix crash on illegal Parquet file
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9145/ : 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/17714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
Gerrit-Change-Number: 17714
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Jul 2021 17:28:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10814: Fix crash on illegal Parquet file
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17714 )
Change subject: IMPALA-10814: Fix crash on illegal Parquet file
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/17714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
Gerrit-Change-Number: 17714
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 25 Jul 2021 09:20:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10814: Fix crash on illegal Parquet file
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17714 )
Change subject: IMPALA-10814: Fix crash on illegal Parquet file
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17714/1/be/src/exec/parquet/parquet-metadata-utils.cc
File be/src/exec/parquet/parquet-metadata-utils.cc:
http://gerrit.cloudera.org:8080/#/c/17714/1/be/src/exec/parquet/parquet-metadata-utils.cc@349
PS1, Line 349: schema_element.precision
> Can we add the precision in the message? User might be confused if (schema_
Sure, done.
--
To view, visit http://gerrit.cloudera.org:8080/17714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
Gerrit-Change-Number: 17714
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 23 Jul 2021 08:21:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10814: Fix crash on illegal Parquet file
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17714 )
Change subject: IMPALA-10814: Fix crash on illegal Parquet file
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
Gerrit-Change-Number: 17714
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 25 Jul 2021 03:02:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10814: Fix crash on illegal Parquet file
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17714 )
Change subject: IMPALA-10814: Fix crash on illegal Parquet file
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
Gerrit-Change-Number: 17714
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 25 Jul 2021 03:03:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10814: Fix crash on illegal Parquet file
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17714
to look at the new patch set (#2).
Change subject: IMPALA-10814: Fix crash on illegal Parquet file
......................................................................
IMPALA-10814: Fix crash on illegal Parquet file
In IMPALA-7087/IMPALA-8131 we allowed reading decimals with
different precision/scale than table metadata. To allow this
we relaxed some checks against the Parquet file schema.
However we should still add some sanity checks, e.g. ignoring
negative values, require that scale is not greater than precision.
This patch adds these sanity checks. Without these checks we might
hit DCHECKS in Parquet fuzz testing.
Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
---
M be/src/exec/parquet/parquet-metadata-utils.cc
1 file changed, 10 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/17714/2
--
To view, visit http://gerrit.cloudera.org:8080/17714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
Gerrit-Change-Number: 17714
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10814: Fix crash on illegal Parquet file
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/17714 )
Change subject: IMPALA-10814: Fix crash on illegal Parquet file
......................................................................
IMPALA-10814: Fix crash on illegal Parquet file
In IMPALA-7087/IMPALA-8131 we allowed reading decimals with
different precision/scale than table metadata. To allow this
we relaxed some checks against the Parquet file schema.
However we should still add some sanity checks, e.g. ignoring
negative values, require that scale is not greater than precision.
This patch adds these sanity checks. Without these checks we might
hit DCHECKS in Parquet fuzz testing.
Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
Reviewed-on: http://gerrit.cloudera.org:8080/17714
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/parquet/parquet-metadata-utils.cc
1 file changed, 10 insertions(+), 0 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/17714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
Gerrit-Change-Number: 17714
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10814: Fix crash on illegal Parquet file
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17714 )
Change subject: IMPALA-10814: Fix crash on illegal Parquet file
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7343/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/17714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6b06ac00e2af8f405f7b2b3c2eb952683821431
Gerrit-Change-Number: 17714
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 25 Jul 2021 03:03:06 +0000
Gerrit-HasComments: No