tom-at-rewbi commented on code in PR #2676:
URL: https://github.com/apache/iceberg-python/pull/2676#discussion_r2487391312


##########
pyiceberg/io/pyarrow.py:
##########
@@ -1726,16 +1726,10 @@ def to_record_batches(self, tasks: 
Iterable[FileScanTask]) -> Iterator[pa.Record
         deletes_per_file = _read_all_delete_files(self._io, tasks)
 
         total_row_count = 0
-        executor = ExecutorFactory.get_or_create()
-
-        def batches_for_task(task: FileScanTask) -> List[pa.RecordBatch]:
-            # Materialize the iterator here to ensure execution happens within 
the executor.
-            # Otherwise, the iterator would be lazily consumed later (in the 
main thread),
-            # defeating the purpose of using executor.map.
-            return 
list(self._record_batches_from_scan_tasks_and_deletes([task], deletes_per_file))

Review Comment:
   If I'm not mistaken, reading is still parallel after this change. Every time 
a scan task is processed, `pyarrow.dataset.Scanner.from_fragment` is called 
with the default arguments for `use_threads`, which has a default of maximum 
parallelism. [(link to 
source)](https://github.com/tom-at-rewbi/iceberg-python/blob/c8b76875bad3b81dc8e9b4ee9ec9c459b3babd8a/pyiceberg/io/pyarrow.py#L1557)
   
    Are there any situations where we would expect stacking these two forms of 
parallelism to be beneficial?



-- 
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]

Reply via email to