[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

2024-03-01 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21061 )

Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg 
Metadata tables
..


Patch Set 11:

I have not figured out the reason behind the clang-tidy failures yet:
The fail is:
```
FAILED: Unified backend test executable returned an error when trying
to list tests.
Command: /home/ubuntu/Impala/bin/run-binary.sh 
/home/ubuntu/Impala/be/build/release//service/unifiedbetests --gtest_list_tests
Return Code: -11
```
Which suggests that the unifiedbetests failed with SIGSEGV.

As PS8 was fine, I narrowed down the issue to one commit:
```
commit 9b8e43e9a1ae871e676cded8e9eddc90aa96f314
IMPALA-12426: QueryStateRecord Refactor
```
Looks like this these changes are colliding, but I do not see why ATM.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
Gerrit-Change-Number: 21061
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 01 Mar 2024 16:12:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

2024-02-29 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21061 )

Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg 
Metadata tables
..


Patch Set 10:

Carrying over +2 after rebase.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
Gerrit-Change-Number: 21061
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 29 Feb 2024 22:50:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

2024-02-29 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#10). ( 
http://gerrit.cloudera.org:8080/21061 )

Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg 
Metadata tables
..

IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

This commit adds support for reading ARRAY columns inside Iceberg
Metadata tables.

The change starts with some refactoring, to consolidate accessing JVM
through JNI a new backend class was introduced, IcebergMetadataScanner.
This class is the C++ part of the Java IcebergMetadataScanner, it is
responsible to manage the Java scanner object.

In Iceberg array types do not have accessors, so structs inside arrays
have to be accessed by position, for the value obtaining logics have been
changed to allow access by position.

The IcebergRowReader needed an IcebergMetadataScanner, so that it can
iterate over the arrays returned by the scanner and add them to the
collection.

This change will not cover MAP, these slots are set to NULL, it will
be done in IMPALA-12611.

Testing:
 - Added E2E tests.

Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
---
M be/src/exec/iceberg-metadata/CMakeLists.txt
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
A 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 be/src/service/impalad-main.cc
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
11 files changed, 734 insertions(+), 292 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
Gerrit-Change-Number: 21061
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

2024-02-29 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21061 )

Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg 
Metadata tables
..


Patch Set 7:

(3 comments)

Thanks again for the swift reviews!

http://gerrit.cloudera.org:8080/#/c/21061/7/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/21061/7/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@250
PS7, Line 250:   VLOG(3) << "Skipping unsupported column type: " << 
type.type;
> ahh, I just noticed that we just write a log line if there is an unsupporte
It is about skipping the resolution currently and making these slots NULL 
later. It cannot be skipped if a DCHECK catches it.


http://gerrit.cloudera.org:8080/#/c/21061/6/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/21061/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@715
PS6, Line 715: # Describe all the metadata tables once
> Ok, I'm going to add support for MAPs probably soon anyway. But please ment
Done


http://gerrit.cloudera.org:8080/#/c/21061/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@715
PS6, Line 715: # Describe all the metadata tables once
> Expand complex types would come after we have MAP support anyway. So I stil
Keeping it as NULL has already been approved by you and Zoltan a few months 
ago, given that MAP support is around the corner I would just leave it as is.

reference: https://gerrit.cloudera.org/#/c/20010/



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
Gerrit-Change-Number: 21061
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 29 Feb 2024 15:23:24 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

2024-02-29 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/21061 )

Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg 
Metadata tables
..

IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

This commit adds support for reading ARRAY columns inside Iceberg
Metadata tables.

The change starts with some refactoring, to consolidate accessing JVM
through JNI a new backend class was introduced, IcebergMetadataScanner.
This class is the C++ part of the Java IcebergMetadataScanner, it is
responsible to manage the Java scanner object.

In Iceberg array types do not have accessors, so structs inside arrays
have to be accessed by position, for the value obtaining logics have been
changed to allow access by position.

The IcebergRowReader needed an IcebergMetadataScanner, so that it can
iterate over the arrays returned by the scanner and add them to the
collection.

This change will not cover MAP, these slots are set to NULL, it will
be done in IMPALA-12611.

Testing:
 - Added E2E tests.

Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
---
M be/src/exec/iceberg-metadata/CMakeLists.txt
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
A 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 be/src/service/impalad-main.cc
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
12 files changed, 734 insertions(+), 293 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
Gerrit-Change-Number: 21061
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

2024-02-29 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/21061 )

Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg 
Metadata tables
..

IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

This commit adds support for reading ARRAY columns inside Iceberg
Metadata tables.

The change starts with some refactoring, to consolidate accessing JVM
through JNI a new backend class was introduced, IcebergMetadataScanner.
This class is the C++ part of the Java IcebergMetadataScanner, it is
responsible to manage the Java scanner object.

In Iceberg array types do not have accessors, so structs inside arrays
have to be accessed by position, for the value obtaining logics have been
changed to allow access by position.

The IcebergRowReader needed an IcebergMetadataScanner, so that it can
iterate over the arrays returned by the scanner and add them to the
collection.

Testing:
 - Added E2E tests.

Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
---
M be/src/exec/iceberg-metadata/CMakeLists.txt
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
A 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 be/src/service/impalad-main.cc
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
12 files changed, 735 insertions(+), 293 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
Gerrit-Change-Number: 21061
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

2024-02-29 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/21061 )

Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg 
Metadata tables
..

IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

This commit adds support for reading ARRAY columns inside Iceberg
Metadata tables.

The change starts with some refactoring, to consolidate accessing JVM
through JNI a new backend class was introduced, IcebergMetadataScanner.
This class is the C++ part of the Java IcebergMetadataScanner, it is
responsible to manage the Java scanner object.

In Iceberg array types do not have accessors, so structs inside arrays
have to be accessed by position, for the alue obtaining logics have been
changed to allow access by position.

The IcebergRowReader needed an IcebergMetadataScanner, so that it can
iterate over the arrays returned by the scanner and add them to the
collection.

Testing:
 - Added E2E tests.

Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
---
M be/src/exec/iceberg-metadata/CMakeLists.txt
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
A 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 be/src/service/impalad-main.cc
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
12 files changed, 746 insertions(+), 293 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
Gerrit-Change-Number: 21061
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

2024-02-29 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/21061 )

Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg 
Metadata tables
..

IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

This commit adds support for reading ARRAY columns inside Iceberg
Metadata tables.

The change starts with some refactoring, to consolidate accessing JVM
through JNI a new backend class was introduced, IcebergMetadataScanner.
This class is the C++ part of the Java IcebergMetadataScanner, it is
responsible to manage the Java scanner object.

In Iceberg array types do not have accessors, so structs inside arrays
have to be accessed by position, for the alue obtaining logics have been
changed to allow access by position.

The IcebergRowReader needed an IcebergMetadataScanner, so that it can
iterate over the arrays returned by the scanner and add them to the
collection.

Testing:
 - Added E2E tests.

Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
---
M be/src/exec/iceberg-metadata/CMakeLists.txt
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
A 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 be/src/service/impalad-main.cc
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
12 files changed, 746 insertions(+), 293 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
Gerrit-Change-Number: 21061
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

2024-02-28 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21061 )

Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg 
Metadata tables
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21061/3/fe/src/main/java/org/apache/impala/analysis/FromClause.java
File fe/src/main/java/org/apache/impala/analysis/FromClause.java:

http://gerrit.cloudera.org:8080/#/c/21061/3/fe/src/main/java/org/apache/impala/analysis/FromClause.java@169
PS3, Line 169: tblRef.getDesc().getType().isMapType()) {
> I just realised that this change won't cover collections inside from clause
Adding Jira:  IMPALA-12853



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
Gerrit-Change-Number: 21061
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 28 Feb 2024 15:22:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

2024-02-23 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/21061 )

Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg 
Metadata tables
..

IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

This commit adds support for reading ARRAY columns inside Iceberg
Metadata tables.

The change starts with some refactoring, to consolidate accessing JVM
through JNI a new backend class was introduced IcebergMetadataScanner.
This class is the C++ part of the Java IcebergMetadataScanner, it is
responsible to manage the Java scanner object.

In Iceberg array types do not have accessors, so structs inside arrays
have to be accessed by position, for the the value obtaining logics have
been changed to allow access by position.

The IcebergRowReader needed an IcebergMetadataScanner, so that it can
iterate over the arrays returned by the scanner and add them to the
collection.

Testing:
 - Added E2E tests.

Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
---
M be/src/exec/iceberg-metadata/CMakeLists.txt
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
A 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 be/src/service/impalad-main.cc
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
12 files changed, 621 insertions(+), 238 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
Gerrit-Change-Number: 21061
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

2024-02-23 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/21061 )

Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg 
Metadata tables
..

IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

This commit adds support for reading ARRAY columns inside Iceberg
Metadata tables.

The change starts with some refactoring, to consolidate accessing JVM
through JNI a new backend class was introduced IcebergMetadataScanner.
This class is the C++ part of the Java IcebergMetadataScanner, it is
responsible to manage the Java scanner object.

In Iceberg array types do not have accessors, so structs inside arrays
have to be accessed by position, for the the value obtaining logics have
been changed to allow access by position.

The IcebergRowReader needed an IcebergMetadataScanner, so that it can
iterate over the arrays returned by the scanner and add them to the
collection.

Testing:
 - Added E2E tests.

Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
---
M be/src/exec/iceberg-metadata/CMakeLists.txt
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
A 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 be/src/service/impalad-main.cc
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
12 files changed, 622 insertions(+), 238 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
Gerrit-Change-Number: 21061
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

2024-02-23 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/21061


Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg 
Metadata tables
..

IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables

This commit adds support for reading ARRAY columns inside Iceberg
Metadata tables.

The change starts with some refactoring, to consolidate accessing JVM
through JNI a new backend class was introduced IcebergMetadataScanner.
This class is the C++ part of the Java IcebergMetadataScanner, it is
responsible to manage the Java scanner object.

In Iceberg array types do not have accessors, so structs inside arrays
have to be accessed by position, for the the value obtaining logics have
been changed to allow access by position.

The IcebergRowReader needed an IcebergMetadataScanner, so that it can
iterate over the arrays returned by the scanner and add them to the
collection.

Testing:
 - Added E2E tests.

Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
---
M be/src/exec/iceberg-metadata/CMakeLists.txt
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc
A 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 be/src/service/impalad-main.cc
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
12 files changed, 621 insertions(+), 238 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
Gerrit-Change-Number: 21061
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate 


[Impala-ASF-CR] IMPALA-12787: Concurrent DELETE and UPDATE operations on Iceberg tables can be problematic

2024-02-05 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20999 )

Change subject: IMPALA-12787: Concurrent DELETE and UPDATE operations on 
Iceberg tables can be problematic
..


Patch Set 1: Code-Review+1

(1 comment)

Thanks for this fix Zoltan! LGTM!

http://gerrit.cloudera.org:8080/#/c/20999/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/20999/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@372
PS1, Line 372:   //added in the meantime, they potentially contain records 
that should have been
nit: space



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e581ea17fa8f6ccd9c87aaad1281bb694079f6e
Gerrit-Change-Number: 20999
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Mon, 05 Feb 2024 13:21:32 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12764: Fix Iceberg metadata table scan's LIMIT clause

2024-02-02 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/20968 )

Change subject: IMPALA-12764: Fix Iceberg metadata table scan's LIMIT clause
..

IMPALA-12764: Fix Iceberg metadata table scan's LIMIT clause

Earlier the ReachedLimit() condition was not evaluated during
IcebergMetadataScanNode::GetNext(). This missing condition has been
added and LIMIT is now being evaluated.

Testing:
 - Added E2E test

Change-Id: Iea73716fe475c8b063235d2ae971a4074b8e2a20
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
2 files changed, 24 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iea73716fe475c8b063235d2ae971a4074b8e2a20
Gerrit-Change-Number: 20968
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zihao Ye 


[Impala-ASF-CR] IMPALA-12072: Include snapshot id of Iceberg tables in query plan / profile

2024-02-01 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20204 )

Change subject: IMPALA-12072: Include snapshot id of Iceberg tables in query 
plan / profile
..


Patch Set 11: Code-Review+1

(2 comments)

Thanks for the clarifications Zoltan. LGTM!

http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:

http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@132
PS9, Line 132: 0
nit: in Impala I have found -1s as invalid snapshot ids.


http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@145
PS9, Line 145: IcebergUtil.getSnapshotId(getIceTable(), 
tblRef_.getTimeTravelSpec());
> An IcebergScanPlanner can create up to 3 IcebergScanNode objects, so we can
Thanks for the explanation Zoltan, it totally makes sense to keep it as is.

My intent with the comment was to keep the constructor light if possible. In 
case someone in the future has to instantiate an IcebergScanNode in a place 
where it is difficult to obtain.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee0b4967429ea733729ad8e44df32e3b24b88525
Gerrit-Change-Number: 20204
Gerrit-PatchSet: 11
Gerrit-Owner: Gergely Fürnstáhl 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 01 Feb 2024 18:41:58 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12072: Include snapshot id of Iceberg tables in query plan / profile

2024-02-01 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20204 )

Change subject: IMPALA-12072: Include snapshot id of Iceberg tables in query 
plan / profile
..


Patch Set 9:

(3 comments)

Thanks for reviving this change Zoltan.
Could you check if my idea to simplify the code makes sense?

http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@74
PS9, Line 74:   private long snapshotId_;
This could be final.


http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@277
PS9, Line 277:
nit: indentation


http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:

http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@145
PS9, Line 145: IcebergUtil.getSnapshotId(getIceTable(), 
tblRef_.getTimeTravelSpec());
Shouldn't we put this inside the IcebergScanNode's constructor? I think we 
could avoid passing it to every object because the TableRef is passed to the 
ScanNode, which holds a reference to the IcebergTable object.

We might not even need the getSnapshotId() method.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee0b4967429ea733729ad8e44df32e3b24b88525
Gerrit-Change-Number: 20204
Gerrit-PatchSet: 9
Gerrit-Owner: Gergely Fürnstáhl 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 01 Feb 2024 11:39:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12764: Fix Iceberg metadata table scan's LIMIT clause

2024-01-29 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20968


Change subject: IMPALA-12764: Fix Iceberg metadata table scan's LIMIT clause
..

IMPALA-12764: Fix Iceberg metadata table scan's LIMIT clause

Earlier the ReachedLimit() condition was not evaluated during
IcebergMetadataScanNode::GetNext(). This missing condition has been
added and LIMIT is now being evaluated.

Testing:
 - Added E2E test

Change-Id: Iea73716fe475c8b063235d2ae971a4074b8e2a20
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
2 files changed, 13 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iea73716fe475c8b063235d2ae971a4074b8e2a20
Gerrit-Change-Number: 20968
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate 


[Impala-ASF-CR] IMPALA-12598: Allow multiple equality filed id lists for Iceberg tables

2024-01-26 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20951 )

Change subject: IMPALA-12598: Allow multiple equality filed id lists for 
Iceberg tables
..


Patch Set 3:

(6 comments)

Hi Gabor, thank you for the feature, it looks great.
Marked some nits and I have a question, what was the reason behind moving from 
Thrift to FlatBuffer?

http://gerrit.cloudera.org:8080/#/c/20951/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20951/3//COMMIT_MSG@7
PS3, Line 7: d
nit: d


http://gerrit.cloudera.org:8080/#/c/20951/3//COMMIT_MSG@9
PS3, Line 9: has
nit: have
It refers to the plural object.


http://gerrit.cloudera.org:8080/#/c/20951/3/common/fbs/IcebergObjects.fbs
File common/fbs/IcebergObjects.fbs:

http://gerrit.cloudera.org:8080/#/c/20951/3/common/fbs/IcebergObjects.fbs@53
PS3, Line 53: equality_field_ids
I saw this in multiple places, the new term became equality_fiel_ids, in many 
places it was left as equality_ids.
I will mark some and I think we should use one of them to avoid confusion and 
to be consistent. Iceberg uses equality_ids to refer to equality field ids.
https://iceberg.apache.org/spec/


http://gerrit.cloudera.org:8080/#/c/20951/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:

http://gerrit.cloudera.org:8080/#/c/20951/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@126
PS3, Line 126:   private Set allEqualityFieldIds_ = new HashSet<>();
 :   private Map, Set> 
equalityIdsToDeleteFiles_ =
 :   new HashMap<>();
nit: Here we use both equality ids and equality field ids.


http://gerrit.cloudera.org:8080/#/c/20951/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@510
PS3, Line 510: d
nit: D


http://gerrit.cloudera.org:8080/#/c/20951/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-read-equality-deletes.test
File 
testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-read-equality-deletes.test:

http://gerrit.cloudera.org:8080/#/c/20951/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-read-equality-deletes.test@114
PS3, Line 114: filed
nit: field



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3e52d7a5800bf1b479f0c234679be92442d09f79
Gerrit-Change-Number: 20951
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 26 Jan 2024 10:33:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12590: Fix dmesg call during precommit for Ubuntu 20.04

2024-01-18 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20763 )

Change subject: IMPALA-12590: Fix dmesg call during precommit for Ubuntu 20.04
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic20193740c6e5cb9e8e155c03bede55184875de5
Gerrit-Change-Number: 20763
Gerrit-PatchSet: 1
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Thu, 18 Jan 2024 15:46:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12708: An UPDATE creates 2 new snapshots in Iceberg tables

2024-01-18 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20903 )

Change subject: IMPALA-12708: An UPDATE creates 2 new snapshots in Iceberg 
tables
..


Patch Set 2: Code-Review+2

(4 comments)

Thank you for the update Zoltan!

Marked some nits, but LGTM!

http://gerrit.cloudera.org:8080/#/c/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@361
PS2, Line 361: public
nit: this could be private for now


http://gerrit.cloudera.org:8080/#/c/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@373
PS2, Line 373: public
nit: this could be private for now


http://gerrit.cloudera.org:8080/#/c/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@384
PS2, Line 384:
nit: indentation


http://gerrit.cloudera.org:8080/#/c/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@393
PS2, Line 393:   rowDelta.validateFromSnapshot(initialSnapshotId);
 :   rowDelta.validateNoConflictingDataFiles();
 :   rowDelta.commit();
nit: this could be chained



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ceb80b939c644388707b21061bf55451234dcd3
Gerrit-Change-Number: 20903
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Noemi Pap-Takacs 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 18 Jan 2024 15:35:44 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-11710: Table properties are not updated in Iceberg metadata files

2024-01-17 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20907 )

Change subject: IMPALA-11710: Table properties are not updated in Iceberg 
metadata files
..


Patch Set 2: Code-Review+2

LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a82d022534e1e212d542fbd7916ae033c381c20
Gerrit-Change-Number: 20907
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Noemi Pap-Takacs 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 17 Jan 2024 13:55:49 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-11710: Table properties are not updated in Iceberg metadata files

2024-01-16 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20907 )

Change subject: IMPALA-11710: Table properties are not updated in Iceberg 
metadata files
..


Patch Set 1: Code-Review+1

Nice catch, LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a82d022534e1e212d542fbd7916ae033c381c20
Gerrit-Change-Number: 20907
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Tue, 16 Jan 2024 22:15:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12708: An UPDATE creates 2 new snapshots in Iceberg tables

2024-01-16 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20903 )

Change subject: IMPALA-12708: An UPDATE creates 2 new snapshots in Iceberg 
tables
..


Patch Set 1:

(2 comments)

Thanks for this fix Zoltan, LGTM!

I left a suggestion on the modifyRows method, this could be just a matter of 
preference.

http://gerrit.cloudera.org:8080/#/c/20903/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/20903/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@354
PS1, Line 354:   case DELETE:
 :   case UPDATE: modifyRows(feIcebergTable, txn, icebergOp); 
break;
The two operations that modifyRows manages with the two conditions makes it a 
bit difficult to read. I think it would be more readable to keep these two 
operations separate. ie.: deleteRows/updateRows.


http://gerrit.cloudera.org:8080/#/c/20903/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@379
PS1, Line 379: if (dataFilesFb != null) {
Following up on the previous comment, if I am not mistaken this can only happen 
with UPDATEs.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ceb80b939c644388707b21061bf55451234dcd3
Gerrit-Change-Number: 20903
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Noemi Pap-Takacs 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Tue, 16 Jan 2024 09:49:52 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12714: Fix test reduced cardinality by filter for non-HDFS

2024-01-15 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20902 )

Change subject: IMPALA-12714: Fix test_reduced_cardinality_by_filter for 
non-HDFS
..


Patch Set 1: Code-Review+2

Thanks for the quick fix Riza! LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icbf72687cc3c5a99aa0a0a74e229ed8c88ed06ef
Gerrit-Change-Number: 20902
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Mon, 15 Jan 2024 17:13:32 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12704: Fix NPE when quering empty iceberg table's metadata

2024-01-15 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20890 )

Change subject: IMPALA-12704: Fix NPE when quering empty iceberg table's 
metadata
..


Patch Set 4: Code-Review+2

Thanks Zihao Ye! LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b4d4fb81a45214045b8809a4bdd910a1f1f3843
Gerrit-Change-Number: 20890
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zihao Ye 
Gerrit-Comment-Date: Mon, 15 Jan 2024 13:06:13 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12704: Fix NPE when quering empty iceberg table's metadata

2024-01-15 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20890 )

Change subject: IMPALA-12704: Fix NPE when quering empty iceberg table's 
metadata
..


Patch Set 3:

(1 comment)

Hi Zihao Ye, thank you for catching and fixing this issue.
The change looks good to me and can +2 after a small commit message change.

http://gerrit.cloudera.org:8080/#/c/20890/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20890/3//COMMIT_MSG@12
PS3, Line 12:
On the Impala project we usually add a short: "Tests" or "Testing" section to 
the commit message to highlight how the testing was done. In this case a simple:

```
Testing:
 - Added E2E test to cover this case
```

would suffice in my opinion.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b4d4fb81a45214045b8809a4bdd910a1f1f3843
Gerrit-Change-Number: 20890
Gerrit-PatchSet: 3
Gerrit-Owner: Zihao Ye 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Mon, 15 Jan 2024 10:20:34 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12706: Fix nested struct querying for Iceberg metadata tables

2024-01-11 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20883 )

Change subject: IMPALA-12706: Fix nested struct querying for Iceberg metadata 
tables
..


Patch Set 3:

Looks like the failure is due to a timing issue in:
query_test.test_cancellation.TestCancellationParallel.test_cancel_select


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadd029a4edc500bd8d8fca3f958903c2dbe09e8e
Gerrit-Change-Number: 20883
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 12 Jan 2024 06:59:37 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12706: Fix nested struct querying for Iceberg metadata tables

2024-01-11 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/20883 )

Change subject: IMPALA-12706: Fix nested struct querying for Iceberg metadata 
tables
..

IMPALA-12706: Fix nested struct querying for Iceberg metadata tables

This commit fixes a DCHECK failure when querying a struct inside a
struct. The previous field accessor creation logic was trying to find
the ColumnDescriptor for a struct inside a struct and hit a DCHECK
because there are no ColumnDescriptors for struct fields. The logic
has been reworked to only use ColumnDescriptors for top level columns.

Testing:
 - Added E2E test to cover this case

Change-Id: Iadd029a4edc500bd8d8fca3f958903c2dbe09e8e
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
3 files changed, 29 insertions(+), 22 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iadd029a4edc500bd8d8fca3f958903c2dbe09e8e
Gerrit-Change-Number: 20883
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12706: Fix nested struct querying for Iceberg metadata tables

2024-01-11 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20883


Change subject: IMPALA-12706: Fix nested struct querying for Iceberg metadata 
tables
..

IMPALA-12706: Fix nested struct querying for Iceberg metadata tables

This commit fixes a DCHECK failure when querying a struct inside a
struct. The previous field accessor creation logic was trying to find
the ColumnDescriptor for a struct inside a struct and hit a DCHECK
because there are no ColumnDescriptors for struct fields. The logic
has been reworked to only use ColumnDescriptors for top level columns.

Testing:
 - Added E2E test to cover this case

Change-Id: Iadd029a4edc500bd8d8fca3f958903c2dbe09e8e
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
3 files changed, 29 insertions(+), 22 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iadd029a4edc500bd8d8fca3f958903c2dbe09e8e
Gerrit-Change-Number: 20883
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate 


[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables

2024-01-10 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#12). ( 
http://gerrit.cloudera.org:8080/20695 )

Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..

IMPALA-12495: Describe statement for Iceberg metadata tables

Iceberg metadata tables are virtual tables and their schemata are
predefined by the Iceberg library. This commit extends the DESCRIBE
 statement, so the users can print the table description of these
tables.

Metadata tables do not exist in the HMS, therefore
DESCRIBE FORMATTED|EXTENDED statements are not permitted.

Testing:
 - Added E2E tests

Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
---
M be/src/service/frontend.cc
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
11 files changed, 377 insertions(+), 50 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20695/12
--
To view, visit http://gerrit.cloudera.org:8080/20695
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 12
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables

2024-01-10 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#10). ( 
http://gerrit.cloudera.org:8080/20695 )

Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..

IMPALA-12495: Describe statement for Iceberg metadata tables

Iceberg metadata tables are virtual tables and their schemata are
predefined by the Iceberg library. This commit extends the DESCRIBE
 statement, so the users can print the table description of these
tables.

Metadata tables do not exist in the HMS, therefore
DESCRIBE FORMATTED|EXTENDED statements are not permitted.

Testing:
 - Added E2E tests

Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
---
M be/src/service/frontend.cc
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
11 files changed, 372 insertions(+), 45 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables

2024-01-09 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#9). ( 
http://gerrit.cloudera.org:8080/20695 )

Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..

IMPALA-12495: Describe statement for Iceberg metadata tables

Iceberg metadata tables are virtual tables and their schemata are
predefined by the Iceberg library. This commit extends the DESCRIBE
 statement, so the users can print the table description of these
tables.

Metadata tables do not exist in the HMS, therefore
DESCRIBE FORMATTED|EXTENDED statements are not permitted.

Testing:
 - Added E2E tests

Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
---
M be/src/service/frontend.cc
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
11 files changed, 375 insertions(+), 46 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20695/9
--
To view, visit http://gerrit.cloudera.org:8080/20695
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables

2024-01-03 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/20695 )

Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..

IMPALA-12495: Describe statement for Iceberg metadata tables

Iceberg metadata tables are virtual tables and their schemata are
predefined by the Iceberg library. This commit extends the DESCRIBE
 statement, so the users can print the table description of these
tables.

Metadata tables do not exist in the HMS, therefore
DESCRIBE FORMATTED|EXTENDED statements are not permitted.

Testing:
 - Added E2E tests

Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
---
M be/src/service/frontend.cc
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
11 files changed, 374 insertions(+), 46 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables

2024-01-02 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/20695 )

Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..

IMPALA-12495: Describe statement for Iceberg metadata tables

Iceberg metadata tables are virtual tables and their schemata are
predefined by the Iceberg library. This commit extends the DESCRIBE
 statement, so the users can print the table description of these
tables.

Metadata tables do not exist in the HMS, therefore
DESCRIBE FORMATTED|EXTENDED statements are not permitted.

Testing:
 - Added E2E tests

Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
---
M be/src/service/frontend.cc
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
11 files changed, 371 insertions(+), 43 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables

2024-01-02 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20695 )

Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1664
PS5, Line 1664: params.result_struct
> I cannot hurt. Done.
It* :)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 02 Jan 2024 15:02:51 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables

2024-01-02 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20695 )

Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..


Patch Set 6:

(3 comments)

Thank you for the reviews!

http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java:

http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java@96
PS5, Line 96:   path_ = analyzer.resolvePath(rawPath_, PathType.ANY);
: } catch (AnalysisException ae) {
:   /
> Would it be possible to move 'isIcebergMetadataTable(rawPath_)' and 'analyz
Done, moved it to getTable, LMK your thoughts.


http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1657
PS5, Line 1657:
> Did you add a precondition for it? I still can't see anywhere if we check '
Done


http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1664
PS5, Line 1664:
> Should we add precondition check that this is set?
I cannot hurt. Done.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 02 Jan 2024 14:58:10 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables

2024-01-02 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/20695 )

Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..

IMPALA-12495: Describe statement for Iceberg metadata tables

Iceberg metadata tables are virtual tables and their schemata are
predefined by the Iceberg library. This commit extends the DESCRIBE
 statement, so the users can print the table description of these
tables.

Metadata tables do not exist in the HMS, therefore
DESCRIBE FORMATTED|EXTENDED statements are not permitted.

Testing:
 - Added E2E tests

Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
---
M be/src/service/frontend.cc
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
11 files changed, 368 insertions(+), 43 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR](asf-site) IMPALA-11853: Fix formatted docs query options CSS

2024-01-02 Thread Tamas Mate (Code Review)
Tamas Mate has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20824 )

Change subject: IMPALA-11853: Fix formatted docs query options CSS
..

IMPALA-11853: Fix formatted docs query options CSS

This is the release part of the CSS issue that was fixed by
IMPALA-11853.

The recent documentation formatting changes introduced the navigation
panel on the left. However, due to the length of the query options
navigation title these could overlap with the documentation paragraphs.

This commit removes the underscores from the navigation titles of the
query options, so browsers can break them into multiple lines.
Additionally, the "SET" and "Query Options for the SET Statement" pages
are merged to save some more space for the query option navigation
titles.

Change-Id: Ifcf12f401f3d4975c94ba78084ecf2c20267fad1
Reviewed-on: http://gerrit.cloudera.org:8080/20824
Tested-by: Tamas Mate 
Reviewed-by: Tamas Mate 
---
M docs/build/asf-site-html/index.html
M docs/build/asf-site-html/topics/impala_abort_on_error.html
M docs/build/asf-site-html/topics/impala_allow_erasure_coded_files.html
M docs/build/asf-site-html/topics/impala_allow_unsupported_formats.html
M docs/build/asf-site-html/topics/impala_appx_count_distinct.html
M docs/build/asf-site-html/topics/impala_batch_size.html
M docs/build/asf-site-html/topics/impala_broadcast_bytes_limit.html
M docs/build/asf-site-html/topics/impala_buffer_pool_limit.html
M docs/build/asf-site-html/topics/impala_compression_codec.html
M docs/build/asf-site-html/topics/impala_compute_stats_min_sample_size.html
M docs/build/asf-site-html/topics/impala_config_options.html
M docs/build/asf-site-html/topics/impala_debug_action.html
M docs/build/asf-site-html/topics/impala_decimal_v2.html
M docs/build/asf-site-html/topics/impala_default_file_format.html
M docs/build/asf-site-html/topics/impala_default_hints_insert_statement.html
M docs/build/asf-site-html/topics/impala_default_join_distribution_mode.html
M docs/build/asf-site-html/topics/impala_default_spillable_buffer_size.html
M docs/build/asf-site-html/topics/impala_default_transactional_type.html
M docs/build/asf-site-html/topics/impala_delete_stats_in_truncate.html
M docs/build/asf-site-html/topics/impala_disable_codegen.html
M docs/build/asf-site-html/topics/impala_disable_codegen_rows_threshold.html
M docs/build/asf-site-html/topics/impala_disable_hbase_num_rows_estimate.html
M docs/build/asf-site-html/topics/impala_disable_row_runtime_filtering.html
M docs/build/asf-site-html/topics/impala_disable_streaming_preaggregations.html
M docs/build/asf-site-html/topics/impala_disable_unsafe_spills.html
M docs/build/asf-site-html/topics/impala_enable_expr_rewrites.html
M docs/build/asf-site-html/topics/impala_exec_single_node_rows_threshold.html
M docs/build/asf-site-html/topics/impala_exec_time_limit_s.html
M docs/build/asf-site-html/topics/impala_expand_complex_types.html
M docs/build/asf-site-html/topics/impala_explain_level.html
M docs/build/asf-site-html/topics/impala_fetch_rows_timeout_ms.html
M docs/build/asf-site-html/topics/impala_hbase_cache_blocks.html
M docs/build/asf-site-html/topics/impala_hbase_caching.html
M docs/build/asf-site-html/topics/impala_idle_session_timeout.html
M docs/build/asf-site-html/topics/impala_join_rows_produced_limit.html
M docs/build/asf-site-html/topics/impala_kudu_read_mode.html
M docs/build/asf-site-html/topics/impala_live_progress.html
M docs/build/asf-site-html/topics/impala_live_summary.html
M docs/build/asf-site-html/topics/impala_max_errors.html
M docs/build/asf-site-html/topics/impala_max_mem_estimate_for_admission.html
M docs/build/asf-site-html/topics/impala_max_num_runtime_filters.html
M docs/build/asf-site-html/topics/impala_max_result_spooling_mem.html
M docs/build/asf-site-html/topics/impala_max_row_size.html
M docs/build/asf-site-html/topics/impala_max_scan_range_length.html
M docs/build/asf-site-html/topics/impala_max_spilled_result_spooling_mem.html
M docs/build/asf-site-html/topics/impala_mem_limit.html
M docs/build/asf-site-html/topics/impala_min_spillable_buffer_size.html
M docs/build/asf-site-html/topics/impala_mt_dop.html
M docs/build/asf-site-html/topics/impala_num_nodes.html
M docs/build/asf-site-html/topics/impala_num_rows_produced_limit.html
M docs/build/asf-site-html/topics/impala_num_scanner_threads.html
M docs/build/asf-site-html/topics/impala_optimize_partition_key_scans.html
M docs/build/asf-site-html/topics/impala_parquet_annotate_strings_utf8.html
M docs/build/asf-site-html/topics/impala_parquet_array_resolution.html
M docs/build/asf-site-html/topics/impala_parquet_compression_codec.html
M docs/build/asf-site-html/topics/impala_parquet_dictionary_filtering.html
M docs/build/asf-site-html/topics/impala_parquet_fallback_schema_resolution.html
M docs/build/asf-site-html/topics/impala_parquet_file_size.html
M docs/build/asf-site-html/topics/impala_parquet_object_store_split_size.html

[Impala-ASF-CR](asf-site) IMPALA-11853: Fix formatted docs query options CSS

2024-01-02 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20824 )

Change subject: IMPALA-11853: Fix formatted docs query options CSS
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcf12f401f3d4975c94ba78084ecf2c20267fad1
Gerrit-Change-Number: 20824
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Tue, 02 Jan 2024 11:57:56 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12658: UPDATE Iceberg table FROM view throws IllegalStateException

2024-01-02 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20825 )

Change subject: IMPALA-12658: UPDATE Iceberg table FROM view throws 
IllegalStateException
..


Patch Set 1: Code-Review+2

Thanks for the quick fix! LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I80ccdb61327a50082f792a6d51f946b11c467dab
Gerrit-Change-Number: 20825
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Noemi Pap-Takacs 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Tue, 02 Jan 2024 11:53:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12661: Fix ASAN heap-use-after-free in IcebergMetadataScanNode

2023-12-21 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20829


Change subject: IMPALA-12661: Fix ASAN heap-use-after-free in 
IcebergMetadataScanNode
..

IMPALA-12661: Fix ASAN heap-use-after-free in IcebergMetadataScanNode

The ASAN builds detected that the IcebergMetadataScanNode uses heap
allocated memory after it has been freed.

In CreateFieldAccessors() method, during tree traversal, the
current_type variable is reassigned to its children which is part of
of the object. However, by the end of the assignment the rhs object will
be destroyed. To fix this issue, the variable was replaced with a pointer.

Testing:
 - Ran tests on ASAN build

Change-Id: I6df9c9cb6914a0c6c93b61aa0dd02acfdba68851
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
1 file changed, 4 insertions(+), 4 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6df9c9cb6914a0c6c93b61aa0dd02acfdba68851
Gerrit-Change-Number: 20829
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate 


[Impala-ASF-CR](asf-site) IMPALA-11853: Fix formatted docs query options CSS

2023-12-20 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20824 )

Change subject: IMPALA-11853: Fix formatted docs query options CSS
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcf12f401f3d4975c94ba78084ecf2c20267fad1
Gerrit-Change-Number: 20824
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Wed, 20 Dec 2023 14:36:52 +
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) IMPALA-11853: Fix formatted docs query options CSS

2023-12-20 Thread Tamas Mate (Code Review)
Tamas Mate has removed a vote on this change.

Change subject: IMPALA-11853: Fix formatted docs query options CSS
..


Removed Verified-1 by Impala Public Jenkins 
--
To view, visit http://gerrit.cloudera.org:8080/20824
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ifcf12f401f3d4975c94ba78084ecf2c20267fad1
Gerrit-Change-Number: 20824
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 


[Impala-ASF-CR](asf-site) IMPALA-11853: Fix formatted docs query options CSS

2023-12-20 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20824


Change subject: IMPALA-11853: Fix formatted docs query options CSS
..

IMPALA-11853: Fix formatted docs query options CSS

This is the release part of the CSS issue that was fixed by
IMPALA-11853.

The recent documentation formatting changes introduced the navigation
panel on the left. However, due to the length of the query options
navigation title these could overlap with the documentation paragraphs.

This commit removes the underscores from the navigation titles of the
query options, so browsers can break them into multiple lines.
Additionally, the "SET" and "Query Options for the SET Statement" pages
are merged to save some more space for the query option navigation
titles.

Change-Id: Ifcf12f401f3d4975c94ba78084ecf2c20267fad1
---
M docs/build/asf-site-html/index.html
M docs/build/asf-site-html/topics/impala_abort_on_error.html
M docs/build/asf-site-html/topics/impala_allow_erasure_coded_files.html
M docs/build/asf-site-html/topics/impala_allow_unsupported_formats.html
M docs/build/asf-site-html/topics/impala_appx_count_distinct.html
M docs/build/asf-site-html/topics/impala_batch_size.html
M docs/build/asf-site-html/topics/impala_broadcast_bytes_limit.html
M docs/build/asf-site-html/topics/impala_buffer_pool_limit.html
M docs/build/asf-site-html/topics/impala_compression_codec.html
M docs/build/asf-site-html/topics/impala_compute_stats_min_sample_size.html
M docs/build/asf-site-html/topics/impala_config_options.html
M docs/build/asf-site-html/topics/impala_debug_action.html
M docs/build/asf-site-html/topics/impala_decimal_v2.html
M docs/build/asf-site-html/topics/impala_default_file_format.html
M docs/build/asf-site-html/topics/impala_default_hints_insert_statement.html
M docs/build/asf-site-html/topics/impala_default_join_distribution_mode.html
M docs/build/asf-site-html/topics/impala_default_spillable_buffer_size.html
M docs/build/asf-site-html/topics/impala_default_transactional_type.html
M docs/build/asf-site-html/topics/impala_delete_stats_in_truncate.html
M docs/build/asf-site-html/topics/impala_disable_codegen.html
M docs/build/asf-site-html/topics/impala_disable_codegen_rows_threshold.html
M docs/build/asf-site-html/topics/impala_disable_hbase_num_rows_estimate.html
M docs/build/asf-site-html/topics/impala_disable_row_runtime_filtering.html
M docs/build/asf-site-html/topics/impala_disable_streaming_preaggregations.html
M docs/build/asf-site-html/topics/impala_disable_unsafe_spills.html
M docs/build/asf-site-html/topics/impala_enable_expr_rewrites.html
M docs/build/asf-site-html/topics/impala_exec_single_node_rows_threshold.html
M docs/build/asf-site-html/topics/impala_exec_time_limit_s.html
M docs/build/asf-site-html/topics/impala_expand_complex_types.html
M docs/build/asf-site-html/topics/impala_explain_level.html
M docs/build/asf-site-html/topics/impala_fetch_rows_timeout_ms.html
M docs/build/asf-site-html/topics/impala_hbase_cache_blocks.html
M docs/build/asf-site-html/topics/impala_hbase_caching.html
M docs/build/asf-site-html/topics/impala_idle_session_timeout.html
M docs/build/asf-site-html/topics/impala_join_rows_produced_limit.html
M docs/build/asf-site-html/topics/impala_kudu_read_mode.html
M docs/build/asf-site-html/topics/impala_live_progress.html
M docs/build/asf-site-html/topics/impala_live_summary.html
M docs/build/asf-site-html/topics/impala_max_errors.html
M docs/build/asf-site-html/topics/impala_max_mem_estimate_for_admission.html
M docs/build/asf-site-html/topics/impala_max_num_runtime_filters.html
M docs/build/asf-site-html/topics/impala_max_result_spooling_mem.html
M docs/build/asf-site-html/topics/impala_max_row_size.html
M docs/build/asf-site-html/topics/impala_max_scan_range_length.html
M docs/build/asf-site-html/topics/impala_max_spilled_result_spooling_mem.html
M docs/build/asf-site-html/topics/impala_mem_limit.html
M docs/build/asf-site-html/topics/impala_min_spillable_buffer_size.html
M docs/build/asf-site-html/topics/impala_mt_dop.html
M docs/build/asf-site-html/topics/impala_num_nodes.html
M docs/build/asf-site-html/topics/impala_num_rows_produced_limit.html
M docs/build/asf-site-html/topics/impala_num_scanner_threads.html
M docs/build/asf-site-html/topics/impala_optimize_partition_key_scans.html
M docs/build/asf-site-html/topics/impala_parquet_annotate_strings_utf8.html
M docs/build/asf-site-html/topics/impala_parquet_array_resolution.html
M docs/build/asf-site-html/topics/impala_parquet_compression_codec.html
M docs/build/asf-site-html/topics/impala_parquet_dictionary_filtering.html
M docs/build/asf-site-html/topics/impala_parquet_fallback_schema_resolution.html
M docs/build/asf-site-html/topics/impala_parquet_file_size.html
M docs/build/asf-site-html/topics/impala_parquet_object_store_split_size.html
M docs/build/asf-site-html/topics/impala_parquet_page_row_count_limit.html
M docs/build/asf-site-html/topic

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

2023-12-19 Thread Tamas Mate (Code Review)
Tamas Mate has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Reviewed-on: http://gerrit.cloudera.org:8080/20759
Tested-by: Impala Public Jenkins 
Reviewed-by: Gabor Kaszab 
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.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/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
11 files changed, 280 insertions(+), 94 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 12
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


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

2023-12-19 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#11). ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.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/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
11 files changed, 280 insertions(+), 94 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


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

2023-12-19 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 11:

Forgot to update the planner test, with the new exception.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 19 Dec 2023 15:59:24 +
Gerrit-HasComments: No


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

2023-12-19 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#10). ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.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/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
10 files changed, 280 insertions(+), 58 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


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

2023-12-18 Thread Tamas Mate (Code Review)
Hello Gabor Kaszab, Zoltan Borok-Nagy, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.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/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
9 files changed, 270 insertions(+), 56 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


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

2023-12-18 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 8: Code-Review+2

(1 comment)

Thanks Gabor!
Carrying over the +2.

http://gerrit.cloudera.org:8080/#/c/20759/7/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
File be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc:

http://gerrit.cloudera.org:8080/#/c/20759/7/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@105
PS7, Line 105: childr
> nit: children
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:58:56 +
Gerrit-HasComments: Yes


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

2023-12-18 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 7:

I opened IMPALA-12651 for binary type as string support.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:39:13 +
Gerrit-HasComments: No


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

2023-12-18 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 7:

(2 comments)

Thanks Zoltan!

http://gerrit.cloudera.org:8080/#/c/20759/6/fe/src/main/java/org/apache/impala/analysis/FromClause.java
File fe/src/main/java/org/apache/impala/analysis/FromClause.java:

http://gerrit.cloudera.org:8080/#/c/20759/6/fe/src/main/java/org/apache/impala/analysis/FromClause.java@96
PS6, Line 96: checkIcebergCollectionSupport(tblRef);
: checkTopLevelComplexAcidScan(analyzer, 
(CollectionTableRef)tblRef);
: if (firstZippingUnnestRef != null && 
tblRef.isZippingUnnest() &&
:
> nit: for readability, can we put this check into its own method?
Done


http://gerrit.cloudera.org:8080/#/c/20759/6/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/20759/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@540
PS6, Line 540: 
> Thanks for adding these tests! Could you please add one with this JOIN synt
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 18 Dec 2023 15:27:39 +
Gerrit-HasComments: Yes


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

2023-12-18 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.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/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
9 files changed, 270 insertions(+), 56 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


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

2023-12-18 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.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/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
9 files changed, 256 insertions(+), 56 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12313: (part 3) Add UPDATE support for Iceberg tables.

2023-12-15 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20760 )

Change subject: IMPALA-12313: (part 3) Add UPDATE support for Iceberg tables.
..


Patch Set 11:

(8 comments)

Looks great Zoltan, just a few minor comments.

http://gerrit.cloudera.org:8080/#/c/20760/11/be/src/exec/iceberg-buffered-delete-sink.h
File be/src/exec/iceberg-buffered-delete-sink.h:

http://gerrit.cloudera.org:8080/#/c/20760/11/be/src/exec/iceberg-buffered-delete-sink.h@28
PS11, Line 28: struct StringVal;
How come StringVal has to be redefined here?


http://gerrit.cloudera.org:8080/#/c/20760/11/be/src/exec/iceberg-buffered-delete-sink.h@138
PS11, Line 138:
nit: empty space


http://gerrit.cloudera.org:8080/#/c/20760/11/be/src/exec/iceberg-delete-sink-base.h
File be/src/exec/iceberg-delete-sink-base.h:

http://gerrit.cloudera.org:8080/#/c/20760/11/be/src/exec/iceberg-delete-sink-base.h@49
PS11, Line 49:   Status ConstructPartitionInfo(
 :   const TupleRow* row,
 :   OutputPartition* output_partition);
nit: this should fit into one line


http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java
File fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java:

http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java@20
PS11, Line 20: import static java.lang.String.format;
unused import


http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java@30
PS11, Line 30: import org.apache.impala.catalog.FeFsTable;
unused import


http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java@37
PS11, Line 37: import org.apache.impala.planner.IcebergDeleteSink;
unused import


http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java@44
PS11, Line 44: import com.google.common.collect.ImmutableList;
unused import


http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/planner/IcebergBufferedDeleteSink.java
File fe/src/main/java/org/apache/impala/planner/IcebergBufferedDeleteSink.java:

http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/planner/IcebergBufferedDeleteSink.java@34
PS11, Line 34: IcebergBufferedDeleteSink
For me it looks like that ~40% of this class is identical with 
IcebergDeleteSink. Do you think it would worth inheriting from 
IcebergDeleteSink class instead of TableSink?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2bb97b4454165a292975d88dc9c23adb22ff7315
Gerrit-Change-Number: 20760
Gerrit-PatchSet: 11
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 15 Dec 2023 14:27:50 +
Gerrit-HasComments: Yes


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

2023-12-13 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.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/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
8 files changed, 241 insertions(+), 56 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


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

2023-12-11 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.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/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
8 files changed, 209 insertions(+), 60 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


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

2023-12-08 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.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/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
8 files changed, 204 insertions(+), 60 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12597: Basic Equality delete read support for Iceberg tables

2023-12-08 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20753 )

Change subject: IMPALA-12597: Basic Equality delete read support for Iceberg 
tables
..


Patch Set 2:

(4 comments)

Looks great, found a few nits.

http://gerrit.cloudera.org:8080/#/c/20753/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20753/1//COMMIT_MSG@41
PS1, Line 41: TODO
Is there a Jira for these items? It would help linking this commit with other 
tasks.


http://gerrit.cloudera.org:8080/#/c/20753/2/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:

http://gerrit.cloudera.org:8080/#/c/20753/2/common/thrift/CatalogObjects.thrift@635
PS2, Line 635: }
nit: missing new line


http://gerrit.cloudera.org:8080/#/c/20753/2/fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java
File fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java:

http://gerrit.cloudera.org:8080/#/c/20753/2/fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java@106
PS2, Line 106: TODO
TODO without Jira ID.


http://gerrit.cloudera.org:8080/#/c/20753/2/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test:

http://gerrit.cloudera.org:8080/#/c/20753/2/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test@1179
PS2, Line 1179: 04:UNION   
nit: trailing spaces



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2053e6f321c69f1c82059a84a5d99aeaa9814cad
Gerrit-Change-Number: 20753
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 08 Dec 2023 11:56:20 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12313: (part 2) Limited UPDATE support for Iceberg tables

2023-12-08 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20677 )

Change subject: IMPALA-12313: (part 2) Limited UPDATE support for Iceberg tables
..


Patch Set 9:

(1 comment)

Looks good to me!
I can plus two after these comments.

http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
File fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java:

http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java@63
PS9, Line 63: // Currently only Kudu tables are supported.
Not anymore :)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iff0ef6075a2b6ebe130d15daa389ac1a505a7a08
Gerrit-Change-Number: 20677
Gerrit-PatchSet: 9
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Noemi Pap-Takacs 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 08 Dec 2023 11:32:54 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12313: (part 2) Limited UPDATE support for Iceberg tables

2023-12-07 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20677 )

Change subject: IMPALA-12313: (part 2) Limited UPDATE support for Iceberg tables
..


Patch Set 9:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/exec/iceberg-delete-sink.h
File be/src/exec/iceberg-delete-sink.h:

http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/exec/iceberg-delete-sink.h@81
PS9, Line 81:   /// Verifies that the row batch does not contain duplicated 
rows.
Could you add an explanation of intent, it is not clear for me why this is 
needed.


http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/exec/iceberg-delete-sink.h@82
PS9, Line 82: VerifyRowBatch
Nit: For readability could you rename to something like: 
VerifyRowsNotDuplicated/VerifyRowDistinctiveness


http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/exec/multi-table-sink.cc
File be/src/exec/multi-table-sink.cc:

http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/exec/multi-table-sink.cc@52
PS9, Line 52: tsinkBase
nit: tsink_base


http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/runtime/dml-exec-state.h
File be/src/runtime/dml-exec-state.h:

http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/runtime/dml-exec-state.h@72
PS9, Line 72:   void UpdatePartition(const std::string& partition_name,
:   int64_t num_rows_delta, const DmlStatsPB* insert_stats,
:   bool is_delete = false);
nit: should fit into 2 lines


http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java:

http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java@120
PS9, Line 120: id
nit: ++nextTableId_ and return nextTableId_


http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/IcebergModifyImpl.java
File fe/src/main/java/org/apache/impala/analysis/IcebergModifyImpl.java:

http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/IcebergModifyImpl.java@87
PS9, Line 87:   public List getDeletePartitionExprs(Analyzer analyzer)
:   throws AnalysisException {
nit: should fit into 1 line


http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/IcebergModifyImpl.java@94
PS9, Line 94:   public List getDeleteResultExprs(Analyzer analyzer)
:   throws AnalysisException {
nit: should fit into 1 line


http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/planner/Planner.java
File fe/src/main/java/org/apache/impala/planner/Planner.java:

http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/planner/Planner.java@185
PS9, Line 185:
nit: indentation



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iff0ef6075a2b6ebe130d15daa389ac1a505a7a08
Gerrit-Change-Number: 20677
Gerrit-PatchSet: 9
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Noemi Pap-Takacs 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 07 Dec 2023 17:55:35 +
Gerrit-HasComments: Yes


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

2023-12-07 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.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/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
8 files changed, 164 insertions(+), 41 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


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

2023-12-07 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20759 )

Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20759/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/20759/1/be/src/exec/iceberg-metadata/iceberg-row-reader.h@45
PS1, Line 45:   Status MaterializeTuple(JNIEnv* env, jobject struct_like_row, 
const TupleDescriptor* tuple_desc,
> line too long (98 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/20759/1/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/20759/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@79
PS1, Line 79:   accessed_value = env->CallObjectMethod(accessor, 
iceberg_accessor_get_, struct_like_row);
> line too long (95 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/20759/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@104
PS1, Line 104: RETURN_IF_ERROR(WriteStructSlot(env, struct_like_row, 
slot_desc, tuple, tuple_data_pool));
> line too long (98 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/20759/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@172
PS1, Line 172: Status IcebergRowReader::WriteStructSlot(JNIEnv* env, jobject 
struct_like_row, SlotDescriptor* slot_desc, Tuple* tuple,
> line too long (119 > 90)
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 07 Dec 2023 10:07:24 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12313: (part 2) Limited UPDATE support for Iceberg tables

2023-12-06 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20677 )

Change subject: IMPALA-12313: (part 2) Limited UPDATE support for Iceberg tables
..


Patch Set 7:

(1 comment)

Great change Zoltan!
Started to review and tested it a bit.

The MultiTableSink counters in the profile seem to be off, the counters are not 
updated based on the sum of "child" sinks.

http://gerrit.cloudera.org:8080/#/c/20677/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20677/7//COMMIT_MSG@75
PS7, Line 75: Patch
nit: Part



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iff0ef6075a2b6ebe130d15daa389ac1a505a7a08
Gerrit-Change-Number: 20677
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Noemi Pap-Takacs 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 06 Dec 2023 14:16:21 +
Gerrit-HasComments: Yes


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

2023-12-06 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20759


Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table 
columns
..

IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns

As the slots have already been created on the frontend this change
focuses on populating them on the backend side. There are two major
parts of this commit. Obtaining the right Accessors for the slot and
recursively filling the tuples with data.

The field ids are present in the struct slot's ColumnType field as a
list of integers. This list can be indexed with the correct element of
the SchemaPath to obtain the field id for a struct member and with that
the Accessor.

Once the Accessors are available the IcebergRowReader's MaterializeTuple
method can be called recursively to write the primitive slots of a
struct slot.

Testing:
 - Added E2E tests

Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
---
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.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/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
8 files changed, 162 insertions(+), 41 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273
Gerrit-Change-Number: 20759
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables

2023-11-23 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/20695 )

Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..

IMPALA-12495: Describe statement for Iceberg metadata tables

Iceberg metadata tables are virtual tables and their schemata are
predefined by the Iceberg library. This commit extends the DESCRIBE
 statement, so the users can print the table description of these
tables.

Metadata tables do not exist in the HMS, therefore
DESCRIBE FORMATTED|EXTENDED statements are not permitted.

Testing:
 - Added E2E tests

Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
---
M be/src/service/frontend.cc
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
8 files changed, 363 insertions(+), 40 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 


[Impala-ASF-CR] IMPALA-12243: Add support for DROP PARTITION for Iceberg tables

2023-11-22 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20515 )

Change subject: IMPALA-12243: Add support for DROP PARTITION for Iceberg tables
..


Patch Set 9: Code-Review+1

(1 comment)

Thanks Peter! LGTM!

http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java
File fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java:

http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@253
PS8, Line 253: String.format("Unable to parse timestamp value from: %s",
> This throw statement is in the same block as the whole try-catch structure,
Probably a misclick and wanted to comment eh LOG.warn() lines.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a768ba2966f570454687e02e4e6d67df46741f9
Gerrit-Change-Number: 20515
Gerrit-PatchSet: 9
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 22 Nov 2023 10:04:35 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables

2023-11-14 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20695 )

Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..


Patch Set 2:

(9 comments)

Thank you for the review Daniel!
Updated the patch.

http://gerrit.cloudera.org:8080/#/c/20695/1/be/src/service/frontend.cc
File be/src/service/frontend.cc:

http://gerrit.cloudera.org:8080/#/c/20695/1/be/src/service/frontend.cc@183
PS1, Line 183:   if (params.__isset.metadata_table_name) 
tparams.__set_metadata_table_name(
> line too long (104 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java:

http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java@96
PS2, Line 96: analyzeIcebergMetadataTable
> I found the name 'analyzeIcebergMetadataTable' a bit misleading, to me it s
Indeed, I decided to move the 'isIcebergMetadataTable' outside of the 
'analyzeIcebergMetadataTable' method.


http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
File fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java:

http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java@358
PS2, Line 358: a
> Nit: "an".
Done


http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/Frontend.java@1642
PS2, Line 1642:* Returns table metadata, such as the column descriptors, in 
the specified table.
> Could mention 'vTableName' in the comment, including that it should be null
Refactored this part, let me know your thoughts.


http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/Frontend.java@1650
PS2, Line 1650: String.format("fetching table %s.%s", 
tableName.db_name, tableName.table_name));
> Isn't this message misleading if we are actually looking up a metadata tabl
The "parent" table is being loaded for this query. The metadata table does not 
exist in the catalog.


http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/Frontend.java@1708
PS2, Line 1708:   Preconditions.checkArgument(outputStyle == 
TDescribeOutputStyle.FORMATTED ||
> Optional: could add a Precondition check that 'vTableName' is null.
Done


http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:

http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java@503
PS2, Line 503: metadata_table_name
> Shouldn't we check whether this param is set? I don't know if Thrift guaran
Taken care of as part of the refactoring.


http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
File 
fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java:

http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java@273
PS2, Line 273: ""
> Shouldn't it be null instead?
With the refactoring this changed.


http://gerrit.cloudera.org:8080/#/c/20695/2/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/20695/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@618
PS2, Line 618: describe 
functional_parquet.iceberg_query_metadata.all_data_files;
> Shouldn't the 'all_*_files' tables be grouped together with the other '*fil
You mean to not duplicate them?
The reason I kept them separate is that there is an Iceberg library class 
representation for each of these, such as
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/AllDataFilesTable.java



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Tue, 14 Nov 2023 11:28:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12243: Add support for DROP PARTITION for Iceberg tables

2023-11-13 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20515 )

Change subject: IMPALA-12243: Add support for DROP PARTITION for Iceberg tables
..


Patch Set 8:

(17 comments)

Thank you for the changes Peter.
Added some comments, mainly nits/formatting.

http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
File 
fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java:

http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java@96
PS8, Line 96: addPartParams
This is not originating from your change, but looks like an copy&paste from 
AlterTableAddPartitionStmt. Could you rename it?


http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java
File fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java:

http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java@39
PS8, Line 39:
nit: empty new line


http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java@129
PS8, Line 129: Column column = desc.getColumn();
 : IcebergColumn icebergColumn = (IcebergColumn) column;
nit: could be one line; then we don't need to import Column


http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java@133
PS8, Line 133: partitionSpec_.getFieldsBySourceId(
 : icebergColumn.getFieldId());
nit: should fit into one line


http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java@182
PS8, Line 182: /*Keeping it empty to avoid constant folding */
nit: missing space


http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpressionRewriter.java
File 
fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpressionRewriter.java:

http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpressionRewriter.java@102
PS8, Line 102: if (!(literal instanceof LiteralExpr)) {
 :   return binaryPredicate;
 : }
nit: this could fit into one line, like L107


http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
File fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java:

http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java@98
PS8, Line 98: if (table instanceof IcebergTable){
nit: space


http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java@100
PS8, Line 100: }
 : else{
nit: one line


http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@401
PS2, Line 401:
> It originates from IcebergUtil/getPartitionTransformType. Before this patch
Yeah, somehow we should prevent AnalysisException during table load.


http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java
File fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java:

http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@170
PS8, Line 170: Preconditions.checkState(false,
 : "Unsupported Iceberg type considered for predicate: 
%s",
 : literal.getType().toSql());
 :   }
 : }
 : throw new ImpalaRuntimeException(
 : String.format("Unable to parse Iceberg value: %s,",
 : literal.getStringValue()));
This throw + precondition check looks redundant.


http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@253
PS8, Line 253: throw new ImpalaRuntimeException(
nit: indentation


http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@296
PS8, Line 296:   String.format("Unable to create term from expression: 
%s",
 :   expr.toSql()));
nit: one line


http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@302
PS8, Line 302:   String.format("Invalid column type %s for column: %s",
 :   column.getType(), column));
nit: one line


h

[Impala-ASF-CR] IMPALA-12308: DIRECTED distribution mode for V2 Iceberg tables

2023-11-13 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20548 )

Change subject: IMPALA-12308: DIRECTED distribution mode for V2 Iceberg tables
..


Patch Set 13: Code-Review+2

(1 comment)

Thank you for the changes Gabor!
LGTM!

http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:

http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/query-state.h@135
PS10, Line 135: int
> protobuf doesn't have int just int32, but here int is a synonym for for int
I do not think that anything could go south here, this was mainly a readability 
suggestion and to guarantee the size of the integer here. Protobuff generates 
int32_t here if I am not mistaken.
https://protobuf.dev/getting-started/cpptutorial/#protobuf-api



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I212afd7c9e94551a1c50a40ccb0e3c1f7ecdf3d2
Gerrit-Change-Number: 20548
Gerrit-PatchSet: 13
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 13 Nov 2023 13:38:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12527: Fix Iceberg metadata table test S3 paths

2023-11-13 Thread Tamas Mate (Code Review)
Tamas Mate has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20659 )

Change subject: IMPALA-12527: Fix Iceberg metadata table test S3 paths
..

IMPALA-12527: Fix Iceberg metadata table test S3 paths

This commit excludes test_metadata_tables executions which could fail
due to:
 1) hardcoded uris in delete files
 2) reused data load result, that points to a different file system

Testing:
 - HDFS: tested in my local dev environment
 - S3: executed a jenkins job with the failing S3 parameters

Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562
Reviewed-on: http://gerrit.cloudera.org:8080/20659
Tested-by: Impala Public Jenkins 
Reviewed-by: Zoltan Borok-Nagy 
Reviewed-by: Peter Rozsa 
---
M tests/query_test/test_iceberg.py
1 file changed, 1 insertion(+), 0 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Zoltan Borok-Nagy: Looks good to me, approved
  Peter Rozsa: Looks good to me, but someone else must approve

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562
Gerrit-Change-Number: 20659
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12527: Fix Iceberg metadata table test S3 paths

2023-11-13 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20659 )

Change subject: IMPALA-12527: Fix Iceberg metadata table test S3 paths
..


Patch Set 5:

Thank you for the reviews!
I will split the tests with the STRUCT type addition.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562
Gerrit-Change-Number: 20659
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 13 Nov 2023 09:27:50 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables

2023-11-10 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/20695 )

Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..

IMPALA-12495: Describe statement for Iceberg metadata tables

Iceberg metadata tables are virtual tables and their schemata are
predefined by the Iceberg library. This commit extends the DESCRIBE
 statement, so the users can print the table description of these
tables.

Metadata tables do not exist in the HMS, therefore
DESCRIBE FORMATTED|EXTENDED statements are not permitted.

Testing:
 - Added E2E tests

Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
---
M be/src/service/frontend.cc
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
8 files changed, 323 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables

2023-11-10 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20695


Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..

IMPALA-12495: Describe statement for Iceberg metadata tables

Iceberg metadata tables are virtual tables and their schemata are
predefined by the Iceberg library. This commit extends the DESCRIBE
 statement, so the users can print the table description of these
tables.

Metadata tables do not exist in the HMS, therefore
DESCRIBE FORMATTED|EXTENDED statements are not permitted.

Testing:
 - Added E2E tests

Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
---
M be/src/service/frontend.cc
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
8 files changed, 322 insertions(+), 13 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate 


[Impala-ASF-CR] IMPALA-12308: DIRECTED distribution mode for V2 Iceberg tables

2023-11-09 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20548 )

Change subject: IMPALA-12308: DIRECTED distribution mode for V2 Iceberg tables
..


Patch Set 10:

(13 comments)

Thank you for the updates Gabor, I have left some further comments.

http://gerrit.cloudera.org:8080/#/c/20548/4/be/src/exec/data-sink.h
File be/src/exec/data-sink.h:

http://gerrit.cloudera.org:8080/#/c/20548/4/be/src/exec/data-sink.h@90
PS4, Line 90:  protected:
nit: a new line was removed


http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/fragment-state.h
File be/src/runtime/fragment-state.h:

http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/fragment-state.h@242
PS10, Line 242: p
nit: P


http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/fragment-state.cc
File be/src/runtime/fragment-state.cc:

http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/fragment-state.cc@97
PS10, Line 97:
nit: empty line


http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/fragment-state.cc@131
PS10, Line 131: filename_to_hosts_[full_file_path]
Inserting full_file_path to a filename_to_hosts map looks a bit odd.


http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/krpc-data-stream-sender.h
File be/src/runtime/krpc-data-stream-sender.h:

http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/krpc-data-stream-sender.h@193
PS10, Line 193: Funtions
nit: Functions


http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/krpc-data-stream-sender.h@197
PS10, Line 197: i
nit: I


http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:

http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/query-state.h@135
PS10, Line 135: int
int32 to make it a bit more aligned with admission_control_service.proto 
description.


http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/query-state.h@172
PS10, Line 172:
const


http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/scheduling/scheduler.h
File be/src/scheduling/scheduler.h:

http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/scheduling/scheduler.h@86
PS10, Line 86:   /// Map from filenames to hosts where those files are 
scheduled, grouped by scan node
 :   // ID.
nit: one line or the second line needs ///


http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/scheduling/scheduler.cc@980
PS10, Line 980:   ByNodeFilenamesToHosts duplicate_check;
Unused variable, maybe even the parameter in PopulateFilenamesToHostsMapping.


http://gerrit.cloudera.org:8080/#/c/20548/10/common/thrift/Partitions.thrift
File common/thrift/Partitions.thrift:

http://gerrit.cloudera.org:8080/#/c/20548/10/common/thrift/Partitions.thrift@44
PS10, Line 44: of
nit: for


http://gerrit.cloudera.org:8080/#/c/20548/10/fe/src/main/java/org/apache/impala/planner/DataPartition.java
File fe/src/main/java/org/apache/impala/planner/DataPartition.java:

http://gerrit.cloudera.org:8080/#/c/20548/10/fe/src/main/java/org/apache/impala/planner/DataPartition.java@79
PS10, Line 79:   public final static DataPartition DIRECTED = new 
DataPartition(TPartitionType.DIRECTED);
nit: I think this would look more consistent with the class structure in L70


http://gerrit.cloudera.org:8080/#/c/20548/10/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20548/10/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@79
PS10, Line 79:   }
nit: missing empty line



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I212afd7c9e94551a1c50a40ccb0e3c1f7ecdf3d2
Gerrit-Change-Number: 20548
Gerrit-PatchSet: 10
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 09 Nov 2023 16:22:41 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12527: Fix Iceberg metadata table test S3 paths

2023-11-09 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/20659 )

Change subject: IMPALA-12527: Fix Iceberg metadata table test S3 paths
..

IMPALA-12527: Fix Iceberg metadata table test S3 paths

This commit excludes test_metadata_tables executions which could fail
due to:
 1) hardcoded uris in delete files
 2) reused data load result, that points to a different file system

Testing:
 - HDFS: tested in my local dev environment
 - S3: executed a jenkins job with the failing S3 parameters

Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562
---
M tests/query_test/test_iceberg.py
1 file changed, 1 insertion(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562
Gerrit-Change-Number: 20659
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12527: Fix Iceberg metadata table test S3 paths

2023-11-08 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/20659 )

Change subject: IMPALA-12527: Fix Iceberg metadata table test S3 paths
..

IMPALA-12527: Fix Iceberg metadata table test S3 paths

The Iceberg library does not necessarily add the filesystem prefix to
the file paths, which can cause tests on different filesystems to fail.

This change removes the filesystem checks from the file paths.

Testing:
 - HDFS: tested in my local dev environment
 - S3: executed the test on with the failing S3 parameters

Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562
---
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
2 files changed, 43 insertions(+), 42 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562
Gerrit-Change-Number: 20659
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12527: Fix Iceberg metadata table test S3 paths

2023-11-06 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/20659 )

Change subject: IMPALA-12527: Fix Iceberg metadata table test S3 paths
..

IMPALA-12527: Fix Iceberg metadata table test S3 paths

The Iceberg library does not necessarily add the filesystem prefix to
the file paths, which can cause tests on different filesystems to fail.

This change removes the filesystem checks from the file paths.

Testing:
 - HDFS: tested in my local dev environment
 - S3: executed the jenkins job with the failing S3 parameters

Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562
---
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
2 files changed, 44 insertions(+), 43 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562
Gerrit-Change-Number: 20659
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate 


[Impala-ASF-CR] IMPALA-12527: Fix Iceberg metadata table test S3 paths

2023-11-06 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20659


Change subject: IMPALA-12527: Fix Iceberg metadata table test S3 paths
..

IMPALA-12527: Fix Iceberg metadata table test S3 paths

The Iceberg library does not necessarily add the filesystem prefix to
the file paths, which can cause tests on different filesystems to fail.

This change removes the filesystem checks from the file paths.

Testing:
 - HDFS: tested in my local dev environment
 - S3: executed the test on with the failing S3 parameters

Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562
---
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
2 files changed, 44 insertions(+), 43 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562
Gerrit-Change-Number: 20659
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate 


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-26 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..


Patch Set 17: Code-Review+2

Fixed a test case that was introduced by the Iceberg lib upgrade.
Carrying over the +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 17
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 26 Oct 2023 08:17:34 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-26 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#17). ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..

IMPALA-11996: Scanner change for Iceberg metadata querying

This commit adds a scan node for querying Iceberg metadata tables. The
scan node creates a Java scanner object that creates and scans the
metadata table. The scanner uses the Iceberg API to scan the table after
that the scan node fetches the rows one by one and materialises them
into RowBatches. The Iceberg row reader on the backend does the
translation between Iceberg and Impala types.

There is only one fragment created to query the Iceberg metadata table
which is supposed to be executed on the coordinator node that already
has the Iceberg table loaded. This way there is no need for further
table loading on the executor side.

This change will not cover nested column types, these slots are set to
NULL, it will be done in IMPALA-12205.

Testing:
 - Added e2e tests for querying metadata tables
 - Updated planner tests

Performance testing:
Created a table and inserted ~5500 rows one by one, this generated
~27 ALL_MANIFESTS metadata table records. This table is quite wide
and has a String column as well.

I only mention count(*) test on ALL_MANIFESTS, because every row is
materialized in every scenario currently:
  - Cold cache: 15.76s
- IcebergApiScanTime: 124.407ms
- MaterializeTupleTime: 8s368ms
  - Warm cache: 7.56s
- IcebergApiScanTime: 3.646ms
- MaterializeTupleTime: 7s477ms

Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
---
M be/CMakeLists.txt
M be/src/exec/CMakeLists.txt
M be/src/exec/exec-node.cc
A be/src/exec/iceberg-metadata/CMakeLists.txt
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-row-reader.cc
A be/src/exec/iceberg-metadata/iceberg-row-reader.h
M be/src/scheduling/scheduler.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impalad-main.cc
M be/src/util/jni-util.cc
M be/src/util/jni-util.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M 
testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/authorization/test_ranger.py
M tests/query_test/test_iceberg.py
32 files changed, 1,419 insertions(+), 167 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 17
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-9118: Show catalog operation details in catalogd webUI

2023-10-25 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20428 )

Change subject: IMPALA-9118: Show catalog operation details in catalogd webUI
..


Patch Set 7: Code-Review+2

Thanks Quanlong, LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3cf3f0da2be2be79e546762a8083d4de338ff6aa
Gerrit-Change-Number: 20428
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Wed, 25 Oct 2023 09:05:23 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-24 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..


Patch Set 15:

(2 comments)

Thank you for the comments!
Published PS15 and answered your questions/comments.

http://gerrit.cloudera.org:8080/#/c/20010/15/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/20010/15/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@411
PS15, Line 411:  RESULTS
> In this test couldn't you assert on the exact values instead of checking re
I don't think I can test it without using the same code path to generate the 
expected results.

Also, I cannot pre-generate a table, because the metadata gets malformed when 
using avrotools (comment on PS7).


http://gerrit.cloudera.org:8080/#/c/20010/15/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@416
PS15, Line 416:
> Can we add a few more complex queries from https://www.apachecon.com/acna20
This commit does not resolve the nested type columns. I can add more complex 
queries with the nested type sub-task, IMPALA-12205.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 15
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 24 Oct 2023 08:46:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-24 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#15). ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..

IMPALA-11996: Scanner change for Iceberg metadata querying

This commit adds a scan node for querying Iceberg metadata tables. The
scan node creates a Java scanner object that creates and scans the
metadata table. The scanner uses the Iceberg API to scan the table after
that the scan node fetches the rows one by one and materialises them
into RowBatches. The Iceberg row reader on the backend does the
translation between Iceberg and Impala types.

There is only one fragment created to query the Iceberg metadata table
which is supposed to be executed on the coordinator node that already
has the Iceberg table loaded. This way there is no need for further
table loading on the executor side.

This change will not cover nested column types, these slots are set to
NULL, it will be done in IMPALA-12205.

Testing:
 - Added e2e tests for querying metadata tables
 - Updated planner tests

Performance testing:
Created a table and inserted ~5500 rows one by one, this generated
~27 ALL_MANIFESTS metadata table records. This table is quite wide
and has a String column as well.

I only mention count(*) test on ALL_MANIFESTS, because every row is
materialized in every scenario currently:
  - Cold cache: 15.76s
- IcebergApiScanTime: 124.407ms
- MaterializeTupleTime: 8s368ms
  - Warm cache: 7.56s
- IcebergApiScanTime: 3.646ms
- MaterializeTupleTime: 7s477ms

Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
---
M be/CMakeLists.txt
M be/src/exec/CMakeLists.txt
M be/src/exec/exec-node.cc
A be/src/exec/iceberg-metadata/CMakeLists.txt
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-row-reader.cc
A be/src/exec/iceberg-metadata/iceberg-row-reader.h
M be/src/scheduling/scheduler.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impalad-main.cc
M be/src/util/jni-util.cc
M be/src/util/jni-util.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M 
testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/authorization/test_ranger.py
M tests/query_test/test_iceberg.py
32 files changed, 1,419 insertions(+), 167 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 15
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-20 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#10). ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..

IMPALA-11996: Scanner change for Iceberg metadata querying

This commit adds a scan node for querying Iceberg metadata tables. The
scan node creates a Java scanner object that creates and scans the
metadata table. The scanner uses the Iceberg API to scan the table after
that the scan node fetches the rows one by one and materialises them
into RowBatches. The Iceberg row reader on the backend does the
translation between Iceberg and Impala types.

There is only one fragment created to query the Iceberg metadata table
which is supposed to be executed on the coordinator node that already
has the Iceberg table loaded. This way there is no need for further
table loading on the executor side.

This change will not cover struct column types, these slots are set to
NULL, it will be done in IMPALA-12205.

Testing:
 - Added e2e tests for querying metadata tables
 - Updated planner tests

Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
---
M be/CMakeLists.txt
M be/src/exec/CMakeLists.txt
M be/src/exec/exec-node.cc
A be/src/exec/iceberg-metadata/CMakeLists.txt
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-row-reader.cc
A be/src/exec/iceberg-metadata/iceberg-row-reader.h
M be/src/scheduling/scheduler.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impalad-main.cc
M be/src/util/jni-util.cc
M be/src/util/jni-util.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M 
testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
31 files changed, 1,358 insertions(+), 175 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-19 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#14). ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..

IMPALA-11996: Scanner change for Iceberg metadata querying

This commit adds a scan node for querying Iceberg metadata tables. The
scan node creates a Java scanner object that creates and scans the
metadata table. The scanner uses the Iceberg API to scan the table after
that the scan node fetches the rows one by one and materialises them
into RowBatches. The Iceberg row reader on the backend does the
translation between Iceberg and Impala types.

There is only one fragment created to query the Iceberg metadata table
which is supposed to be executed on the coordinator node that already
has the Iceberg table loaded. This way there is no need for further
table loading on the executor side.

This change will not cover nested column types, these slots are set to
NULL, it will be done in IMPALA-12205.

Testing:
 - Added e2e tests for querying metadata tables
 - Updated planner tests

Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
---
M be/CMakeLists.txt
M be/src/exec/CMakeLists.txt
M be/src/exec/exec-node.cc
A be/src/exec/iceberg-metadata/CMakeLists.txt
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-row-reader.cc
A be/src/exec/iceberg-metadata/iceberg-row-reader.h
M be/src/scheduling/scheduler.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impalad-main.cc
M be/src/util/jni-util.cc
M be/src/util/jni-util.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M 
testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/authorization/test_ranger.py
M tests/query_test/test_iceberg.py
32 files changed, 1,393 insertions(+), 166 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 14
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-19 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..


Patch Set 14:

(8 comments)

Thank you for the review Zoltan, updated the change.

http://gerrit.cloudera.org:8080/#/c/20010/13/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/20010/13/be/src/exec/iceberg-metadata/iceberg-row-reader.h@20
PS13, Line 20: #include 
> Seems like we can remove this.
Done


http://gerrit.cloudera.org:8080/#/c/20010/13/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/20010/13/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@104
PS13, Line 104: }
  :   }
  :   return Status::OK();
  : }
  :
  : Status IcebergRowReader::WriteBooleanSlot(JNIEnv* env,
  :
> These lines are common in all Read methods. It might make sense to put them
I moved it to MaterializeRow and renamed the Read methods.


http://gerrit.cloudera.org:8080/#/c/20010/13/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@111
PS13, Line 111: java_boolean_c
> Can we check somehow that accessed_value has the expected type?
I could DCHECK an IsInstanceOf call.


http://gerrit.cloudera.org:8080/#/c/20010/13/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
File fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java:

http://gerrit.cloudera.org:8080/#/c/20010/13/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java@414
PS13, Line 414: UNPARTITIONED
> Why are we using UNPARTITIONED instead of RANDOM?
This is for the scheduler, to execute the fragment on the coordinator. 
scheduler.cc:171


http://gerrit.cloudera.org:8080/#/c/20010/13/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/20010/13/fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java@76
PS13, Line 76:* Creates the Metadata{Table} which is a predifined Iceberg 
{Table} object. This method
 :* also starts an Iceberg {TableScan} to scan the {Table}. 
After the scan is ready it
 :* initializes the iterators, so the {GetNext} call can start 
fetching the rows through
 :* the Iceberg Api.
 :*/
> nit: Almost the same as L102-L106. Can we refactor this?
Done


http://gerrit.cloudera.org:8080/#/c/20010/13/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/20010/13/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@6
PS13, Line 6: metadata t
> nit: metadata
Done


http://gerrit.cloudera.org:8080/#/c/20010/13/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@6
PS13, Line 6: Query all the metadata tables once
> Is there a list somewhere about all the metadata tables?
Only in the Iceberg codebase at the moment.
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/MetadataTableType.java


http://gerrit.cloudera.org:8080/#/c/20010/13/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/20010/13/tests/authorization/test_ranger.py@2048
PS13, Line 2048: the metad
> nit: the metadata tables
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 14
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 19 Oct 2023 12:55:01 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12308: DIRECTED distribution mode for V2 Iceberg tables

2023-10-18 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20548 )

Change subject: IMPALA-12308: DIRECTED distribution mode for V2 Iceberg tables
..


Patch Set 4:

(6 comments)

Great change Gabor, I left some nits.

http://gerrit.cloudera.org:8080/#/c/20548/4/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/20548/4/be/src/runtime/krpc-data-stream-sender.cc@1090
PS4, Line 1090: than
nit: then


http://gerrit.cloudera.org:8080/#/c/20548/4/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:

http://gerrit.cloudera.org:8080/#/c/20548/4/be/src/runtime/query-state.h@123
PS4, Line 123: egzact
nit: exact


http://gerrit.cloudera.org:8080/#/c/20548/4/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
File fe/src/main/java/org/apache/impala/planner/ExchangeNode.java:

http://gerrit.cloudera.org:8080/#/c/20548/4/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java@77
PS4, Line 77: give
nit: return


http://gerrit.cloudera.org:8080/#/c/20548/4/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20548/4/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@74
PS4, Line 74:  MultiAggregateInfo aggInfo, 
List fileDescs,
:  
nit: indentation


http://gerrit.cloudera.org:8080/#/c/20548/4/fe/src/main/java/org/apache/impala/planner/JoinNode.java
File fe/src/main/java/org/apache/impala/planner/JoinNode.java:

http://gerrit.cloudera.org:8080/#/c/20548/4/fe/src/main/java/org/apache/impala/planner/JoinNode.java@219
PS4, Line 219:
nit: indentation


http://gerrit.cloudera.org:8080/#/c/20548/4/testdata/workloads/functional-query/queries/QueryTest/set.test
File testdata/workloads/functional-query/queries/QueryTest/set.test:

http://gerrit.cloudera.org:8080/#/c/20548/4/testdata/workloads/functional-query/queries/QueryTest/set.test@135
PS4, Line 135: DIRECTED
This can be confusing, because setting this has no effect on the distribution 
mode. Could we hide it or create doc jira to explain it for the user?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I212afd7c9e94551a1c50a40ccb0e3c1f7ecdf3d2
Gerrit-Change-Number: 20548
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 18 Oct 2023 12:28:48 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-11853: Fix formatted docs query options CSS

2023-10-18 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20556 )

Change subject: IMPALA-11853: Fix formatted docs query options CSS
..


Patch Set 1: Code-Review+2

Thank you for the reviews!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icec787d7a2af848aaaff65be2ecf311a5ce8fe7f
Gerrit-Change-Number: 20556
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Wed, 18 Oct 2023 10:21:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-11853: Fix formatted docs query options CSS

2023-10-18 Thread Tamas Mate (Code Review)
Tamas Mate has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20556 )

Change subject: IMPALA-11853: Fix formatted docs query options CSS
..

IMPALA-11853: Fix formatted docs query options CSS

The recent documentation formatting changes introduced the navigation
panel on the left. However, due to the length of the query options
navigation title these could overlap with the documentation paragraphs.

This commit removes the underscores from the navigation titles of the
query options, so browsers can break them into multiple lines.
Additionally, the "SET" and "Query Options for the SET Statement" pages
are merged to save some more space for the query option navigation
titles.

Testing:
 - Built the documentation and tested manually

Change-Id: Icec787d7a2af848aaaff65be2ecf311a5ce8fe7f
Reviewed-on: http://gerrit.cloudera.org:8080/20556
Tested-by: Impala Public Jenkins 
Reviewed-by: Jason Fehr 
Reviewed-by: Peter Rozsa 
Reviewed-by: Tamas Mate 
---
M docs/impala.ditamap
M docs/impala_keydefs.ditamap
M docs/topics/impala_abort_on_error.xml
M docs/topics/impala_allow_erasure_coded_files.xml
M docs/topics/impala_allow_unsupported_formats.xml
M docs/topics/impala_appx_count_distinct.xml
M docs/topics/impala_batch_size.xml
M docs/topics/impala_broadcast_bytes_limit.xml
M docs/topics/impala_buffer_pool_limit.xml
M docs/topics/impala_compression_codec.xml
M docs/topics/impala_compute_stats_min_sample_size.xml
M docs/topics/impala_config_options.xml
M docs/topics/impala_debug_action.xml
M docs/topics/impala_decimal_v2.xml
M docs/topics/impala_default_file_format.xml
M docs/topics/impala_default_hints_insert_statement.xml
M docs/topics/impala_default_join_distribution_mode.xml
M docs/topics/impala_default_spillable_buffer_size.xml
M docs/topics/impala_default_transactional_type.xml
M docs/topics/impala_delete_stats_in_truncate.xml
M docs/topics/impala_disable_codegen.xml
M docs/topics/impala_disable_codegen_rows_threshold.xml
M docs/topics/impala_disable_hbase_num_rows_estimate.xml
M docs/topics/impala_disable_row_runtime_filtering.xml
M docs/topics/impala_disable_streaming_preaggregations.xml
M docs/topics/impala_disable_unsafe_spills.xml
M docs/topics/impala_enable_expr_rewrites.xml
M docs/topics/impala_exec_single_node_rows_threshold.xml
M docs/topics/impala_exec_time_limit_s.xml
M docs/topics/impala_expand_complex_types.xml
M docs/topics/impala_explain_level.xml
M docs/topics/impala_fetch_rows_timeout_ms.xml
M docs/topics/impala_hbase_cache_blocks.xml
M docs/topics/impala_hbase_caching.xml
M docs/topics/impala_idle_session_timeout.xml
M docs/topics/impala_join_rows_produced_limit.xml
M docs/topics/impala_kudu_read_mode.xml
M docs/topics/impala_live_progress.xml
M docs/topics/impala_live_summary.xml
M docs/topics/impala_max_errors.xml
M docs/topics/impala_max_mem_estimate_for_admission.xml
M docs/topics/impala_max_num_runtime_filters.xml
M docs/topics/impala_max_result_spooling_mem.xml
M docs/topics/impala_max_row_size.xml
M docs/topics/impala_max_scan_range_length.xml
M docs/topics/impala_max_spilled_result_spooling_mem.xml
M docs/topics/impala_mem_limit.xml
M docs/topics/impala_min_spillable_buffer_size.xml
M docs/topics/impala_mt_dop.xml
M docs/topics/impala_num_nodes.xml
M docs/topics/impala_num_rows_produced_limit.xml
M docs/topics/impala_num_scanner_threads.xml
M docs/topics/impala_optimize_partition_key_scans.xml
M docs/topics/impala_parquet_annotate_strings_utf8.xml
M docs/topics/impala_parquet_array_resolution.xml
M docs/topics/impala_parquet_compression_codec.xml
M docs/topics/impala_parquet_dictionary_filtering.xml
M docs/topics/impala_parquet_fallback_schema_resolution.xml
M docs/topics/impala_parquet_file_size.xml
M docs/topics/impala_parquet_object_store_split_size.xml
M docs/topics/impala_parquet_page_row_count_limit.xml
M docs/topics/impala_parquet_read_page_index.xml
M docs/topics/impala_parquet_read_statistics.xml
M docs/topics/impala_parquet_write_page_index.xml
M docs/topics/impala_prefetch_mode.xml
D docs/topics/impala_query_options.xml
M docs/topics/impala_query_timeout_s.xml
M docs/topics/impala_refresh_updated_hms.xml
M docs/topics/impala_replica_preference.xml
M docs/topics/impala_request_pool.xml
M docs/topics/impala_resource_trace_ratio.xml
M docs/topics/impala_retry_failed_queries.xml
M docs/topics/impala_runtime_bloom_filter_size.xml
M docs/topics/impala_runtime_filter_max_size.xml
M docs/topics/impala_runtime_filter_min_size.xml
M docs/topics/impala_runtime_filter_mode.xml
M docs/topics/impala_runtime_filter_wait_time_ms.xml
M docs/topics/impala_s3_skip_insert_staging.xml
M docs/topics/impala_scan_bytes_limit.xml
M docs/topics/impala_schedule_random_replica.xml
M docs/topics/impala_scratch_limit.xml
M docs/topics/impala_seq_compression_mode.xml
M docs/topics/impala_set.xml
M docs/topics/impala_shell_commands.xml
M docs/topics/impala_shuffle_distinct_exprs.xml
M docs/topics/impala_spool_quer

[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-15 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#13). ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..

IMPALA-11996: Scanner change for Iceberg metadata querying

This commit adds a scan node for querying Iceberg metadata tables. The
scan node creates a Java scanner object that creates and scans the
metadata table. The scanner uses the Iceberg API to scan the table after
that the scan node fetches the rows one by one and materialises them
into RowBatches. The Iceberg row reader on the backend does the
translation between Iceberg and Impala types.

There is only one fragment created to query the Iceberg metadata table
which is supposed to be executed on the coordinator node that already
has the Iceberg table loaded. This way there is no need for further
table loading on the executor side.

This change will not cover nested column types, these slots are set to
NULL, it will be done in IMPALA-12205.

Testing:
 - Added e2e tests for querying metadata tables
 - Updated planner tests

Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
---
M be/CMakeLists.txt
M be/src/exec/CMakeLists.txt
M be/src/exec/exec-node.cc
A be/src/exec/iceberg-metadata/CMakeLists.txt
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-row-reader.cc
A be/src/exec/iceberg-metadata/iceberg-row-reader.h
M be/src/scheduling/scheduler.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impalad-main.cc
M be/src/util/jni-util.cc
M be/src/util/jni-util.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M 
testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/authorization/test_ranger.py
M tests/query_test/test_iceberg.py
32 files changed, 1,420 insertions(+), 166 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 13
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-15 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..


Patch Set 13:

Fixed clang tidy errors.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 13
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Sun, 15 Oct 2023 15:09:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-15 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#12). ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..

IMPALA-11996: Scanner change for Iceberg metadata querying

This commit adds a scan node for querying Iceberg metadata tables. The
scan node creates a Java scanner object that creates and scans the
metadata table. The scanner uses the Iceberg API to scan the table after
that the scan node fetches the rows one by one and materialises them
into RowBatches. The Iceberg row reader on the backend does the
translation between Iceberg and Impala types.

There is only one fragment created to query the Iceberg metadata table
which is supposed to be executed on the coordinator node that already
has the Iceberg table loaded. This way there is no need for further
table loading on the executor side.

This change will not cover nested column types, these slots are set to
NULL, it will be done in IMPALA-12205.

Testing:
 - Added e2e tests for querying metadata tables
 - Updated planner tests

Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
---
M be/CMakeLists.txt
M be/src/exec/CMakeLists.txt
M be/src/exec/exec-node.cc
A be/src/exec/iceberg-metadata/CMakeLists.txt
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-row-reader.cc
A be/src/exec/iceberg-metadata/iceberg-row-reader.h
M be/src/scheduling/scheduler.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impalad-main.cc
M be/src/util/jni-util.cc
M be/src/util/jni-util.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M 
testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/authorization/test_ranger.py
M tests/query_test/test_iceberg.py
32 files changed, 1,419 insertions(+), 166 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 12
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-15 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20010/11/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/20010/11/tests/authorization/test_ranger.py@2060
PS11, Line 2060: =
> flake8: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Sun, 15 Oct 2023 12:41:24 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-15 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..


Patch Set 11:

(59 comments)

Hi Peter and Gabor, thank you for the detailed review, updated the change. Let 
me know your thoughts.

http://gerrit.cloudera.org:8080/#/c/20010/10//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20010/10//COMMIT_MSG@21
PS10, Line 21: nested column types
> You still only mention struct type, however, other nested types are affecte
Done


http://gerrit.cloudera.org:8080/#/c/20010/10//COMMIT_MSG@24
PS10, Line 24: Testing:
> I do recall something about a perf test to see how long it takes to query s
It was quite slow to build a table like that so I just skipped it for now.


http://gerrit.cloudera.org:8080/#/c/20010/10//COMMIT_MSG@25
PS10, Line 25:  - Added e2e tests for querying metadata tables
> I'd really like to see some auth tests as well to see that changing privile
Done


http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/exec-node.cc
File be/src/exec/exec-node.cc:

http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/exec-node.cc@55
PS9, Line 55: #include "exec/unnest-node.h"
> nit: could go to L41
Done


http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
File be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h:

http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@33
PS10, Line 33: /// Iceberg API provides predefined metadata tables, these 
tables can be scanned through
> I really like this description of the ScanNode. Thanks!
Thanks!


http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@35
PS10, Line 35: ner to scan Iceberg data, due to the virtua
> nit: "just like any other regular Iceberg tables"?
Done


http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@39
PS10, Line 39: s
> nit: 'an'
Done


http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@69
PS10, Line 69: SED_RESULT;
> Would it cause any harm to call this multiple times? If yes, can we return
Done


http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@89
PS10, Line 89: scan_metadata_table_ = nullptr
> nit: this is the constructor, right? Can we name this 'iceberg_metadata_sca
Done


http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
File be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h:

http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@23
PS9, Line 23: #include 
> Unused include
Done


http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@35
PS9, Line 35: /// its Parquet scanner to scan Iceberg data, due to the virtual 
nature of the metadata
> nit: Parquet
Done


http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
File be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc:

http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@41
PS9, Line 41:
> nit: ; not needed
Done


http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
File be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc:

http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@71
PS10, Line 71:   if (tuple_desc_ == nullptr) {
> Could you include more info here for easier debugging, e.g. tuple Id?
Done


http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@90
PS10, Line 90: d_global_ref;
> I see aboce that after calling this function you also called "RETURN_ERROR_
RETURN_ERROR_IF_EXC checks the environment, LocalToGlobalRef is a wrapper 
around the JNI NewGlobalRef and env is checked in it. At this point, in case 
there is an exception it is already an Impala Status.


http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@131
PS10, Line 131: row_batch->tuple_data_pool(
> You've already checked for nullness few lines above.
Done


http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@165
PS10, Line 165:
> You don't use 'env' in this function. Anyway, Frontend::GetCatalogTable() c
Done


http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@168
PS10, Line 168:   RETURN_IF_ERROR(fe->GetCatalogTable(table_name_, jtable));
> Should we check if 'jtable' is nullptr after this call or would we get an e
Good point, added a DCHECK t

[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying

2023-10-15 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded a new patch set (#11). ( 
http://gerrit.cloudera.org:8080/20010 )

Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..

IMPALA-11996: Scanner change for Iceberg metadata querying

This commit adds a scan node for querying Iceberg metadata tables. The
scan node creates a Java scanner object that creates and scans the
metadata table. The scanner uses the Iceberg API to scan the table after
that the scan node fetches the rows one by one and materialises them
into RowBatches. The Iceberg row reader on the backend does the
translation between Iceberg and Impala types.

There is only one fragment created to query the Iceberg metadata table
which is supposed to be executed on the coordinator node that already
has the Iceberg table loaded. This way there is no need for further
table loading on the executor side.

This change will not cover nested column types, these slots are set to
NULL, it will be done in IMPALA-12205.

Testing:
 - Added e2e tests for querying metadata tables
 - Updated planner tests

Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
---
M be/CMakeLists.txt
M be/src/exec/CMakeLists.txt
M be/src/exec/exec-node.cc
A be/src/exec/iceberg-metadata/CMakeLists.txt
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
A be/src/exec/iceberg-metadata/iceberg-row-reader.cc
A be/src/exec/iceberg-metadata/iceberg-row-reader.h
M be/src/scheduling/scheduler.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impalad-main.cc
M be/src/util/jni-util.cc
M be/src/util/jni-util.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M 
testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/authorization/test_ranger.py
M tests/query_test/test_iceberg.py
32 files changed, 1,420 insertions(+), 168 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Gergely Fürnstáhl 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-11853: Fix formatted docs query options CSS

2023-10-10 Thread Tamas Mate (Code Review)
Tamas Mate has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20556


Change subject: IMPALA-11853: Fix formatted docs query options CSS
..

IMPALA-11853: Fix formatted docs query options CSS

The recent documentation formatting changes introduced the navigation
panel on the left. However, due to the length of the query options
navigation title these could overlap with the documentation paragraphs.

This commit removes the underscores from the navigation titles of the
query options, so browsers can break them into multiple lines.
Additionally, the "SET" and "Query Options for the SET Statement" pages
are merged to save some more space for the query option navigation
titles.

Testing:
 - Built the documentation and tested manually

Change-Id: Icec787d7a2af848aaaff65be2ecf311a5ce8fe7f
---
M docs/impala.ditamap
M docs/impala_keydefs.ditamap
M docs/topics/impala_abort_on_error.xml
M docs/topics/impala_allow_erasure_coded_files.xml
M docs/topics/impala_allow_unsupported_formats.xml
M docs/topics/impala_appx_count_distinct.xml
M docs/topics/impala_batch_size.xml
M docs/topics/impala_broadcast_bytes_limit.xml
M docs/topics/impala_buffer_pool_limit.xml
M docs/topics/impala_compression_codec.xml
M docs/topics/impala_compute_stats_min_sample_size.xml
M docs/topics/impala_config_options.xml
M docs/topics/impala_debug_action.xml
M docs/topics/impala_decimal_v2.xml
M docs/topics/impala_default_file_format.xml
M docs/topics/impala_default_hints_insert_statement.xml
M docs/topics/impala_default_join_distribution_mode.xml
M docs/topics/impala_default_spillable_buffer_size.xml
M docs/topics/impala_default_transactional_type.xml
M docs/topics/impala_delete_stats_in_truncate.xml
M docs/topics/impala_disable_codegen.xml
M docs/topics/impala_disable_codegen_rows_threshold.xml
M docs/topics/impala_disable_hbase_num_rows_estimate.xml
M docs/topics/impala_disable_row_runtime_filtering.xml
M docs/topics/impala_disable_streaming_preaggregations.xml
M docs/topics/impala_disable_unsafe_spills.xml
M docs/topics/impala_enable_expr_rewrites.xml
M docs/topics/impala_exec_single_node_rows_threshold.xml
M docs/topics/impala_exec_time_limit_s.xml
M docs/topics/impala_expand_complex_types.xml
M docs/topics/impala_explain_level.xml
M docs/topics/impala_fetch_rows_timeout_ms.xml
M docs/topics/impala_hbase_cache_blocks.xml
M docs/topics/impala_hbase_caching.xml
M docs/topics/impala_idle_session_timeout.xml
M docs/topics/impala_join_rows_produced_limit.xml
M docs/topics/impala_kudu_read_mode.xml
M docs/topics/impala_live_progress.xml
M docs/topics/impala_live_summary.xml
M docs/topics/impala_max_errors.xml
M docs/topics/impala_max_mem_estimate_for_admission.xml
M docs/topics/impala_max_num_runtime_filters.xml
M docs/topics/impala_max_result_spooling_mem.xml
M docs/topics/impala_max_row_size.xml
M docs/topics/impala_max_scan_range_length.xml
M docs/topics/impala_max_spilled_result_spooling_mem.xml
M docs/topics/impala_mem_limit.xml
M docs/topics/impala_min_spillable_buffer_size.xml
M docs/topics/impala_mt_dop.xml
M docs/topics/impala_num_nodes.xml
M docs/topics/impala_num_rows_produced_limit.xml
M docs/topics/impala_num_scanner_threads.xml
M docs/topics/impala_optimize_partition_key_scans.xml
M docs/topics/impala_parquet_annotate_strings_utf8.xml
M docs/topics/impala_parquet_array_resolution.xml
M docs/topics/impala_parquet_compression_codec.xml
M docs/topics/impala_parquet_dictionary_filtering.xml
M docs/topics/impala_parquet_fallback_schema_resolution.xml
M docs/topics/impala_parquet_file_size.xml
M docs/topics/impala_parquet_object_store_split_size.xml
M docs/topics/impala_parquet_page_row_count_limit.xml
M docs/topics/impala_parquet_read_page_index.xml
M docs/topics/impala_parquet_read_statistics.xml
M docs/topics/impala_parquet_write_page_index.xml
M docs/topics/impala_prefetch_mode.xml
D docs/topics/impala_query_options.xml
M docs/topics/impala_query_timeout_s.xml
M docs/topics/impala_refresh_updated_hms.xml
M docs/topics/impala_replica_preference.xml
M docs/topics/impala_request_pool.xml
M docs/topics/impala_resource_trace_ratio.xml
M docs/topics/impala_retry_failed_queries.xml
M docs/topics/impala_runtime_bloom_filter_size.xml
M docs/topics/impala_runtime_filter_max_size.xml
M docs/topics/impala_runtime_filter_min_size.xml
M docs/topics/impala_runtime_filter_mode.xml
M docs/topics/impala_runtime_filter_wait_time_ms.xml
M docs/topics/impala_s3_skip_insert_staging.xml
M docs/topics/impala_scan_bytes_limit.xml
M docs/topics/impala_schedule_random_replica.xml
M docs/topics/impala_scratch_limit.xml
M docs/topics/impala_seq_compression_mode.xml
M docs/topics/impala_set.xml
M docs/topics/impala_shell_commands.xml
M docs/topics/impala_shuffle_distinct_exprs.xml
M docs/topics/impala_spool_query_results.xml
M docs/topics/impala_support_start_over.xml
M docs/topics/impala_sync_ddl.xml
M docs/topics/impala_thread_reservation_aggregate_limit.xml
M docs/topics/impa

[Impala-ASF-CR] IMPALA-12243: Add support for DROP PARTITION for Iceberg tables

2023-10-05 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20515 )

Change subject: IMPALA-12243: Add support for DROP PARTITION for Iceberg tables
..


Patch Set 2:

(6 comments)

Added some more comments.

http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
File 
fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java:

http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java@131
PS2, Line 131: if (purgePartition_) {
 :   throw new AnalysisException("Iceberg tables can't purge 
partitions");
 : }
I couldn't find a test for this code path.


http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java@146
PS2, Line 146:   if (icebergExpression == null) {
 : throw new AnalysisException(
 : "Invalid partition filtering expression: " + 
expr.toSql());
 :   }
I think it would be cleaner to propagate an exception instead of relying on 
null results.


http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java
File fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java:

http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java@41
PS2, Line 41:
nit: empty line or missing comments


http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/common/IcebergPartitionPredicateConverter.java
File 
fe/src/main/java/org/apache/impala/common/IcebergPartitionPredicateConverter.java:

http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/common/IcebergPartitionPredicateConverter.java@114
PS2, Line 114:   }
nit: missing empty line


http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java
File fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java:

http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@152
PS2, Line 152:
nit: indentation


http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@246
PS2, Line 246:   + "backend handle it.",
 :   ex);
nit: no need for line break.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a768ba2966f570454687e02e4e6d67df46741f9
Gerrit-Change-Number: 20515
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 05 Oct 2023 12:37:23 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12279: Bump Iceberg lib version to 1.3

2023-10-04 Thread Tamas Mate (Code Review)
Tamas Mate has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20529 )

Change subject: IMPALA-12279: Bump Iceberg lib version to 1.3
..

IMPALA-12279: Bump Iceberg lib version to 1.3

This bumps the toolchain version to upgrade the Iceberg library.

Testing:
 - Verified building Impala on Ubuntu 18 and ran the Iceberg tests.

Change-Id: Ieece75dc9764c691917c8ed052d1a7f9cc950ec2
Reviewed-on: http://gerrit.cloudera.org:8080/20529
Tested-by: Impala Public Jenkins 
Reviewed-by: Zoltan Borok-Nagy 
---
M bin/impala-config.sh
1 file changed, 11 insertions(+), 11 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Zoltan Borok-Nagy: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ieece75dc9764c691917c8ed052d1a7f9cc950ec2
Gerrit-Change-Number: 20529
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12243: Add support for DROP PARTITION for Iceberg tables

2023-10-03 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20515 )

Change subject: IMPALA-12243: Add support for DROP PARTITION for Iceberg tables
..


Patch Set 2:

(5 comments)

Hi Peter, nice change! Just did a quick first check. I will be back later today.

http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
File 
fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java:

http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java@62
PS2, Line 62:   /**
:* Stores files selected by Iceberg's partition filtering
:*/
nit: single-line comment


http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java@66
PS2, Line 66:   /**
:* Statistics for selected Iceberg partitions
:*/
nit: single-line comment


http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java@114
PS2, Line 114: this.analyzer_ = analyzer;
Do we need this for Alter statements? StmtBase does this but AlterTableStmt 
overwrites it.


http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@401
PS2, Line 401: AnalysisException
I believe, this is a bit confusing, we should not throw an AnalysisException 
during table load. I am still looking into what made this change necessary.


http://gerrit.cloudera.org:8080/#/c/20515/2/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/20515/2/tests/query_test/test_iceberg.py@1125
PS2, Line 1125:
nit: empty line



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a768ba2966f570454687e02e4e6d67df46741f9
Gerrit-Change-Number: 20515
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 03 Oct 2023 10:09:24 +
Gerrit-HasComments: Yes


  1   2   3   4   5   6   7   >