Skip to content

Upgrade Iceberg to 0.11.0#7049

Merged
electrum merged 1 commit intotrinodb:masterfrom
jshmchenxi:iceberg-0.11.0
Mar 9, 2021
Merged

Upgrade Iceberg to 0.11.0#7049
electrum merged 1 commit intotrinodb:masterfrom
jshmchenxi:iceberg-0.11.0

Conversation

@jshmchenxi
Copy link
Copy Markdown

Iceberg fixed error with negative epoch values in 0.11.0. We should upgrade to this version to remove former workaround code.
Information from Iceberg release notes:

#1981 fixes bug that date and timestamp transforms were producing incorrect values for dates and times before 1970. Before the fix, negative values were incorrectly transformed by date and timestamp transforms to 1 larger than the correct value. For example, day(1969-12-31 10:00:00) produced 0 instead of -1. The fix is backwards compatible, which means predicate projection can still work with the incorrectly transformed partitions written using older versions.

@cla-bot
Copy link
Copy Markdown

cla-bot bot commented Feb 26, 2021

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Xi Chen.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot
Copy link
Copy Markdown

cla-bot bot commented Feb 26, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@cla-bot
Copy link
Copy Markdown

cla-bot bot commented Mar 4, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@jshmchenxi
Copy link
Copy Markdown
Author

@findepi Hello! I'm new to trino community and have sent a CLA to cla@trino.io days ago. I was wondering when will it take effect?

@findepi
Copy link
Copy Markdown
Member

findepi commented Mar 4, 2021

I was wondering when will it take effect?

@martint can answer this

@martint
Copy link
Copy Markdown
Member

martint commented Mar 4, 2021

@cla-bot check

@cla-bot cla-bot bot added the cla-signed label Mar 4, 2021
@cla-bot
Copy link
Copy Markdown

cla-bot bot commented Mar 4, 2021

The cla-bot has been summoned, and re-checked this pull request!

@jshmchenxi
Copy link
Copy Markdown
Author

Thank you! @findepi @martint

Copy link
Copy Markdown
Member

@phd3 phd3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one actionable comment. We should also update iceberg spark runtime version for product tests to keep the testing version for TestSparkCompatibility consistent, but that can be done separately.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: static import for toHiveType (since the return type is pretty clear from method name)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@phd3
Copy link
Copy Markdown
Member

phd3 commented Mar 8, 2021

@electrum FWIW even though this change does not exactly change user side behavior, we might want to add this to release notes, since after this upgrade, other engines on iceberg 0.9.0 may see incorrect results. For example, a table written by iceberg 0.11.0 may not be correctly scanned by 0.9.0 because of the Timestamp related change in apache/iceberg#1981 . (tables written by 0.9.0 can be still read correctly by 0.11.0 .)

-- Trino with 0.11.0
> CREATE TABLE test_hour_transform_with_11 (d TIMESTAMP(6), b BIGINT) WITH (partitioning = ARRAY['hour(d)']);
> INSERT INTO test_hour_transform_with_11 VALUES (TIMESTAMP '1969-12-31 23:25:05.222222', 50);
> SELECT * FROM test_hour_transform_with_11 WHERE d = TIMESTAMP '1969-12-31 23:25:05.222222';
             d              | b  
----------------------------+----
 1969-12-31 23:25:05.222222 | 50 
(1 row)

-- Incorrect scan with  Trino+iceberg 0.9.0
> SELECT * FROM test_hour_transform_with_11 WHERE d = TIMESTAMP '1969-12-31 23:25:05.222222';
 d | b 
---+---
(0 rows)

@jshmchenxi
Copy link
Copy Markdown
Author

LGTM, just one actionable comment. We should also update iceberg spark runtime version for product tests to keep the testing version for TestSparkCompatibility consistent, but that can be done separately.

@phd3 Hello! Thank you for the detailed review!
I searched throw the code, didn't find where to set iceberg spark runtime version. Any hints? :-)

@phd3
Copy link
Copy Markdown
Member

phd3 commented Mar 9, 2021

@jshmchenxi I've raised trinodb/docker-images#91 for this. We can update in Trino after a release there.

@electrum
Copy link
Copy Markdown
Member

electrum commented Mar 9, 2021

Thanks! I'm glad to hear this Iceberg issue (that I found when writing the partition transform code) has been fixed in such an elegant way.

@electrum electrum merged commit 69918c1 into trinodb:master Mar 9, 2021
@phd3 phd3 mentioned this pull request Mar 16, 2021
10 tasks
@martint martint added this to the 354 milestone Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants