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/03/08 14:01:55 UTC

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21125


Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................

IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

This change adds support for querying MAP types from Iceberg Metadata
tables.

The 'IcebergMetadataScanner.ArrayScanner' java class is renamed to
'CollectionScanner' and extended to be able to handle maps. For arrays
the iteration returns the element as before, for maps it returns
'Map.Entry' objects.

Note that collections in the FROM clause are still not supported.

Testing:
- Added E2E tests in iceberg-metadata-tables.test.

Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
6 files changed, 361 insertions(+), 123 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 11: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 11
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2024 18:30:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/21125 )

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................

IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

This change adds support for querying MAP types from Iceberg Metadata
tables.

The 'IcebergMetadataScanner.ArrayScanner' java class is renamed to
'CollectionScanner' and extended to be able to handle maps. For arrays
the iteration returns the element as before, for maps it returns
'Map.Entry' objects.

Note that collections in the FROM clause are still not supported.

Testing:
- Added E2E tests in iceberg-metadata-tables.test.

Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
6 files changed, 400 insertions(+), 124 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/21125/10
-- 
To view, visit http://gerrit.cloudera.org:8080/21125
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 10
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 6:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Mar 2024 16:31:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 31 Mar 2024 22:53:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/21125 )

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................

IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

This change adds support for querying MAP types from Iceberg Metadata
tables.

The 'IcebergMetadataScanner.ArrayScanner' java class is renamed to
'CollectionScanner' and extended to be able to handle maps. For arrays
the iteration returns the element as before, for maps it returns
'Map.Entry' objects.

Note that collections in the FROM clause are still not supported.

Testing:
- Added E2E tests in iceberg-metadata-tables.test.

Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
6 files changed, 395 insertions(+), 123 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/21125/5
-- 
To view, visit http://gerrit.cloudera.org:8080/21125
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 11: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 11
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2024 13:24:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/21125/1/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h
File be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h:

http://gerrit.cloudera.org:8080/#/c/21125/1/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h@139
PS1, Line 139:   Status GetNextCollectionScannerItem(JNIEnv* env, const jobject& scanner, jobject* result)
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/21125/1/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
File be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/21125/1/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc@64
PS1, Line 64:       "(Ljava/util/List;)Lorg/apache/impala/util/IcebergMetadataScanner$CollectionScanner;",
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/21125/1/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc@68
PS1, Line 68:       "(Ljava/util/Map;)Lorg/apache/impala/util/IcebergMetadataScanner$CollectionScanner;",
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/21125/1/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc@220
PS1, Line 220: Status IcebergMetadataScanner::CreateArrayOrMapScanner(JNIEnv* env, const jobject &list_or_map,
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/21125/1/be/src/exec/iceberg-metadata/iceberg-row-reader.h
File be/src/exec/iceberg-metadata/iceberg-row-reader.h:

http://gerrit.cloudera.org:8080/#/c/21125/1/be/src/exec/iceberg-metadata/iceberg-row-reader.h@80
PS1, Line 80:   Status WriteSlot(JNIEnv* env, const jobject* struct_like_row, const jobject& accessed_value,
line too long (94 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Mar 2024 14:02:59 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 5:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Mar 2024 14:49:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................

IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

This change adds support for querying MAP types from Iceberg Metadata
tables.

The 'IcebergMetadataScanner.ArrayScanner' java class is renamed to
'CollectionScanner' and extended to be able to handle maps. For arrays
the iteration returns the element as before, for maps it returns
'Map.Entry' objects.

Note that collections in the FROM clause are still not supported.

Testing:
- Added E2E tests in iceberg-metadata-tables.test.

Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Reviewed-on: http://gerrit.cloudera.org:8080/21125
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
6 files changed, 400 insertions(+), 124 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 12
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 7:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 31 Mar 2024 23:16:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Mar 2024 14:56:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/21125 )

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................

IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

This change adds support for querying MAP types from Iceberg Metadata
tables.

The 'IcebergMetadataScanner.ArrayScanner' java class is renamed to
'CollectionScanner' and extended to be able to handle maps. For arrays
the iteration returns the element as before, for maps it returns
'Map.Entry' objects.

Note that collections in the FROM clause are still not supported.

Testing:
- Added E2E tests in iceberg-metadata-tables.test.

Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
6 files changed, 363 insertions(+), 123 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 6:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h
File be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h:

http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h@63
PS4, Line 63:   /// Note that it returns a GlobalRef, that has to be released explicitly.
> Probably I get this comment wrong, but I was searching for some code that r
It is used in IcebergRowReader::WriteCollectionSlot(), it is freed at the end of the function:
  env->DeleteLocalRef(collection_scanner);


http://gerrit.cloudera.org:8080/#/c/21125/3/be/src/exec/iceberg-metadata/iceberg-row-reader.cc
File be/src/exec/iceberg-metadata/iceberg-row-reader.cc:

http://gerrit.cloudera.org:8080/#/c/21125/3/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@123
PS3, Line 123:       // Skip the unsupported type and set it to NULL
> Does this set Binary cols NULL now?
It shouldn't, in the backend BINARY fields have TYPE_STRING and a separete boolean variable that determines whether it is actually a STRING or a BINARY. See https://github.com/apache/impala/blob/73171cb7164573349bd53a996a51bb7058b778e0/be/src/runtime/types.h#L216.

On the other hand, I don't know where top-level BINARY fields are NULLed for metadata tables.


http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-row-reader.cc
File be/src/exec/iceberg-metadata/iceberg-row-reader.cc:

http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@125
PS4, Line 125:       VLOG(3) << "Skipping unsupported column type: " << slot_desc->type().type;
> I think this 'accessed_value' is allocated one level above in the call chai
Done. Added the deallocation to the other caller, WriteArrayItem(), too.


http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@212
PS4, Line 212:   if constexpr (IS_ARRAY) {
> Let's consider moving these DCHECKs into the same IF-ELSE at L228-234.
If we do that and this function is erroneously called with for example an INT, the DCHECK that fires will be the one added for "item_tuple_desc != nullptr", not the one about the type. I think type info would help more with debugging than info about 'item_tuple_desc'. Also, this is a more basic precondition of the function.


http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@220
PS4, Line 220:   const TupleDescriptor* item_tuple_desc = slot_desc->children_tuple_descriptor();
> nit: DCHECK item_tuple_desc is not null?
Done


http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@236
PS4, Line 236: 
> name is misleading now that this is not just for arrays. remaining_items?
Done


http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@297
PS4, Line 297: jobject value;
             :   RETURN_IF_ERROR(met
> Can the key of a map be a struct?
Done, added a DCHECK instead.


http://gerrit.cloudera.org:8080/#/c/21125/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test:

http://gerrit.cloudera.org:8080/#/c/21125/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@761
PS3, Line 761: ====
> Can't we do a map_col.KEY and map_col.VALUE for maps? Would be nice to have
We can't, to access KEY and VALUE we have to cross-unnest the collection, and it is not supported yet. See https://issues.apache.org/jira/browse/IMPALA-12853.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Mar 2024 15:47:36 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 8:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 8
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2024 10:50:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 3:

(4 comments)

Thanks for the change, Dani! Frankly, reviewing these JNI changes isn't the easiest so I mostly focused on the tests so far. I'll keep reading the c++ implementation meanwhile.

http://gerrit.cloudera.org:8080/#/c/21125/3/fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
File fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java:

http://gerrit.cloudera.org:8080/#/c/21125/3/fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java@163
PS3, Line 163:         return iterator_.next();
nit: merge with line above


http://gerrit.cloudera.org:8080/#/c/21125/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test:

http://gerrit.cloudera.org:8080/#/c/21125/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@761
PS3, Line 761: ====
Checked the Iceberg-Spark page for metadata tables: https://iceberg.apache.org/docs/latest/spark-queries/#snapshots

This query seems interesting:
select
    h.made_current_at,
    s.operation,
    h.snapshot_id,
    h.is_current_ancestor,
    s.summary['spark.app.id']
from prod.db.table.history h
join prod.db.table.snapshots s
  on h.snapshot_id = s.snapshot_id
order by made_current_at;
Do you know if such a query is feasible now with our map implementation?


http://gerrit.cloudera.org:8080/#/c/21125/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@764
PS3, Line 764: # Describe all the metadata tables once
entries table seems to have embedded nested type cols. Could you try to add test coverage for them?


http://gerrit.cloudera.org:8080/#/c/21125/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@765
PS3, Line 765: ####
Can you add a test that queries different collections, arrays, maps?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Mar 2024 16:57:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/21125 )

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................

IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

This change adds support for querying MAP types from Iceberg Metadata
tables.

The 'IcebergMetadataScanner.ArrayScanner' java class is renamed to
'CollectionScanner' and extended to be able to handle maps. For arrays
the iteration returns the element as before, for maps it returns
'Map.Entry' objects.

Note that collections in the FROM clause are still not supported.

Testing:
- Added E2E tests in iceberg-metadata-tables.test.

Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
6 files changed, 401 insertions(+), 124 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/21125/6
-- 
To view, visit http://gerrit.cloudera.org:8080/21125
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 8:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 8
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2024 11:13:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/21125 )

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................

IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

This change adds support for querying MAP types from Iceberg Metadata
tables.

The 'IcebergMetadataScanner.ArrayScanner' java class is renamed to
'CollectionScanner' and extended to be able to handle maps. For arrays
the iteration returns the element as before, for maps it returns
'Map.Entry' objects.

Note that collections in the FROM clause are still not supported.

Testing:
- Added E2E tests in iceberg-metadata-tables.test.

Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
6 files changed, 363 insertions(+), 123 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Mar 2024 15:36:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 6: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10464/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 Mar 2024 05:42:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/21125 )

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................

IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

This change adds support for querying MAP types from Iceberg Metadata
tables.

The 'IcebergMetadataScanner.ArrayScanner' java class is renamed to
'CollectionScanner' and extended to be able to handle maps. For arrays
the iteration returns the element as before, for maps it returns
'Map.Entry' objects.

Note that collections in the FROM clause are still not supported.

Testing:
- Added E2E tests in iceberg-metadata-tables.test.

Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
6 files changed, 399 insertions(+), 124 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/21125/8
-- 
To view, visit http://gerrit.cloudera.org:8080/21125
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 8
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 10:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
File be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc@156
PS8, Line 156:   jobject map_entry;
> Shouldn't we release 'map_entry' at the end of this function?
Done


http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-row-reader.cc
File be/src/exec/iceberg-metadata/iceberg-row-reader.cc:

http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@270
PS8, Line 270: DeleteLocalRef
> Isn't 'collection_scanner' a GlobalRef? We call DeleteLocalRef here so I'm 
I checked the code and JNI doc again and I think it is actually a local ref and the comment was wrong.


http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@270
PS8, Line 270:   env->DeleteLocalRef(collection_scanner);
> I think we can leak memory if any of the RETURN_IF_ERROR or RETURN_IF_CANCE
I checked the code and JNI doc again and I think it is actually a local ref and the comment was wrong. I updated the comments in iceberg-metadata-scanner.h.

If the reference is indeed local, deleting it may not be as important. This is what the doc says about deleting local references (https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#:~:text=The%20JNI%20divides%20object%20references,until%20they%20are%20explicitly%20freed):


In most cases, the programmer should rely on the VM to free all local references after the native method returns. However, there are times when the programmer should explicitly free a local reference. Consider, for example, the following situations:
 - A native method accesses a large Java object, thereby creating a local reference to the Java object. The native method then performs additional computation before returning to the caller. The local reference to the large Java object will prevent the object from being garbage collected, even if the object is no longer used in the remainder of the computation.
 - A native method creates a large number of local references, although not all of them are used at the same time. Since the VM needs a certain amount of space to keep track of a local reference, creating too many local references may cause the system to run out of memory. For example, a native method loops through a large array of objects, retrieves the elements as local references, and operates on one element at each iteration. After each iteration, the programmer no longer needs the local reference to the array element.


I think Tamas added the deletes because of the second case. If an error occurs or the query is cancelled we won't create new (local) references, so freeing these local references is not important.

If you'd like to I am open to creating a wrapper for these refs though.


http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@288
PS8, Line 288:   env->DeleteLocalRef(item);
> Same comment about leaking memory
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 10
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2024 12:36:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 11:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 11
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2024 13:24:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 5:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h
File be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h:

http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h@63
PS4, Line 63:   /// Note that it returns a GlobalRef, that has to be released explicitly.
Probably I get this comment wrong, but I was searching for some code that releases whatever this function returns but didn't think anything relevant.


http://gerrit.cloudera.org:8080/#/c/21125/3/be/src/exec/iceberg-metadata/iceberg-row-reader.cc
File be/src/exec/iceberg-metadata/iceberg-row-reader.cc:

http://gerrit.cloudera.org:8080/#/c/21125/3/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@123
PS3, Line 123:       VLOG(3) << "Skipping unsupported column type: " << slot_desc->type().type;
Does this set Binary cols NULL now?


http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-row-reader.cc
File be/src/exec/iceberg-metadata/iceberg-row-reader.cc:

http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@125
PS4, Line 125:   env->DeleteLocalRef(accessed_value);
I think this 'accessed_value' is allocated one level above in the call chain, so I'd make that level responsible for the deallocation.


http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@212
PS4, Line 212:   if constexpr (IS_ARRAY) {
Let's consider moving these DCHECKs into the same IF-ELSE at L228-234.


http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@220
PS4, Line 220:   const TupleDescriptor* item_tuple_desc = slot_desc->children_tuple_descriptor();
nit: DCHECK item_tuple_desc is not null?


http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@236
PS4, Line 236: remaining_array_size
name is misleading now that this is not just for arrays. remaining_items?


http://gerrit.cloudera.org:8080/#/c/21125/4/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@297
PS4, Line 297: const jobject* key_struct_like_row = key_slot_desc->type().IsStructType()
             :     ? &key : nullptr;
Can the key of a map be a struct?


http://gerrit.cloudera.org:8080/#/c/21125/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test:

http://gerrit.cloudera.org:8080/#/c/21125/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@761
PS3, Line 761: ====
> Added this query (except for "['spark.app.id']").
Can't we do a map_col.KEY and map_col.VALUE for maps? Would be nice to have some test coverage for these if possible.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Mar 2024 15:01:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 4:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Mar 2024 13:44:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 Mar 2024 00:29:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 10: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 10
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2024 13:22:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21125/7/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test:

http://gerrit.cloudera.org:8080/#/c/21125/7/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@763
PS7, Line 763: select
> Removed h.made_current_at and h.snapshot_id from the select list because th
It seems that in s.summary, for the 'overwrite' column, "added-files-size" and "total-files-size" in the map vary by builds, trying a regex instead of a concrete value.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 8
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2024 10:50:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 8:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
File be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc@156
PS8, Line 156:   jobject map_entry;
Shouldn't we release 'map_entry' at the end of this function?


http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-row-reader.cc
File be/src/exec/iceberg-metadata/iceberg-row-reader.cc:

http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@270
PS8, Line 270:   env->DeleteLocalRef(collection_scanner);
I think we can leak memory if any of the RETURN_IF_ERROR or RETURN_IF_CANCELLED returns from the function. Would it be possible to wrap these globalrefs into some custom object that we write and then we can release the memory in the desctructor?


http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@270
PS8, Line 270: DeleteLocalRef
Isn't 'collection_scanner' a GlobalRef? We call DeleteLocalRef here so I'm a bit confused :)


http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@288
PS8, Line 288:   env->DeleteLocalRef(item);
Same comment about leaking memory



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 8
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2024 11:35:08 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 8
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2024 15:50:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/21125 )

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................

IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

This change adds support for querying MAP types from Iceberg Metadata
tables.

The 'IcebergMetadataScanner.ArrayScanner' java class is renamed to
'CollectionScanner' and extended to be able to handle maps. For arrays
the iteration returns the element as before, for maps it returns
'Map.Entry' objects.

Note that collections in the FROM clause are still not supported.

Testing:
- Added E2E tests in iceberg-metadata-tables.test.

Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
6 files changed, 395 insertions(+), 123 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Mar 2024 14:27:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 10:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 10
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2024 12:59:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 7: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10475/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Apr 2024 03:54:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/21125 )

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................

IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

This change adds support for querying MAP types from Iceberg Metadata
tables.

The 'IcebergMetadataScanner.ArrayScanner' java class is renamed to
'CollectionScanner' and extended to be able to handle maps. For arrays
the iteration returns the element as before, for maps it returns
'Map.Entry' objects.

Note that collections in the FROM clause are still not supported.

Testing:
- Added E2E tests in iceberg-metadata-tables.test.

Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
6 files changed, 399 insertions(+), 124 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/21125/7
-- 
To view, visit http://gerrit.cloudera.org:8080/21125
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12611: Add support to MAP type Iceberg Metadata table columns

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

Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21125/7/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test:

http://gerrit.cloudera.org:8080/#/c/21125/7/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@763
PS7, Line 763: select
Removed h.made_current_at and h.snapshot_id from the select list because the values change with every data load.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a
Gerrit-Change-Number: 21125
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker <da...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 31 Mar 2024 22:53:29 +0000
Gerrit-HasComments: Yes