anuragmantri commented on code in PR #17868:
URL: https://github.com/apache/iceberg/pull/17868#discussion_r3884374887
##########
docs/docs/spark-procedures.md:
##########
@@ -415,6 +415,7 @@ Iceberg can compact data files in parallel using Spark with
the `rewriteDataFile
| `output-spec-id` | current partition spec id | Identifier of the output
partition spec. Data will be reorganized during the rewrite to align with the
output partitioning. |
| `remove-dangling-deletes` | false | Remove dangling position and equality
deletes after rewriting. A delete file is considered dangling if it does not
apply to any live data files. Enabling this will generate an additional commit
for the removal. |
| `max-files-to-rewrite` | null | This option sets an upper limit on the
number of eligible files that will be rewritten. If this option is not
specified, all eligible files will be rewritten. |
+| `executor-cache.delete-files.enabled` | false | Use the executor cache for
delete files while rewriting. Enable this when the same delete file applies to
many data files, which is common with equality deletes |
Review Comment:
I kept it to `cache-delete-files`. This is concise and conveys the
intention.
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/RewriteDataFilesSparkAction.java:
##########
@@ -70,6 +70,23 @@ public class RewriteDataFilesSparkAction
extends BaseSnapshotUpdateSparkAction<RewriteDataFilesSparkAction>
implements RewriteDataFiles {
private static final Logger LOG =
LoggerFactory.getLogger(RewriteDataFilesSparkAction.class);
+
+ /**
+ * Use the executor cache for delete files while rewriting.
+ *
+ * <p>Enable this when the same delete file applies to many data files,
which is common with
+ * equality deletes.
+ *
+ * <p>This option sets {@link
SparkSQLProperties#EXECUTOR_CACHE_DELETE_FILES_ENABLED} for the
+ * rewrite, so any value configured for that property in the session is
ignored.
+ *
+ * <p>Defaults to false.
+ */
+ public static final String EXECUTOR_CACHE_DELETE_FILES_ENABLED =
Review Comment:
Done.
##########
spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java:
##########
@@ -2246,14 +2246,29 @@ public void testRewriteDataFilesPreservesLineage()
throws NoSuchTableException {
public void testExecutorCacheForDeleteFilesDisabled() {
Table table = createTablePartitioned(1, 1);
RewriteDataFilesSparkAction action =
SparkActions.get(spark).rewriteDataFiles(table);
+ action.execute();
Review Comment:
Okay, I looked at all the other options test, all test behavior. This one is
different. IMO, we have covered the behavior tests in `TestSparkExecutorCache`
so we probably don't need these tests here. I removed them. Let me know if you
feel otherwise.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
