Spark 3.3: Remove use of deprecated SparkFilesScan #7106
Conversation
| @@ -54,7 +54,7 @@ class SparkFilesScan extends SparkScan { | |||
| @Override | |||
| protected List<CombinedScanTask> taskGroups() { | |||
| if (tasks == null) { | |||
| FileScanTaskSetManager taskSetManager = FileScanTaskSetManager.get(); | |||
There was a problem hiding this comment.
Can we remove this class and SparkFilesScanBuilder completely?
There was a problem hiding this comment.
Shall we also drop FileScanTaskSetManager?
There was a problem hiding this comment.
Oh, wait, 1.2 is not out yet.
There was a problem hiding this comment.
If we plan to merge before 1.2, it is not safe to change SparkFilesScan. We can migrate the compaction code but the old integration has to be functional.
There was a problem hiding this comment.
I'm ok to wait until 1.2 is released
There was a problem hiding this comment.
I agree that this PR should only be merged once 1.2.0 is officially out. Also would be great to delete FileScanTaskSetManager as part of this PR
There was a problem hiding this comment.
Removed in latest commit
| @@ -260,11 +260,6 @@ public MetadataColumn[] metadataColumns() { | |||
|
|
|||
| @Override | |||
| public ScanBuilder newScanBuilder(CaseInsensitiveStringMap options) { | |||
| if (options.containsKey(SparkReadOptions.FILE_SCAN_TASK_SET_ID)) { | |||
There was a problem hiding this comment.
We should either keep this or wait until 1.2 is out and then remove all classes and deprecated vars.
There was a problem hiding this comment.
Removed the class, let's wait after 1.2 release to merge
| @@ -54,7 +54,7 @@ class SparkFilesScan extends SparkScan { | |||
| @Override | |||
| protected List<CombinedScanTask> taskGroups() { | |||
| if (tasks == null) { | |||
| FileScanTaskSetManager taskSetManager = FileScanTaskSetManager.get(); | |||
There was a problem hiding this comment.
I agree that this PR should only be merged once 1.2.0 is officially out. Also would be great to delete FileScanTaskSetManager as part of this PR
| @@ -97,7 +97,7 @@ public void testBinPackRewrite() throws NoSuchTableException, IOException { | |||
| FileRewriteCoordinator rewriteCoordinator = FileRewriteCoordinator.get(); | |||
| Set<DataFile> rewrittenFiles = | |||
| taskSetManager.fetchTasks(table, fileSetID).stream() | |||
| .map(FileScanTask::file) | |||
| .map(t -> ((FileScanTask) t).file()) | |||
There was a problem hiding this comment.
| .map(t -> ((FileScanTask) t).file()) | |
| .map(t -> t.asFileScanTask().file()) |
There was a problem hiding this comment.
Good idea, thanks
| @@ -167,7 +167,7 @@ public void testSortRewrite() throws NoSuchTableException, IOException { | |||
| FileRewriteCoordinator rewriteCoordinator = FileRewriteCoordinator.get(); | |||
| Set<DataFile> rewrittenFiles = | |||
| taskSetManager.fetchTasks(table, fileSetID).stream() | |||
| .map(FileScanTask::file) | |||
| .map(t -> ((FileScanTask) t).file()) | |||
There was a problem hiding this comment.
| .map(t -> ((FileScanTask) t).file()) | |
| .map(t -> t.asFileScanTask().file()) |
| @@ -243,7 +243,7 @@ public void testCommitMultipleRewrites() throws NoSuchTableException, IOExceptio | |||
| Set<DataFile> rewrittenFiles = | |||
| fileSetIDs.stream() | |||
| .flatMap(fileSetID -> taskSetManager.fetchTasks(table, fileSetID).stream()) | |||
| .map(FileScanTask::file) | |||
| .map(t -> ((FileScanTask) t).file()) | |||
szehon-ho
force-pushed
the
remove_file_scan
branch
from
a916484
to
939d6c6
Compare
|
Let me take another look. |
|
Thanks, @szehon-ho! Thanks for reviewing, @nastra! |
Follow up of : #6924