Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to Iceberg 1.1.0 #15079

Merged
merged 1 commit into from
Dec 13, 2022
Merged

Update to Iceberg 1.1.0 #15079

merged 1 commit into from
Dec 13, 2022

Conversation

Fokko
Copy link
Contributor

@Fokko Fokko commented Nov 17, 2022

Description

Testing the new release of Iceberg 1.1.0 against the Trino tests. This shouldn't be merged

Non-technical explanation

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Nov 17, 2022
plugin/trino-iceberg/pom.xml Outdated Show resolved Hide resolved
try (Closeable ignored = writer) {
writer.delete(dataFilePath, 0, GenericRecord.create(icebergTable.schema()));
Copy link
Contributor Author

@Fokko Fokko Nov 17, 2022

Choose a reason for hiding this comment

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

.delete has been deprecated for a while, and removed in 1.1.0

@Fokko Fokko changed the title Plug in Apache Iceberg 1.1.0 RC2 Plug in Apache Iceberg 1.1.0 RC4 Nov 22, 2022
@Fokko Fokko force-pushed the fd-iceberg-1-1-0 branch 2 times, most recently from a4940e3 to ac1b635 Compare November 24, 2022 10:41
pom.xml Outdated Show resolved Hide resolved
testing/trino-faulttolerant-tests/pom.xml Outdated Show resolved Hide resolved
testing/trino-tests/pom.xml Outdated Show resolved Hide resolved
@findepi findepi changed the title Plug in Apache Iceberg 1.1.0 RC4 Update to Iceberg 1.1.0 Nov 28, 2022
@Fokko Fokko marked this pull request as ready for review November 29, 2022 19:27
@findepi
Copy link
Member

findepi commented Nov 30, 2022

CI result may be related

2022-11-29T21:41:57.0340138Z [ERROR] io.trino.plugin.iceberg.TestIcebergAvroConnectorTest.testJoinWithEmptySides[AUTOMATIC](6)  Time elapsed: 1.139 s  <<< FAILURE!
2022-11-29T21:41:57.0340998Z java.lang.AssertionError: Execution of 'actual' query failed: SELECT count(*) FROM nation JOIN region ON nation.regionkey = region.regionkey AND region.regionkey < 0
2022-11-29T21:41:57.0341442Z 	at org.testng.Assert.fail(Assert.java:83)
2022-11-29T21:41:57.0341822Z 	at io.trino.testing.QueryAssertions.assertQuery(QueryAssertions.java:150)
2022-11-29T21:41:57.0342274Z 	at io.trino.testing.QueryAssertions.assertQuery(QueryAssertions.java:106)
2022-11-29T21:41:57.0342771Z 	at io.trino.testing.AbstractTestQueryFramework.assertQuery(AbstractTestQueryFramework.java:309)
2022-11-29T21:41:57.0343449Z 	at io.trino.testing.BaseConnectorTest.testJoinWithEmptySides(BaseConnectorTest.java:713)
2022-11-29T21:41:57.0343955Z 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
2022-11-29T21:41:57.0344476Z 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
2022-11-29T21:41:57.0345060Z 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
2022-11-29T21:41:57.0345541Z 	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
2022-11-29T21:41:57.0346003Z 	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:104)
2022-11-29T21:41:57.0346508Z 	at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:54)
2022-11-29T21:41:57.0346970Z 	at org.testng.internal.InvokeMethodRunnable.run(InvokeMethodRunnable.java:44)
2022-11-29T21:41:57.0347412Z 	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
2022-11-29T21:41:57.0347906Z 	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
2022-11-29T21:41:57.0348367Z 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
2022-11-29T21:41:57.0348863Z 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
2022-11-29T21:41:57.0349342Z 	at java.base/java.lang.Thread.run(Thread.java:833)
2022-11-29T21:41:57.0350080Z Caused by: io.trino.testing.QueryFailedException: Cannot invoke "org.apache.iceberg.io.CloseableIterable.close()" because "this.currentIterable" is null
2022-11-29T21:41:57.0350675Z 	at io.trino.testing.AbstractTestingTrinoClient.execute(AbstractTestingTrinoClient.java:122)
2022-11-29T21:41:57.0351259Z 	at io.trino.testing.DistributedQueryRunner.execute(DistributedQueryRunner.java:490)
2022-11-29T21:41:57.0351714Z 	at io.trino.testing.QueryAssertions.assertQuery(QueryAssertions.java:147)
2022-11-29T21:41:57.0352013Z 	... 15 more
2022-11-29T21:41:57.0352414Z 	Suppressed: java.lang.Exception: SQL: SELECT count(*) FROM nation JOIN region ON nation.regionkey = region.regionkey AND region.regionkey < 0
2022-11-29T21:41:57.0353010Z 		at io.trino.testing.DistributedQueryRunner.execute(DistributedQueryRunner.java:493)
2022-11-29T21:41:57.0353326Z 		... 16 more
2022-11-29T21:41:57.0353760Z Caused by: java.lang.NullPointerException: Cannot invoke "org.apache.iceberg.io.CloseableIterable.close()" because "this.currentIterable" is null
2022-11-29T21:41:57.0354459Z 	at org.apache.iceberg.io.CloseableIterable$ConcatCloseableIterable$ConcatCloseableIterator.close(CloseableIterable.java:282)
2022-11-29T21:41:57.0354968Z 	at org.apache.iceberg.io.CloseableGroup.close(CloseableGroup.java:80)
2022-11-29T21:41:57.0355477Z 	at org.apache.iceberg.io.CloseableIterable$3.close(CloseableIterable.java:93)
2022-11-29T21:41:57.0355908Z 	at org.apache.iceberg.io.CloseableIterable$2.close(CloseableIterable.java:67)
2022-11-29T21:41:57.0356302Z 	at com.google.common.io.Closer.close(Closer.java:218)
2022-11-29T21:41:57.0356720Z 	at io.trino.plugin.iceberg.IcebergSplitSource.close(IcebergSplitSource.java:315)
2022-11-29T21:41:57.0357189Z 	at io.trino.plugin.iceberg.IcebergSplitSource.finish(IcebergSplitSource.java:290)
2022-11-29T21:41:57.0357685Z 	at io.trino.plugin.iceberg.IcebergSplitSource.getNextBatch(IcebergSplitSource.java:202)
2022-11-29T21:41:57.0358353Z 	at io.trino.plugin.base.classloader.ClassLoaderSafeConnectorSplitSource.getNextBatch(ClassLoaderSafeConnectorSplitSource.java:44)
2022-11-29T21:41:57.0359019Z 	at io.trino.split.ConnectorAwareSplitSource.getNextBatch(ConnectorAwareSplitSource.java:53)
2022-11-29T21:41:57.0359533Z 	at io.trino.split.BufferingSplitSource$GetNextBatch.fetchSplits(BufferingSplitSource.java:105)
2022-11-29T21:41:57.0360050Z 	at io.trino.split.BufferingSplitSource$GetNextBatch.fetchNextBatchAsync(BufferingSplitSource.java:88)
2022-11-29T21:41:57.0360555Z 	at io.trino.split.BufferingSplitSource.getNextBatch(BufferingSplitSource.java:52)
2022-11-29T21:41:57.0361104Z 	at io.trino.execution.scheduler.SourcePartitionedScheduler.schedule(SourcePartitionedScheduler.java:247)
2022-11-29T21:41:57.0361715Z 	at io.trino.execution.scheduler.SourcePartitionedScheduler$1.schedule(SourcePartitionedScheduler.java:172)
2022-11-29T21:41:57.0362351Z 	at io.trino.execution.scheduler.PipelinedQueryScheduler$DistributedStagesScheduler.schedule(PipelinedQueryScheduler.java:1230)
2022-11-29T21:41:57.0362908Z 	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
2022-11-29T21:41:57.0363323Z 	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
2022-11-29T21:41:57.0363778Z 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
2022-11-29T21:41:57.0364274Z 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
2022-11-29T21:41:57.0364655Z 	at java.base/java.lang.Thread.run(Thread.java:833)

@nastra
Copy link
Contributor

nastra commented Nov 30, 2022

I've opened apache/iceberg#6322 to fix the NPE, which is unfortunate that we didn't see it earlier.
There is a less-than-ideal workaround however that we could use to avoid the NPE while waiting for the PR to be merged+released by calling hasNext() once on the CloseableIterator (fileScanTaskIterator in this particular case)

@nastra nastra force-pushed the fd-iceberg-1-1-0 branch 3 times, most recently from e501033 to c8ebb3c Compare November 30, 2022 12:47
dongjoon-hyun pushed a commit to apache/orc that referenced this pull request Dec 6, 2022
### Why are the changes needed?

We're seeing some conflicts in the proto files when updating Iceberg in Trino: trinodb/trino#15079 (comment)

I think we should exclude the proto files from the `nohive` jar.

Before the change:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) ✗ unzip orc-core-1.9.0-SNAPSHOT-nohive.jar
➜  target git:(main) find . | grep -i "google/protobuf/"
./google/protobuf/timestamp.proto
./google/protobuf/field_mask.proto
./google/protobuf/api.proto
./google/protobuf/duration.proto
./google/protobuf/struct.proto
./google/protobuf/wrappers.proto
./google/protobuf/source_context.proto
./google/protobuf/any.proto
./google/protobuf/type.proto
./google/protobuf/empty.proto
./google/protobuf/compiler
./google/protobuf/compiler/plugin.proto
./google/protobuf/descriptor.proto
```

After the change:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) ✗ unzip orc-core-1.9.0-SNAPSHOT-nohive.jar
➜  target git:(main) ✗ find . | grep -i "google/protobuf/"
∅
```

Interestingly enough, the normal jar doesn't contain these proto files:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) unzip orc-core-1.9.0-SNAPSHOT.jar| grep -i proto
➜  target git:(main) find . | grep -i "google/protobuf/"
∅
```

### What changes were proposed in this pull request?

Exclude the proto files

### How was this patch tested?

Inspecting the produced jar

Closes #1333

Closes #1334 from Fokko/fd-exclude-proto-files.

Authored-by: Fokko Driesprong <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun pushed a commit to apache/orc that referenced this pull request Dec 6, 2022
### Why are the changes needed?

We're seeing some conflicts in the proto files when updating Iceberg in Trino: trinodb/trino#15079 (comment)

I think we should exclude the proto files from the `nohive` jar.

Before the change:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) ✗ unzip orc-core-1.9.0-SNAPSHOT-nohive.jar
➜  target git:(main) find . | grep -i "google/protobuf/"
./google/protobuf/timestamp.proto
./google/protobuf/field_mask.proto
./google/protobuf/api.proto
./google/protobuf/duration.proto
./google/protobuf/struct.proto
./google/protobuf/wrappers.proto
./google/protobuf/source_context.proto
./google/protobuf/any.proto
./google/protobuf/type.proto
./google/protobuf/empty.proto
./google/protobuf/compiler
./google/protobuf/compiler/plugin.proto
./google/protobuf/descriptor.proto
```

After the change:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) ✗ unzip orc-core-1.9.0-SNAPSHOT-nohive.jar
➜  target git:(main) ✗ find . | grep -i "google/protobuf/"
∅
```

Interestingly enough, the normal jar doesn't contain these proto files:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) unzip orc-core-1.9.0-SNAPSHOT.jar| grep -i proto
➜  target git:(main) find . | grep -i "google/protobuf/"
∅
```

### What changes were proposed in this pull request?

Exclude the proto files

### How was this patch tested?

Inspecting the produced jar

Closes #1333

Closes #1334 from Fokko/fd-exclude-proto-files.

Authored-by: Fokko Driesprong <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit f6fee07)
Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun pushed a commit to apache/orc that referenced this pull request Dec 6, 2022
We're seeing some conflicts in the proto files when updating Iceberg in Trino: trinodb/trino#15079 (comment)

I think we should exclude the proto files from the `nohive` jar.

Before the change:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) ✗ unzip orc-core-1.9.0-SNAPSHOT-nohive.jar
➜  target git:(main) find . | grep -i "google/protobuf/"
./google/protobuf/timestamp.proto
./google/protobuf/field_mask.proto
./google/protobuf/api.proto
./google/protobuf/duration.proto
./google/protobuf/struct.proto
./google/protobuf/wrappers.proto
./google/protobuf/source_context.proto
./google/protobuf/any.proto
./google/protobuf/type.proto
./google/protobuf/empty.proto
./google/protobuf/compiler
./google/protobuf/compiler/plugin.proto
./google/protobuf/descriptor.proto
```

After the change:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) ✗ unzip orc-core-1.9.0-SNAPSHOT-nohive.jar
➜  target git:(main) ✗ find . | grep -i "google/protobuf/"
∅
```

Interestingly enough, the normal jar doesn't contain these proto files:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) unzip orc-core-1.9.0-SNAPSHOT.jar| grep -i proto
➜  target git:(main) find . | grep -i "google/protobuf/"
∅
```

Exclude the proto files

Inspecting the produced jar

Closes #1333

Closes #1334 from Fokko/fd-exclude-proto-files.

Authored-by: Fokko Driesprong <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit f6fee07)
Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun pushed a commit to apache/orc that referenced this pull request Dec 6, 2022
We're seeing some conflicts in the proto files when updating Iceberg in Trino: trinodb/trino#15079 (comment)

I think we should exclude the proto files from the `nohive` jar.

Before the change:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) ✗ unzip orc-core-1.9.0-SNAPSHOT-nohive.jar
➜  target git:(main) find . | grep -i "google/protobuf/"
./google/protobuf/timestamp.proto
./google/protobuf/field_mask.proto
./google/protobuf/api.proto
./google/protobuf/duration.proto
./google/protobuf/struct.proto
./google/protobuf/wrappers.proto
./google/protobuf/source_context.proto
./google/protobuf/any.proto
./google/protobuf/type.proto
./google/protobuf/empty.proto
./google/protobuf/compiler
./google/protobuf/compiler/plugin.proto
./google/protobuf/descriptor.proto
```

After the change:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) ✗ unzip orc-core-1.9.0-SNAPSHOT-nohive.jar
➜  target git:(main) ✗ find . | grep -i "google/protobuf/"
∅
```

Interestingly enough, the normal jar doesn't contain these proto files:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) unzip orc-core-1.9.0-SNAPSHOT.jar| grep -i proto
➜  target git:(main) find . | grep -i "google/protobuf/"
∅
```

Exclude the proto files

Inspecting the produced jar

Closes #1333

Closes #1334 from Fokko/fd-exclude-proto-files.

Authored-by: Fokko Driesprong <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit f6fee07)
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 5ea8c06)
Signed-off-by: Dongjoon Hyun <[email protected]>
@Fokko
Copy link
Contributor Author

Fokko commented Dec 12, 2022

Just rebased against latest master

@Fokko
Copy link
Contributor Author

Fokko commented Dec 13, 2022

@findepi all green :)

@findepi findepi added the no-release-notes This pull request does not require release notes entry label Dec 13, 2022
@findepi findepi merged commit cd8eac6 into trinodb:master Dec 13, 2022
@findepi
Copy link
Member

findepi commented Dec 13, 2022

@Fokko i assume this provides no end-user visible changes, correct?

@Fokko Fokko deleted the fd-iceberg-1-1-0 branch December 13, 2022 15:52
@Fokko
Copy link
Contributor Author

Fokko commented Dec 13, 2022

Thanks for merging @findepi! No user-facing changes, so we should be good 👍🏻

@github-actions github-actions bot added this to the 404 milestone Dec 13, 2022
cxzl25 pushed a commit to cxzl25/orc that referenced this pull request Jan 11, 2024
### Why are the changes needed?

We're seeing some conflicts in the proto files when updating Iceberg in Trino: trinodb/trino#15079 (comment)

I think we should exclude the proto files from the `nohive` jar.

Before the change:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) ✗ unzip orc-core-1.9.0-SNAPSHOT-nohive.jar
➜  target git:(main) find . | grep -i "google/protobuf/"
./google/protobuf/timestamp.proto
./google/protobuf/field_mask.proto
./google/protobuf/api.proto
./google/protobuf/duration.proto
./google/protobuf/struct.proto
./google/protobuf/wrappers.proto
./google/protobuf/source_context.proto
./google/protobuf/any.proto
./google/protobuf/type.proto
./google/protobuf/empty.proto
./google/protobuf/compiler
./google/protobuf/compiler/plugin.proto
./google/protobuf/descriptor.proto
```

After the change:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) ✗ unzip orc-core-1.9.0-SNAPSHOT-nohive.jar
➜  target git:(main) ✗ find . | grep -i "google/protobuf/"
∅
```

Interestingly enough, the normal jar doesn't contain these proto files:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) unzip orc-core-1.9.0-SNAPSHOT.jar| grep -i proto
➜  target git:(main) find . | grep -i "google/protobuf/"
∅
```

### What changes were proposed in this pull request?

Exclude the proto files

### How was this patch tested?

Inspecting the produced jar

Closes apache#1333

Closes apache#1334 from Fokko/fd-exclude-proto-files.

Authored-by: Fokko Driesprong <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
cxzl25 pushed a commit to cxzl25/orc that referenced this pull request Jan 11, 2024
We're seeing some conflicts in the proto files when updating Iceberg in Trino: trinodb/trino#15079 (comment)

I think we should exclude the proto files from the `nohive` jar.

Before the change:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) ✗ unzip orc-core-1.9.0-SNAPSHOT-nohive.jar
➜  target git:(main) find . | grep -i "google/protobuf/"
./google/protobuf/timestamp.proto
./google/protobuf/field_mask.proto
./google/protobuf/api.proto
./google/protobuf/duration.proto
./google/protobuf/struct.proto
./google/protobuf/wrappers.proto
./google/protobuf/source_context.proto
./google/protobuf/any.proto
./google/protobuf/type.proto
./google/protobuf/empty.proto
./google/protobuf/compiler
./google/protobuf/compiler/plugin.proto
./google/protobuf/descriptor.proto
```

After the change:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) ✗ unzip orc-core-1.9.0-SNAPSHOT-nohive.jar
➜  target git:(main) ✗ find . | grep -i "google/protobuf/"
∅
```

Interestingly enough, the normal jar doesn't contain these proto files:
```
➜  java git:(main) ✗ rm -rf core/target/
➜  java git:(main) ✗ mvn package -DskipTests
➜  java git:(main) ✗ cd core/target
➜  target git:(main) unzip orc-core-1.9.0-SNAPSHOT.jar| grep -i proto
➜  target git:(main) find . | grep -i "google/protobuf/"
∅
```

Exclude the proto files

Inspecting the produced jar

Closes apache#1333

Closes apache#1334 from Fokko/fd-exclude-proto-files.

Authored-by: Fokko Driesprong <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit f6fee07)
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed no-release-notes This pull request does not require release notes entry
Development

Successfully merging this pull request may close these issues.

4 participants