Bump Iceberg and Nessie versions #21034

ajantha-bhat · 2024-03-12T13:36:11Z

Description

Bump Iceberg to 1.5.0 and Nessie to 0.77.1
Remove deprecated class usage
update ForwardingFileIo to handle API: Extend FileIO and add EncryptingFileIO. apache/iceberg#9592

Additional context and related issues

https://iceberg.apache.org/releases/#150-release

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.

nastra · 2024-03-12T14:02:11Z

...eberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/IcebergNessieTableOperations.java

@@ -126,7 +128,11 @@ protected void commitToExistingTable(TableMetadata base, TableMetadata metadata)
    {
        verify(version.orElseThrow() >= 0, "commitToExistingTable called on a new table");
        try {
-            nessieClient.commitTable(base, metadata, writeNewMetadata(metadata, version.getAsInt() + 1), table, toKey(new SchemaTableName(database, this.tableName)));
+            if (table == null) {
+                table = nessieClient.table(toIdentifier(new SchemaTableName(database, tableName)));


it's not clear to me why these changes are necessary

nessieClient.commitTable signature has changed. It just needs contentID instead of expected content.

ExpectedContent(aka table) was null before this PR also for commitToExistingTable. I have debug the original integration to understand it.

nastra

LGTM but it's not clear why the changes in IcebergNessieTableOperations are necessary

findinpath · 2024-03-12T20:08:37Z

Do we need to update the docker image before or we'll do it in a follow-up (see #19564 ) ?

cc @wendigo

findinpath · 2024-03-12T20:12:28Z

InputFile.length is being called since apache/iceberg#9592

Pls provide relevant code link in the 1.5.0 tag of the Iceberg code.

The InputFile.length extra-call for the manifests seems to be coming from

https://github.com/apache/iceberg/blob/43c3397528101859250160f123a0749bae79fb4d/core/src/main/java/org/apache/iceberg/avro/AvroIterable.java#L99-L102

I'm not sure though that apache/iceberg#9592 is the cause for the extra InputFile.length call.

findinpath · 2024-03-12T20:45:55Z

@wendigo could you please run the PR with secrets to see the output of the cloud tests as well?

ebyhr · 2024-03-12T21:39:07Z

/test-with-secrets sha=054aecc7acca690b80e048a7647c26924bb42fb8

github-actions · 2024-03-12T22:15:59Z

The CI workflow run with tests that require additional secrets has been started: https://github.com/trinodb/trino/actions/runs/8256356235

...eberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/IcebergNessieTableOperations.java

ebyhr · 2024-03-13T07:11:36Z

...eberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/IcebergNessieTableOperations.java

+        SchemaTableName schemaTableName = new SchemaTableName(databaseName, tableName);
+        return ContentKey.of(Namespace.parse(schemaTableName.getSchemaName()), schemaTableName.getTableName());


Why not pass databaseName & tableName to Namespace.parse() directly?

we miss the validation of empty schemaname and conversion of the identifiers into lower case.
I don't know the background. But I assumed it was written for that purpose. So, retained the SchemaTableName

nastra · 2024-03-13T07:28:47Z

InputFile.length is being called since apache/iceberg#9592

Pls provide relevant code link in the 1.5.0 tag of the Iceberg code.

The InputFile.length extra-call for the manifests seems to be coming from

https://github.com/apache/iceberg/blob/43c3397528101859250160f123a0749bae79fb4d/core/src/main/java/org/apache/iceberg/avro/AvroIterable.java#L99-L102

I'm not sure though that apache/iceberg#9592 is the cause for the extra InputFile.length call.

@findinpath I can confirm that those extra InputFile.length calls come from apache/iceberg#9592 because I adjusted the tests when we switched from 1.5.0 RC0 to RC1 (which is when that PR made it into the release)

plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergFileOperations.java

wendigo · 2024-03-14T11:53:09Z

LGTM @findepi @findinpath do you want to chime in?

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/fileio/ForwardingFileIo.java

plugin/trino-iceberg/pom.xml

- Bump Iceberg to 1.5.0 and Nessie to 0.77.1 - Remove deprecated usage - Fix regression when reading manifest file by overriding newInputfile and passing in the known length Co-authored-by: amogh-jahagirdar <[email protected]>

mosabua · 2024-03-21T03:28:03Z

This has no user facing impact and therefore needs to release notes entry? Can you confirm @wendigo and @ajantha-bhat ?

ajantha-bhat · 2024-03-21T04:45:00Z

I don't remember mentioning Nessie version bumps in the release notes.

ebyhr · 2024-03-21T04:48:40Z

@mosabua No need to mention in the release note.

cla-bot bot added the cla-signed label Mar 12, 2024

github-actions bot added the iceberg Iceberg connector label Mar 12, 2024

ajantha-bhat mentioned this pull request Mar 12, 2024

WIP: Test Upcoming Iceberg and Nessie versions #20957

Closed

ajantha-bhat requested a review from nastra March 12, 2024 13:37

nastra reviewed Mar 12, 2024

View reviewed changes

ajantha-bhat requested review from electrum, findepi and findinpath March 12, 2024 14:11

wendigo approved these changes Mar 12, 2024

View reviewed changes

findinpath requested a review from alexjo2144 March 12, 2024 19:46

ebyhr reviewed Mar 12, 2024

View reviewed changes

ajantha-bhat force-pushed the 1.5.0 branch from 054aecc to 520ff22 Compare March 13, 2024 07:08

ebyhr reviewed Mar 13, 2024

View reviewed changes

findepi reviewed Mar 13, 2024

View reviewed changes

plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergFileOperations.java Outdated Show resolved Hide resolved

ajantha-bhat force-pushed the 1.5.0 branch from bad4ac6 to 1761451 Compare March 14, 2024 01:51

findepi reviewed Mar 14, 2024

View reviewed changes

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/fileio/ForwardingFileIo.java Show resolved Hide resolved

plugin/trino-iceberg/pom.xml Outdated Show resolved Hide resolved

Bump Iceberg and Nessie versions

ec1d5dd

- Bump Iceberg to 1.5.0 and Nessie to 0.77.1 - Remove deprecated usage - Fix regression when reading manifest file by overriding newInputfile and passing in the known length Co-authored-by: amogh-jahagirdar <[email protected]>

ajantha-bhat force-pushed the 1.5.0 branch from 1761451 to ec1d5dd Compare March 15, 2024 07:58

findinpath approved these changes Mar 17, 2024

View reviewed changes

wendigo approved these changes Mar 17, 2024

View reviewed changes

wendigo merged commit 996debc into trinodb:master Mar 17, 2024
94 checks passed

github-actions bot added this to the 443 milestone Mar 17, 2024

ebyhr mentioned this pull request Mar 18, 2024

Add metadata_log_entries table to Iceberg #20410

Merged

mosabua mentioned this pull request Mar 21, 2024

Add Trino 443 release notes #21178

Merged

nastra deleted the 1.5.0 branch March 21, 2024 07:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump Iceberg and Nessie versions #21034

Bump Iceberg and Nessie versions #21034

ajantha-bhat commented Mar 12, 2024 •

edited

Loading

nastra Mar 12, 2024

ajantha-bhat Mar 12, 2024

nastra left a comment

findinpath commented Mar 12, 2024

findinpath commented Mar 12, 2024 •

edited

Loading

findinpath commented Mar 12, 2024

ebyhr commented Mar 12, 2024

github-actions bot commented Mar 12, 2024

ebyhr Mar 13, 2024

ajantha-bhat Mar 13, 2024

nastra commented Mar 13, 2024

wendigo commented Mar 14, 2024

mosabua commented Mar 21, 2024

ajantha-bhat commented Mar 21, 2024

ebyhr commented Mar 21, 2024

		SchemaTableName schemaTableName = new SchemaTableName(databaseName, tableName);
		return ContentKey.of(Namespace.parse(schemaTableName.getSchemaName()), schemaTableName.getTableName());

Bump Iceberg and Nessie versions #21034

Bump Iceberg and Nessie versions #21034

Conversation

ajantha-bhat commented Mar 12, 2024 • edited Loading

Description

Additional context and related issues

Release notes

nastra Mar 12, 2024

Choose a reason for hiding this comment

ajantha-bhat Mar 12, 2024

Choose a reason for hiding this comment

nastra left a comment

Choose a reason for hiding this comment

findinpath commented Mar 12, 2024

findinpath commented Mar 12, 2024 • edited Loading

findinpath commented Mar 12, 2024

ebyhr commented Mar 12, 2024

github-actions bot commented Mar 12, 2024

ebyhr Mar 13, 2024

Choose a reason for hiding this comment

ajantha-bhat Mar 13, 2024

Choose a reason for hiding this comment

nastra commented Mar 13, 2024

wendigo commented Mar 14, 2024

mosabua commented Mar 21, 2024

ajantha-bhat commented Mar 21, 2024

ebyhr commented Mar 21, 2024

ajantha-bhat commented Mar 12, 2024 •

edited

Loading

findinpath commented Mar 12, 2024 •

edited

Loading