You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Daniel Becker (Code Review)" <ge...@cloudera.org> on 2024/02/05 09:45:59 UTC
[Impala-ASF-CR] IMPALA-12783: Nested struct with varlen data crashes
Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20997
Change subject: IMPALA-12783: Nested struct with varlen data crashes
......................................................................
IMPALA-12783: Nested struct with varlen data crashes
If a struct ("main") is within an array and contains two child structs
("s1" ans "s2") which both contain strings (or other varlen data),
Impala crashes when this struct is re-materialised (for example in a
sort with limit) if codegen is enabled.
To reproduce:
In Hive:
create table nested (arr ARRAY<STRUCT<s1: STRUCT<str1: STRING>, s2:
STRUCT<str2: STRING>>>) stored as parquet;
insert into nested values (array( named_struct("s1",
named_struct("str1", "A string that is long"), "s2",
named_struct("str2", "Another string that is long") )));
In Impala:
select 1, arr from nested order by 1 limit 1;
This is because in the codegen'd code, when checking if the strings
("str1" and "str2" in the example) are NULL, we incorrectly calculate
the offset of their null indicator bytes from the memory address of
their containing struct, not from the beginning of the "master tuple",
which in this case is the item tuple of the array.
Note that the null indicators of struct members are always at the end of
the tuple containing the struct (recursively), i.e. the master tuple.
This change corrects the behaviour, passing the master tuple to
functions that need it.
Testing:
- extended the column 'arr_contains_nested_struct' in table
'collection_struct_mix' to include two nested structs with string
members. Updated existing queries, which now cover the problem.
Change-Id: Ide2b63f8b18633f38fbe939a17db923606ccb101
---
M be/src/runtime/descriptors.cc
M be/src/runtime/descriptors.h
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-query/queries/QueryTest/mixed-collections-and-structs.test
M testdata/workloads/functional-query/queries/QueryTest/sort-complex.test
M testdata/workloads/functional-query/queries/QueryTest/top-n-complex.test
6 files changed, 85 insertions(+), 75 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/20997/1
--
To view, visit http://gerrit.cloudera.org:8080/20997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ide2b63f8b18633f38fbe939a17db923606ccb101
Gerrit-Change-Number: 20997
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
[Impala-ASF-CR] IMPALA-12783: Nested struct with varlen data crashes
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20997 )
Change subject: IMPALA-12783: Nested struct with varlen data crashes
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15167/ : 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/20997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide2b63f8b18633f38fbe939a17db923606ccb101
Gerrit-Change-Number: 20997
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Feb 2024 10:11:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12783: Nested struct with varlen data crashes
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20997 )
Change subject: IMPALA-12783: Nested struct with varlen data crashes
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide2b63f8b18633f38fbe939a17db923606ccb101
Gerrit-Change-Number: 20997
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Feb 2024 15:20:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12783: Nested struct with varlen data crashes
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20997 )
Change subject: IMPALA-12783: Nested struct with varlen data crashes
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15170/ : 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/20997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide2b63f8b18633f38fbe939a17db923606ccb101
Gerrit-Change-Number: 20997
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Feb 2024 15:46:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12783: Nested struct with varlen data crashes
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20997 )
Change subject: IMPALA-12783: Nested struct with varlen data crashes
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10239/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide2b63f8b18633f38fbe939a17db923606ccb101
Gerrit-Change-Number: 20997
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Feb 2024 15:20:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12783: Nested struct with varlen data crashes
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20997 )
Change subject: IMPALA-12783: Nested struct with varlen data crashes
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide2b63f8b18633f38fbe939a17db923606ccb101
Gerrit-Change-Number: 20997
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Feb 2024 19:49:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12783: Nested struct with varlen data crashes
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20997 )
Change subject: IMPALA-12783: Nested struct with varlen data crashes
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20997/1/be/src/runtime/descriptors.h
File be/src/runtime/descriptors.h:
http://gerrit.cloudera.org:8080/#/c/20997/1/be/src/runtime/descriptors.h@308
PS1, Line 308: // 'master_tuple' is the first non-struct parent tuple. Applies to the below functions too.
line too long (93 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/20997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide2b63f8b18633f38fbe939a17db923606ccb101
Gerrit-Change-Number: 20997
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Feb 2024 09:46:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12783: Nested struct with varlen data crashes
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20997 )
Change subject: IMPALA-12783: Nested struct with varlen data crashes
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide2b63f8b18633f38fbe939a17db923606ccb101
Gerrit-Change-Number: 20997
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Feb 2024 13:17:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12783: Nested struct with varlen data crashes
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/20997 )
Change subject: IMPALA-12783: Nested struct with varlen data crashes
......................................................................
IMPALA-12783: Nested struct with varlen data crashes
If a struct ("main") is within an array and contains two child structs
("s1" ans "s2") which both contain strings (or other varlen data),
Impala crashes when this struct is re-materialised (for example in a
sort with limit) if codegen is enabled.
To reproduce:
In Hive:
create table nested (arr ARRAY<STRUCT<s1: STRUCT<str1: STRING>, s2:
STRUCT<str2: STRING>>>) stored as parquet;
insert into nested values (array( named_struct("s1",
named_struct("str1", "A string that is long"), "s2",
named_struct("str2", "Another string that is long") )));
In Impala:
select 1, arr from nested order by 1 limit 1;
This is because in the codegen'd code, when checking if the strings
("str1" and "str2" in the example) are NULL, we incorrectly calculate
the offset of their null indicator bytes from the memory address of
their containing struct, not from the beginning of the "master tuple",
which in this case is the item tuple of the array.
Note that the null indicators of struct members are always at the end of
the tuple containing the struct (recursively), i.e. the master tuple.
This change corrects the behaviour, passing the master tuple to
functions that need it.
Testing:
- extended the column 'arr_contains_nested_struct' in table
'collection_struct_mix' to include two nested structs with string
members. Updated existing queries, which now cover the problem.
Change-Id: Ide2b63f8b18633f38fbe939a17db923606ccb101
---
M be/src/runtime/descriptors.cc
M be/src/runtime/descriptors.h
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-query/queries/QueryTest/mixed-collections-and-structs.test
M testdata/workloads/functional-query/queries/QueryTest/sort-complex.test
M testdata/workloads/functional-query/queries/QueryTest/top-n-complex.test
6 files changed, 86 insertions(+), 75 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/20997/2
--
To view, visit http://gerrit.cloudera.org:8080/20997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide2b63f8b18633f38fbe939a17db923606ccb101
Gerrit-Change-Number: 20997
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
[Impala-ASF-CR] IMPALA-12783: Nested struct with varlen data crashes
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/20997 )
Change subject: IMPALA-12783: Nested struct with varlen data crashes
......................................................................
IMPALA-12783: Nested struct with varlen data crashes
If a struct ("main") is within an array and contains two child structs
("s1" ans "s2") which both contain strings (or other varlen data),
Impala crashes when this struct is re-materialised (for example in a
sort with limit) if codegen is enabled.
To reproduce:
In Hive:
create table nested (arr ARRAY<STRUCT<s1: STRUCT<str1: STRING>, s2:
STRUCT<str2: STRING>>>) stored as parquet;
insert into nested values (array( named_struct("s1",
named_struct("str1", "A string that is long"), "s2",
named_struct("str2", "Another string that is long") )));
In Impala:
select 1, arr from nested order by 1 limit 1;
This is because in the codegen'd code, when checking if the strings
("str1" and "str2" in the example) are NULL, we incorrectly calculate
the offset of their null indicator bytes from the memory address of
their containing struct, not from the beginning of the "master tuple",
which in this case is the item tuple of the array.
Note that the null indicators of struct members are always at the end of
the tuple containing the struct (recursively), i.e. the master tuple.
This change corrects the behaviour, passing the master tuple to
functions that need it.
Testing:
- extended the column 'arr_contains_nested_struct' in table
'collection_struct_mix' to include two nested structs with string
members. Updated existing queries, which now cover the problem.
Change-Id: Ide2b63f8b18633f38fbe939a17db923606ccb101
Reviewed-on: http://gerrit.cloudera.org:8080/20997
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/descriptors.cc
M be/src/runtime/descriptors.h
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-query/queries/QueryTest/mixed-collections-and-structs.test
M testdata/workloads/functional-query/queries/QueryTest/sort-complex.test
M testdata/workloads/functional-query/queries/QueryTest/top-n-complex.test
6 files changed, 86 insertions(+), 75 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/20997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ide2b63f8b18633f38fbe939a17db923606ccb101
Gerrit-Change-Number: 20997
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>