alamb commented on code in PR #5455:
URL: https://github.com/apache/arrow-datafusion/pull/5455#discussion_r1123625469
##########
datafusion/core/src/physical_plan/file_format/parquet.rs:
##########
@@ -803,6 +802,7 @@ mod tests {
use crate::datasource::file_format::test_util::scan_format;
use crate::datasource::listing::{FileRange, PartitionedFile};
use crate::datasource::object_store::ObjectStoreUrl;
+ use crate::execution::context::SessionState;
Review Comment:
This is for tests, which I think is ok to depend on datafusion core
##########
datafusion/core/src/physical_plan/file_format/csv.rs:
##########
@@ -280,7 +280,7 @@ impl FileOpener for CsvOpener {
}
pub async fn plan_to_csv(
- state: &SessionState,
+ task_ctx: Arc<TaskContext>,
Review Comment:
The point of the pR is to remove the use of SessionState and hoist the
creation of `TaskContext` into `SessionContext`
##########
datafusion/core/src/physical_plan/file_format/csv.rs:
##########
@@ -300,8 +300,7 @@ pub async fn plan_to_csv(
let path = fs_path.join(filename);
let file = fs::File::create(path)?;
let mut writer = csv::Writer::new(file);
- let task_ctx = Arc::new(TaskContext::from(state));
- let stream = plan.execute(i, task_ctx)?;
+ let stream = plan.execute(i, task_ctx.clone())?;
Review Comment:
Note that `TaskContext` is simply a clone of the state on `SessionState`:
https://github.com/apache/arrow-datafusion/blob/a95e0ec2fd929aae1c2f67148243eb4825d81a3b/datafusion/core/src/execution/context.rs#L2157-L2173
so making it once and cloning is probably better than making multiple
`TaskContext`s (each that have a bunch of `Arcs`)
--
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]
