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 2023/05/22 16:47:21 UTC

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19911


Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................

IMPALA-12153: Parquet STRUCT reader should fill position slots

Before this patch the Parquet STRUCT reader didn't fill the
position slots: collection position, file position. When users
queried these virtual columns Impala was crashed or returned
incorrect results.

The ORC scanner already worked correctly, but there was no tests
written for it.

Test:
 * e2e tests for both ORC / Parquet

Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
---
M be/src/exec/parquet/parquet-struct-column-reader.cc
A testdata/workloads/functional-query/queries/QueryTest/struct-positions.test
M tests/query_test/test_nested_types.py
3 files changed, 132 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/19911/1
-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 3: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 17:40:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 4: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 13:36:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9339/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 12:12:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Gabor Kaszab, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19911

to look at the new patch set (#4).

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................

IMPALA-12153: Parquet STRUCT reader should fill position slots

Before this patch the Parquet STRUCT reader didn't fill the
position slots: collection position, file position. When users
queried these virtual columns Impala was crashed or returned
incorrect results.

The ORC scanner already worked correctly, but there was no tests
written for it.

Test:
 * e2e tests for both ORC / Parquet

Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
---
M be/src/exec/parquet/parquet-column-readers.cc
M be/src/exec/parquet/parquet-column-readers.h
M be/src/exec/parquet/parquet-struct-column-reader.cc
A testdata/workloads/functional-query/queries/QueryTest/struct-positions.test
M tests/query_test/test_nested_types.py
5 files changed, 225 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/19911/4
-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19911/1/testdata/workloads/functional-query/queries/QueryTest/struct-positions.test
File testdata/workloads/functional-query/queries/QueryTest/struct-positions.test:

http://gerrit.cloudera.org:8080/#/c/19911/1/testdata/workloads/functional-query/queries/QueryTest/struct-positions.test@18
PS1, Line 18: levle
> typo
Done


http://gerrit.cloudera.org:8080/#/c/19911/1/testdata/workloads/functional-query/queries/QueryTest/struct-positions.test@103
PS1, Line 103: select file__position, pos, item from complextypestbl c, c.nested_struct.c.d.item;
> Can you add a few tests that also have predicates? E.g. one with a predicat
Added a few new tests.

Also added tests with a different from clause, e.g.:

 FROM complextypestbl.nested_struct.c.d.item

which creates tuples differently, and uses the batched interfaces to read.



-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 10:09:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19911/3/be/src/exec/parquet/parquet-struct-column-reader.cc
File be/src/exec/parquet/parquet-struct-column-reader.cc:

http://gerrit.cloudera.org:8080/#/c/19911/3/be/src/exec/parquet/parquet-struct-column-reader.cc@104
PS3, Line 104: pos_slot_desc
> Can you add DCHECKs about not having pos_slot_desc and file_pos_slot_desc a
Done.

We also have this: https://github.com/apache/impala/blob/1b6011c6a08123bb15921add253074dca7b4d390/be/src/exec/parquet/hdfs-parquet-scanner.cc#LL2919C10-L2919C28



-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 13:17:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

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/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................

IMPALA-12153: Parquet STRUCT reader should fill position slots

Before this patch the Parquet STRUCT reader didn't fill the
position slots: collection position, file position. When users
queried these virtual columns Impala was crashed or returned
incorrect results.

The ORC scanner already worked correctly, but there was no tests
written for it.

Test:
 * e2e tests for both ORC / Parquet

Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Reviewed-on: http://gerrit.cloudera.org:8080/19911
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/parquet/parquet-column-readers.cc
M be/src/exec/parquet/parquet-column-readers.h
M be/src/exec/parquet/parquet-struct-column-reader.cc
A testdata/workloads/functional-query/queries/QueryTest/struct-positions.test
M tests/query_test/test_nested_types.py
5 files changed, 225 insertions(+), 12 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19911/3/be/src/exec/parquet/parquet-struct-column-reader.cc
File be/src/exec/parquet/parquet-struct-column-reader.cc:

http://gerrit.cloudera.org:8080/#/c/19911/3/be/src/exec/parquet/parquet-struct-column-reader.cc@104
PS3, Line 104: pos_slot_desc
Can you add DCHECKs about not having pos_slot_desc and file_pos_slot_desc at the same time?



-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 12:33:48 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Gabor Kaszab, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19911

to look at the new patch set (#3).

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................

IMPALA-12153: Parquet STRUCT reader should fill position slots

Before this patch the Parquet STRUCT reader didn't fill the
position slots: collection position, file position. When users
queried these virtual columns Impala was crashed or returned
incorrect results.

The ORC scanner already worked correctly, but there was no tests
written for it.

Test:
 * e2e tests for both ORC / Parquet

Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
---
M be/src/exec/parquet/parquet-column-readers.cc
M be/src/exec/parquet/parquet-column-readers.h
M be/src/exec/parquet/parquet-struct-column-reader.cc
A testdata/workloads/functional-query/queries/QueryTest/struct-positions.test
M tests/query_test/test_nested_types.py
5 files changed, 223 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/19911/3
-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13090/ : 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/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 May 2023 17:08:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13098/ : 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/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 10:28:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13099/ : 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/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 10:31:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 5: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 19:57:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9341/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 19:57:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 1: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19911/1/testdata/workloads/functional-query/queries/QueryTest/struct-positions.test
File testdata/workloads/functional-query/queries/QueryTest/struct-positions.test:

http://gerrit.cloudera.org:8080/#/c/19911/1/testdata/workloads/functional-query/queries/QueryTest/struct-positions.test@18
PS1, Line 18: levle
typo


http://gerrit.cloudera.org:8080/#/c/19911/1/testdata/workloads/functional-query/queries/QueryTest/struct-positions.test@103
PS1, Line 103: select file__position, pos, item from complextypestbl c, c.nested_struct.c.d.item;
Can you add a few tests that also have predicates? E.g. one with a predicates on pos/file_position and one on a member of item



-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 06:19:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13103/ : 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/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 13:37:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Gabor Kaszab, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19911

to look at the new patch set (#2).

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................

IMPALA-12153: Parquet STRUCT reader should fill position slots

Before this patch the Parquet STRUCT reader didn't fill the
position slots: collection position, file position. When users
queried these virtual columns Impala was crashed or returned
incorrect results.

The ORC scanner already worked correctly, but there was no tests
written for it.

Test:
 * e2e tests for both ORC / Parquet

Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
---
M be/src/exec/parquet/parquet-column-readers.cc
M be/src/exec/parquet/parquet-column-readers.h
M be/src/exec/parquet/parquet-struct-column-reader.cc
A testdata/workloads/functional-query/queries/QueryTest/struct-positions.test
M tests/query_test/test_nested_types.py
5 files changed, 223 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/19911/2
-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-12153: Parquet STRUCT reader should fill position slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19911 )

Change subject: IMPALA-12153: Parquet STRUCT reader should fill position slots
......................................................................


Patch Set 5: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/19911
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a808a11f4543cd404ed9f3958e9b4e971ca1f4
Gerrit-Change-Number: 19911
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 May 2023 01:18:55 +0000
Gerrit-HasComments: No