You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "AndrewMakin (via GitHub)" <gi...@apache.org> on 2025/08/04 08:05:33 UTC
[PR] Properly handle UUID in partition field [iceberg]
AndrewMakin opened a new pull request, #13726:
URL: https://github.com/apache/iceberg/pull/13726
Closes #13703
Previously getUTF8StringInternal and collectionToArrayData assumed CharSequence was the only class it would have to handle. Now allow either CharSequence or UUID.
Added test to reproduce error case from issue. Below is output from one of these new tests failing against the existing code base:
```
testRewriteManifestsWithUUIDPartition()
org.apache.spark.SparkException: Job aborted due to stage failure: Task 0 in stage 7.0 failed 1 times, most recent failure: Lost task 0.0 in stage 7.0 (TID 6) (192.168.0.97 executor driver): java.lang.IllegalArgumentException: Wrong class, expected java.lang.CharSequence, but was java.util.UUID, for object: 849d9bcd-ed84-4bf2-8019-35b001ece9a3
at org.apache.iceberg.PartitionData.get(PartitionData.java:126)
at org.apache.iceberg.util.StructProjection.get(StructProjection.java:214)
at org.apache.iceberg.spark.source.StructInternalRow.getUTF8StringInternal(StructInternalRow.java:178)
at org.apache.iceberg.spark.source.StructInternalRow.getUTF8String(StructInternalRow.java:174)
at org.apache.spark.sql.catalyst.expressions.GeneratedClass$GeneratedIteratorForCodegenStage1.project_writeFields_1_0$(Unknown Source)
at org.apache.spark.sql.catalyst.expressions.GeneratedClass$GeneratedIteratorForCodegenStage1.processNext(Unknown Source)
at org.apache.spark.sql.execution.BufferedRowIterator.hasNext(BufferedRowIterator.java:43)
at org.apache.spark.sql.execution.WholeStageCodegenEvaluatorFactory$WholeStageCodegenPartitionEvaluator$$anon$1.hasNext(WholeStageCodegenEvaluatorFactory.scala:50)
at scala.collection.Iterator$$anon$9.hasNext(Iterator.scala:583)
at org.apache.spark.shuffle.sort.BypassMergeSortShuffleWriter.write(BypassMergeSortShuffleWriter.java:143)
at org.apache.spark.shuffle.ShuffleWriteProcessor.write(ShuffleWriteProcessor.scala:57)
at org.apache.spark.scheduler.ShuffleMapTask.runTask(ShuffleMapTask.scala:111)
at org.apache.spark.scheduler.ShuffleMapTask.runTask(ShuffleMapTask.scala:54)
at org.apache.spark.TaskContext.runTaskWithListeners(TaskContext.scala:171)
at org.apache.spark.scheduler.Task.run(Task.scala:147)
at org.apache.spark.executor.Executor$TaskRunner.$anonfun$run$5(Executor.scala:647)
at org.apache.spark.util.SparkErrorUtils.tryWithSafeFinally(SparkErrorUtils.scala:80)
at org.apache.spark.util.SparkErrorUtils.tryWithSafeFinally$(SparkErrorUtils.scala:77)
at org.apache.spark.util.Utils$.tryWithSafeFinally(Utils.scala:99)
at org.apache.spark.executor.Executor$TaskRunner.run(Executor.scala:650)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
at java.base/java.lang.Thread.run(Thread.java:1583)
Driver stacktrace:
at app//org.apache.spark.scheduler.DAGScheduler.$anonfun$abortStage$3(DAGScheduler.scala:2935)
at app//scala.Option.getOrElse(Option.scala:201)
at app//org.apache.spark.scheduler.DAGScheduler.$anonfun$abortStage$2(DAGScheduler.scala:2935)
at app//org.apache.spark.scheduler.DAGScheduler.$anonfun$abortStage$2$adapted(DAGScheduler.scala:2927)
at app//scala.collection.immutable.List.foreach(List.scala:334)
at app//org.apache.spark.scheduler.DAGScheduler.abortStage(DAGScheduler.scala:2927)
at app//org.apache.spark.scheduler.DAGScheduler.$anonfun$handleTaskSetFailed$1(DAGScheduler.scala:1295)
at app//org.apache.spark.scheduler.DAGScheduler.$anonfun$handleTaskSetFailed$1$adapted(DAGScheduler.scala:1295)
at app//scala.Option.foreach(Option.scala:437)
at app//org.apache.spark.scheduler.DAGScheduler.handleTaskSetFailed(DAGScheduler.scala:1295)
at app//org.apache.spark.scheduler.DAGSchedulerEventProcessLoop.doOnReceive(DAGScheduler.scala:3207)
at app//org.apache.spark.scheduler.DAGSchedulerEventProcessLoop.onReceive(DAGScheduler.scala:3141)
at app//org.apache.spark.scheduler.DAGSchedulerEventProcessLoop.onReceive(DAGScheduler.scala:3130)
at app//org.apache.spark.util.EventLoop$$anon$1.run(EventLoop.scala:50)
Caused by: java.lang.IllegalArgumentException: Wrong class, expected java.lang.CharSequence, but was java.util.UUID, for object: 849d9bcd-ed84-4bf2-8019-35b001ece9a3
at org.apache.iceberg.PartitionData.get(PartitionData.java:126)
at org.apache.iceberg.util.StructProjection.get(StructProjection.java:214)
at org.apache.iceberg.spark.source.StructInternalRow.getUTF8StringInternal(StructInternalRow.java:178)
at org.apache.iceberg.spark.source.StructInternalRow.getUTF8String(StructInternalRow.java:174)
at org.apache.spark.sql.catalyst.expressions.GeneratedClass$GeneratedIteratorForCodegenStage1.project_writeFields_1_0$(Unknown Source)
at org.apache.spark.sql.catalyst.expressions.GeneratedClass$GeneratedIteratorForCodegenStage1.processNext(Unknown Source)
at org.apache.spark.sql.execution.BufferedRowIterator.hasNext(BufferedRowIterator.java:43)
at org.apache.spark.sql.execution.WholeStageCodegenEvaluatorFactory$WholeStageCodegenPartitionEvaluator$$anon$1.hasNext(WholeStageCodegenEvaluatorFactory.scala:50)
at scala.collection.Iterator$$anon$9.hasNext(Iterator.scala:583)
at org.apache.spark.shuffle.sort.BypassMergeSortShuffleWriter.write(BypassMergeSortShuffleWriter.java:143)
at org.apache.spark.shuffle.ShuffleWriteProcessor.write(ShuffleWriteProcessor.scala:57)
at org.apache.spark.scheduler.ShuffleMapTask.runTask(ShuffleMapTask.scala:111)
at org.apache.spark.scheduler.ShuffleMapTask.runTask(ShuffleMapTask.scala:54)
at org.apache.spark.TaskContext.runTaskWithListeners(TaskContext.scala:171)
at org.apache.spark.scheduler.Task.run(Task.scala:147)
at org.apache.spark.executor.Executor$TaskRunner.$anonfun$run$5(Executor.scala:647)
at org.apache.spark.util.SparkErrorUtils.tryWithSafeFinally(SparkErrorUtils.scala:80)
at org.apache.spark.util.SparkErrorUtils.tryWithSafeFinally$(SparkErrorUtils.scala:77)
at org.apache.spark.util.Utils$.tryWithSafeFinally(Utils.scala:99)
at org.apache.spark.executor.Executor$TaskRunner.run(Executor.scala:650)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
at java.base/java.lang.Thread.run(Thread.java:1583)
```
After the fix all tests pass including these new tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Properly handle UUID in partition field [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#issuecomment-3524470892
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Properly handle UUID in partition field [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#issuecomment-3383626516
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Properly handle UUID in partition field [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#issuecomment-3149887595
@AndrewMakin Thanks for your contribution. Please target your PR against Spark 4.0 first for easier review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Properly handle UUID in partition field [iceberg]
Posted by "AndrewMakin (via GitHub)" <gi...@apache.org>.
AndrewMakin commented on PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#issuecomment-3162001147
FYI I won't be able to add tests until this weekend unfortunately. Sorry to drop off on this issue a bit, this week has turned out to be busy. Will pick it up this weekend though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Properly handle UUID in partition field [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#discussion_r2251118805
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/source/StructInternalRow.java:
##########
@@ -304,8 +312,18 @@ private ArrayData collectionToArrayData(Type elementType, Collection<?> values)
return fillArray(
values,
array ->
- (BiConsumer<Integer, CharSequence>)
- (pos, seq) -> array[pos] = UTF8String.fromString(seq.toString()));
+ (BiConsumer<Integer, Object>)
Review Comment:
I don't think this is currently being tested in the test you added. Can you please add a separate test to make sure this is covered?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Properly handle UUID in partition field [iceberg]
Posted by "AndrewMakin (via GitHub)" <gi...@apache.org>.
AndrewMakin commented on code in PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#discussion_r2251218618
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/source/StructInternalRow.java:
##########
@@ -304,8 +312,18 @@ private ArrayData collectionToArrayData(Type elementType, Collection<?> values)
return fillArray(
values,
array ->
- (BiConsumer<Integer, CharSequence>)
- (pos, seq) -> array[pos] = UTF8String.fromString(seq.toString()));
+ (BiConsumer<Integer, Object>)
Review Comment:
Added the new test to exercise UUID array conversion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Properly handle UUID in partition field [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#issuecomment-3263312009
@AndrewMakin are you still working on this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Properly handle UUID in partition field [iceberg]
Posted by "AndrewMakin (via GitHub)" <gi...@apache.org>.
AndrewMakin commented on PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#issuecomment-3149564026
Hi, first time contributing to Apache Iceberg, and I don't have much exposure to open source contributing in general. Please let me know what I can do better in the future to make the process easier on reviewers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Properly handle UUID in partition field [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#discussion_r2251117857
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/source/StructInternalRow.java:
##########
@@ -175,8 +175,16 @@ public UTF8String getUTF8String(int ordinal) {
}
private UTF8String getUTF8StringInternal(int ordinal) {
- CharSequence seq = struct.get(ordinal, CharSequence.class);
- return UTF8String.fromString(seq.toString());
+ Object value = struct.get(ordinal, Object.class);
Review Comment:
this makes sense to me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Properly handle UUID in partition field [iceberg]
Posted by "AndrewMakin (via GitHub)" <gi...@apache.org>.
AndrewMakin commented on PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#issuecomment-3149974289
Hi, thanks for the fast feedback @manuzhang. Just updated with only the 4.0 changes. This also resolved the style checker complaints.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Properly handle UUID in partition field [iceberg]
Posted by "AndrewMakin (via GitHub)" <gi...@apache.org>.
AndrewMakin commented on PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#issuecomment-3150076364
> Please also start your PR title and commit message with Spark 4.0:
Done. This commit message update canceled the running workflows.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Properly handle UUID in partition field [iceberg]
Posted by "aperture147 (via GitHub)" <gi...@apache.org>.
aperture147 commented on PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#issuecomment-3396634258
hi, is there any updates?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Properly handle UUID in partition field [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#issuecomment-3150063012
Please also start your PR title and commit message with `Spark 4.0:`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Properly handle UUID in partition field [iceberg]
Posted by "AndrewMakin (via GitHub)" <gi...@apache.org>.
AndrewMakin commented on PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#issuecomment-3150287555
Added new tests. Tests also exposed incorrect behavior and I added handling for UUID case specifically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Properly handle UUID in partition field [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#discussion_r2251453160
##########
spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/source/TestUUIDArrayStructInternalRow.java:
##########
@@ -0,0 +1,169 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.spark.source;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.List;
+import java.util.UUID;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.TableProperties;
+import org.apache.iceberg.data.GenericRecord;
+import org.apache.iceberg.data.Record;
+import org.apache.iceberg.hadoop.HadoopTables;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.spark.TestBase;
+import org.apache.iceberg.types.Types;
+import org.apache.spark.sql.Dataset;
+import org.apache.spark.sql.Row;
+import org.apache.spark.sql.catalyst.util.ArrayData;
+import org.apache.spark.unsafe.types.UTF8String;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+public class TestUUIDArrayStructInternalRow extends TestBase {
+
+ private static final HadoopTables TABLES = new HadoopTables(new Configuration());
+ private static final Schema SCHEMA =
+ new Schema(
+ Types.NestedField.optional(1, "id", Types.IntegerType.get()),
+ Types.NestedField.optional(
+ 2, "uuid_array", Types.ListType.ofOptional(3, Types.UUIDType.get())),
Review Comment:
probably also good to check how it behaves with a Map (either as the key or as the value of a map)
##########
spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/actions/TestUUIDPartitionRewriteManifests.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.spark.actions;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.iceberg.ManifestFile;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.TableProperties;
+import org.apache.iceberg.actions.RewriteManifests;
+import org.apache.iceberg.hadoop.HadoopTables;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.spark.TestBase;
+import org.apache.iceberg.types.Types;
+import org.apache.spark.sql.Dataset;
+import org.apache.spark.sql.Row;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+public class TestUUIDPartitionRewriteManifests extends TestBase {
+
+ private static final HadoopTables TABLES = new HadoopTables(new Configuration());
+ private static final Schema SCHEMA =
+ new Schema(
+ Types.NestedField.optional(1, "id", Types.IntegerType.get()),
+ Types.NestedField.optional(2, "uuid_col", Types.UUIDType.get()),
+ Types.NestedField.optional(3, "data", Types.StringType.get()));
+
+ private String tableLocation = null;
+
+ @TempDir private Path temp;
Review Comment:
this isn't used
##########
spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/actions/TestUUIDPartitionRewriteManifests.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.spark.actions;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.iceberg.ManifestFile;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.TableProperties;
+import org.apache.iceberg.actions.RewriteManifests;
+import org.apache.iceberg.hadoop.HadoopTables;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.spark.TestBase;
+import org.apache.iceberg.types.Types;
+import org.apache.spark.sql.Dataset;
+import org.apache.spark.sql.Row;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+public class TestUUIDPartitionRewriteManifests extends TestBase {
+
+ private static final HadoopTables TABLES = new HadoopTables(new Configuration());
+ private static final Schema SCHEMA =
+ new Schema(
+ Types.NestedField.optional(1, "id", Types.IntegerType.get()),
+ Types.NestedField.optional(2, "uuid_col", Types.UUIDType.get()),
+ Types.NestedField.optional(3, "data", Types.StringType.get()));
+
+ private String tableLocation = null;
+
+ @TempDir private Path temp;
+ @TempDir private File tableDir;
+
+ @BeforeEach
+ public void setupTableLocation() throws Exception {
+ this.tableLocation = tableDir.toURI().toString();
+ }
+
+ @Test
+ public void testRewriteManifestsWithUUIDPartition() {
+ PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).identity("uuid_col").build();
+ Map<String, String> options = Maps.newHashMap();
+ options.put(TableProperties.FORMAT_VERSION, "2");
+ Table table = TABLES.create(SCHEMA, spec, options, tableLocation);
Review Comment:
```suggestion
Table table = TABLES.create(SCHEMA, spec, tableLocation);
```
##########
spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/source/TestUUIDArrayStructInternalRow.java:
##########
@@ -0,0 +1,169 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.spark.source;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.List;
+import java.util.UUID;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.TableProperties;
+import org.apache.iceberg.data.GenericRecord;
+import org.apache.iceberg.data.Record;
+import org.apache.iceberg.hadoop.HadoopTables;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.spark.TestBase;
+import org.apache.iceberg.types.Types;
+import org.apache.spark.sql.Dataset;
+import org.apache.spark.sql.Row;
+import org.apache.spark.sql.catalyst.util.ArrayData;
+import org.apache.spark.unsafe.types.UTF8String;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+public class TestUUIDArrayStructInternalRow extends TestBase {
Review Comment:
no need to create a separate test class because that just adds additional overhead. Please move everything into a single test class
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/SparkValueConverter.java:
##########
@@ -78,6 +78,8 @@ public static Object convert(Type type, Object object) {
return ByteBuffer.wrap((byte[]) object);
case INTEGER:
return ((Number) object).intValue();
+ case UUID:
Review Comment:
this also needs some tests in `TestSparkValueConverter`
##########
spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/actions/TestUUIDPartitionRewriteManifests.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.spark.actions;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.iceberg.ManifestFile;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.TableProperties;
+import org.apache.iceberg.actions.RewriteManifests;
+import org.apache.iceberg.hadoop.HadoopTables;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.spark.TestBase;
+import org.apache.iceberg.types.Types;
+import org.apache.spark.sql.Dataset;
+import org.apache.spark.sql.Row;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+public class TestUUIDPartitionRewriteManifests extends TestBase {
+
+ private static final HadoopTables TABLES = new HadoopTables(new Configuration());
+ private static final Schema SCHEMA =
+ new Schema(
+ Types.NestedField.optional(1, "id", Types.IntegerType.get()),
+ Types.NestedField.optional(2, "uuid_col", Types.UUIDType.get()),
+ Types.NestedField.optional(3, "data", Types.StringType.get()));
+
+ private String tableLocation = null;
+
+ @TempDir private Path temp;
+ @TempDir private File tableDir;
+
+ @BeforeEach
+ public void setupTableLocation() throws Exception {
+ this.tableLocation = tableDir.toURI().toString();
+ }
+
+ @Test
+ public void testRewriteManifestsWithUUIDPartition() {
+ PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).identity("uuid_col").build();
+ Map<String, String> options = Maps.newHashMap();
Review Comment:
you can omiti the `options`, since the default format-version will be 2 anyway
##########
spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/actions/TestUUIDPartitionRewriteManifests.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.spark.actions;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.iceberg.ManifestFile;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.TableProperties;
+import org.apache.iceberg.actions.RewriteManifests;
+import org.apache.iceberg.hadoop.HadoopTables;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.spark.TestBase;
+import org.apache.iceberg.types.Types;
+import org.apache.spark.sql.Dataset;
+import org.apache.spark.sql.Row;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+public class TestUUIDPartitionRewriteManifests extends TestBase {
+
+ private static final HadoopTables TABLES = new HadoopTables(new Configuration());
+ private static final Schema SCHEMA =
+ new Schema(
+ Types.NestedField.optional(1, "id", Types.IntegerType.get()),
+ Types.NestedField.optional(2, "uuid_col", Types.UUIDType.get()),
+ Types.NestedField.optional(3, "data", Types.StringType.get()));
+
+ private String tableLocation = null;
+
+ @TempDir private Path temp;
+ @TempDir private File tableDir;
+
+ @BeforeEach
+ public void setupTableLocation() throws Exception {
+ this.tableLocation = tableDir.toURI().toString();
+ }
+
+ @Test
+ public void testRewriteManifestsWithUUIDPartition() {
+ PartitionSpec spec = PartitionSpec.builderFor(SCHEMA).identity("uuid_col").build();
+ Map<String, String> options = Maps.newHashMap();
+ options.put(TableProperties.FORMAT_VERSION, "2");
+ Table table = TABLES.create(SCHEMA, spec, options, tableLocation);
+
+ // Create test data with UUID partition values
+ UUID uuid1 = UUID.randomUUID();
+ UUID uuid2 = UUID.randomUUID();
+
+ // Write first batch of records
+ Dataset<Row> df1 =
+ spark.sql(
+ String.format(
+ "SELECT 1 as id, cast('%s' as string) as uuid_col, 'data1' as data",
+ uuid1.toString()));
+ df1.write().format("iceberg").mode("append").save(tableLocation);
+
+ // Write second batch of records
+ Dataset<Row> df2 =
+ spark.sql(
+ String.format(
+ "SELECT 2 as id, cast('%s' as string) as uuid_col, 'data2' as data",
+ uuid2.toString()));
+ df2.write().format("iceberg").mode("append").save(tableLocation);
+
+ table.refresh();
+
+ List<ManifestFile> manifests = table.currentSnapshot().allManifests(table.io());
+ assertThat(manifests).as("Should have 2 manifests before rewrite").hasSize(2);
+
+ SparkActions actions = SparkActions.get();
+
+ // This should not throw an exception with our fix
+ RewriteManifests.Result result =
+ actions.rewriteManifests(table).rewriteIf(manifest -> true).execute();
Review Comment:
```suggestion
SparkActions.get().rewriteManifests(table).execute();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Properly handle UUID in partition field [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13726:
URL: https://github.com/apache/iceberg/pull/13726#issuecomment-3263305151
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org