[Impala-ASF-CR] IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

2024-05-28 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21429 )

Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in 
Iceberg metadata tables
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Gerrit-Change-Number: 21429
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Comment-Date: Tue, 28 May 2024 14:36:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

2024-05-28 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/21429 )

Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in 
Iceberg metadata tables
..

IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

DECIMAL values are not supported in Iceberg metadata tables and Impala
runs on a DCHECK and crashes if it encounters one.

Until this issue is properly fixed (see IMPALA-13080), this commit
introduces a temporary solution: DECIMAL values coming from Iceberg
metadata tables are NULLed out and a warning is issued.

Testing:
 - added a DECIMAL column to the 'iceberg_metadata_alltypes' test table,
   so querying the `files` metadata table will include a DECIMAL in the
   'readable_metrics' struct.

Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Reviewed-on: http://gerrit.cloudera.org:8080/21429
Reviewed-by: Daniel Becker 
Tested-by: Impala Public Jenkins 
---
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M testdata/datasets/functional/functional_schema_template.sql
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
4 files changed, 31 insertions(+), 5 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Gerrit-Change-Number: 21429
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 


[Impala-ASF-CR] IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

2024-05-28 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21429 )

Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in 
Iceberg metadata tables
..


Patch Set 5:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/16228/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Gerrit-Change-Number: 21429
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Comment-Date: Tue, 28 May 2024 09:52:38 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

2024-05-28 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21429 )

Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in 
Iceberg metadata tables
..


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Gerrit-Change-Number: 21429
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Comment-Date: Tue, 28 May 2024 09:27:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

2024-05-28 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/21429 )

Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in 
Iceberg metadata tables
..

IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

DECIMAL values are not supported in Iceberg metadata tables and Impala
runs on a DCHECK and crashes if it encounters one.

Until this issue is properly fixed (see IMPALA-13080), this commit
introduces a temporary solution: DECIMAL values coming from Iceberg
metadata tables are NULLed out and a warning is issued.

Testing:
 - added a DECIMAL column to the 'iceberg_metadata_alltypes' test table,
   so querying the `files` metadata table will include a DECIMAL in the
   'readable_metrics' struct.

Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
---
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M testdata/datasets/functional/functional_schema_template.sql
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
4 files changed, 31 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Gerrit-Change-Number: 21429
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 


[Impala-ASF-CR] IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

2024-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21429 )

Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in 
Iceberg metadata tables
..


Patch Set 4: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Gerrit-Change-Number: 21429
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Comment-Date: Tue, 21 May 2024 14:16:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

2024-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21429 )

Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in 
Iceberg metadata tables
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Gerrit-Change-Number: 21429
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Comment-Date: Tue, 21 May 2024 09:12:23 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

2024-05-21 Thread Peter Rozsa (Code Review)
Peter Rozsa has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21429 )

Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in 
Iceberg metadata tables
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Gerrit-Change-Number: 21429
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Comment-Date: Tue, 21 May 2024 07:32:32 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

2024-05-16 Thread Peter Rozsa (Code Review)
Peter Rozsa has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21429 )

Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in 
Iceberg metadata tables
..


Patch Set 3: Code-Review+1

Thank you!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Gerrit-Change-Number: 21429
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Comment-Date: Thu, 16 May 2024 13:37:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

2024-05-16 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/21429 )

Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in 
Iceberg metadata tables
..

IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

DECIMAL values are not supported in Iceberg metadata tables and Impala
runs on a DCHECK and crashes if it encounters one.

Until this issue is properly fixed (see IMPALA-13080), this commit
introduces a temporary solution: DECIMAL values coming from Iceberg
metadata tables are NULLed out and a warning is issued.

Testing:
 - added a DECIMAL column to the 'iceberg_metadata_alltypes' test table,
   so querying the `files` metadata table will include a DECIMAL in the
   'readable_metrics' struct.

Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
---
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M be/src/exec/iceberg-metadata/iceberg-row-reader.h
M testdata/datasets/functional/functional_schema_template.sql
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
4 files changed, 31 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Gerrit-Change-Number: 21429
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Peter Rozsa 


[Impala-ASF-CR] IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

2024-05-16 Thread Daniel Becker (Code Review)
Daniel Becker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21429 )

Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in 
Iceberg metadata tables
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21429/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/21429/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@125
PS1, Line 125:
> represented as NULL, or displayed as NULL. NULLed out sounds a bit unusual.
Done


http://gerrit.cloudera.org:8080/#/c/21429/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@125
PS1, Line 125:   RETURN_IF_ERROR(WriteDecimalSlot(slot_desc, tuple, state));
> The warning string could be extracted as a constant.
I made it a constexpr char array. I don't think it would make sense to extract 
it out of this function, it is only used here and makes the function more 
readable.


http://gerrit.cloudera.org:8080/#/c/21429/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@126
PS1, Line 126:   break;
> With this approach, the warning will be posted as many times the query enco
I introduced a member that tracks whether the warning has already been emitted. 
Therefore one warning will be emitted for each scanner that encounters a 
DECIMAL from a metadata table.

Also changed the type of the error from TErrorCode::GENERAL to 
TErrorCode::NOT_IMPLEMENTED_ERROR so these errors will be aggregated and 
displayed only once even if there are multiple scanners reading DECIMALs from 
metadata tables.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Gerrit-Change-Number: 21429
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Comment-Date: Thu, 16 May 2024 11:01:54 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

2024-05-16 Thread Peter Rozsa (Code Review)
Peter Rozsa has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21429 )

Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in 
Iceberg metadata tables
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21429/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/21429/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@125
PS1, Line 125: are NULLed out.
represented as NULL, or displayed as NULL. NULLed out sounds a bit unusual.


http://gerrit.cloudera.org:8080/#/c/21429/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@125
PS1, Line 125:   string warning = "DECIMAL values from Iceberg metadata 
tables are NULLed out.";
The warning string could be extracted as a constant.


http://gerrit.cloudera.org:8080/#/c/21429/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@126
PS1, Line 126:   LOG(WARNING) << warning;
With this approach, the warning will be posted as many times the query 
encounters a decimal, it would be better to post the warning once.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Gerrit-Change-Number: 21429
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Peter Rozsa 
Gerrit-Comment-Date: Thu, 16 May 2024 09:03:34 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

2024-05-15 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/21429


Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in 
Iceberg metadata tables
..

IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables

DECIMAL values are not supported in Iceberg metadata tables and Impala
runs on a DCHECK and crashes if it encounters one.

Until this issue is properly fixed (see IMPALA-13080), this commit
introduces a temporary solution: DECIMAL values coming from Iceberg
metadata tables are NULLed out and a warning is issued.

Testing:
 - added a DECIMAL column to the 'iceberg_metadata_alltypes' test table,
   so querying the `files` metadata table will include a DECIMAL in the
   'readable_metrics' struct.

Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
---
M be/src/exec/iceberg-metadata/iceberg-row-reader.cc
M testdata/datasets/functional/functional_schema_template.sql
M 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
3 files changed, 13 insertions(+), 4 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22
Gerrit-Change-Number: 21429
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker