YinZheng-Sun commented on code in PR #956:
URL: https://github.com/apache/iceberg-cpp/pull/956#discussion_r4236955015
##########
src/iceberg/data/file_scan_task_reader.cc:
##########
@@ -164,10 +165,21 @@ class FileScanTaskReader::Impl {
"Data file size must not be negative: {}",
data_file->file_size_in_bytes);
+ auto filter = task.residual_filter();
+ if (filter) {
+ ICEBERG_ASSIGN_OR_RAISE(auto is_bound, IsBoundVisitor::IsBound(filter));
+ if (!is_bound) {
+ ICEBERG_ASSIGN_OR_RAISE(
+ filter,
+ Binder::Bind(*table_schema_, filter,
Review Comment:
I notice that TableScan::ResolveProjectedSchema() already combines the
selected field IDs with the filter’s referenced field IDs when constructing the
scan projection. If FileScanTaskReader receives that schema, the filter columns
are already present, so performing the same merge again seems redundant.
The cases that may lack filter columns are an explicitly supplied projection
or direct reader usage that bypasses scan planning.
Should we require callers on these paths to provide a complete read schema,
or should FileScanTaskReader also handle this? If this is a caller
responsibility, we could simply bind against the supplied projectiE schema
without another merge.
--
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]
