alamb commented on code in PR #19142:
URL: https://github.com/apache/datafusion/pull/19142#discussion_r2600029343
##########
datafusion/core/src/physical_planner.rs:
##########
@@ -1829,6 +1892,105 @@ fn get_physical_expr_pair(
Ok((physical_expr, physical_name))
}
+/// Extract filter predicates from a DML input plan (DELETE/UPDATE).
Review Comment:
I think we need some tests for the planning of delete and update --
specifically that the columns extracted are correct
Perhaps somewhere in
https://github.com/apache/datafusion/blob/2db3aeaa53c2fe7cbc709ad517fd6255d5f02074/datafusion/core/tests/custom_sources_cases/mod.rs#L215-L214
The custom provider doesn't actually need to delete anything, just print out
the columns that it was passed, for example
##########
datafusion/core/src/physical_planner.rs:
##########
@@ -590,6 +593,66 @@ impl DefaultPhysicalPlanner {
);
}
}
+ LogicalPlan::Dml(DmlStatement {
+ table_name,
+ target,
+ op: WriteOp::Delete,
+ input,
+ ..
+ }) => {
+ if let Some(provider) =
+ target.as_any().downcast_ref::<DefaultTableSource>()
+ {
+ let capabilities =
provider.table_provider.dml_capabilities();
+ if !capabilities.delete {
+ return plan_err!(
+ "Table '{}' does not support DELETE operations",
+ table_name
+ );
+ }
+ let filters = extract_dml_filters(input)?;
+ provider
+ .table_provider
+ .delete_from(session_state, filters)
+ .await?
+ } else {
+ return exec_err!(
+ "Table source can't be downcasted to
DefaultTableSource"
+ );
+ }
+ }
Review Comment:
I think a better (smaller API surface) would be to simply try and call
`provider.delete_from(session_state, filters)` and pass along the context
Then you could remove `DmlCapabilities` entirely
Something like:
```rust
LogicalPlan::Dml(DmlStatement {
table_name,
target,
op: WriteOp::Delete,
input,
..
}) => {
if let Some(provider) =
target.as_any().downcast_ref::<DefaultTableSource>()
{
let filters = extract_dml_filters(input)?;
provider
.table_provider
.delete_from(session_state, filters)
.await
.context(format!(
"DELETE operation on table '{}'",
table_name
))?
}
}
```
##########
datafusion/expr/src/logical_plan/dml.rs:
##########
@@ -288,10 +291,103 @@ impl Display for InsertOp {
}
}
+/// DML operations supported by a table.
+#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash)]
+pub struct DmlCapabilities {
Review Comment:
I am not sure this API is necessary (see above for alternate)
--
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]
