aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850894951
##########
core/src/main/java/org/apache/iceberg/BaseTableScan.java:
##########
@@ -83,22 +87,23 @@ protected Collection<String> selectedColumns() {
return context.selectedColumns();
}
+ protected ExecutorService planExecutor() {
+ return context.planExecutor();
+ }
+
protected Map<String, String> options() {
return context.options();
}
- protected TableScanContext context() {
+ protected TableScanContext context() {
return context;
}
@SuppressWarnings("checkstyle:HiddenField")
- protected abstract TableScan newRefinedScan(
- TableOperations ops, Table table, Schema schema, TableScanContext
context);
+ protected abstract TableScan newRefinedScan(TableOperations ops, Table
table, Schema schema,
+ TableScanContext context);
- @SuppressWarnings("checkstyle:HiddenField")
- protected abstract CloseableIterable<FileScanTask> planFiles(
Review Comment:
I am not sure the signature of this method makes sense after we introduced
the context object. This method passes some arguments but not all so the
implementations are not entirely pure. Also, the passed arguments are useless
for some metadata tables. That's why I see little value in having so many
arguments. Just makes calling this method harder.
--
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]
