You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "Nachiket-Roy (via GitHub)" <gi...@apache.org> on 2026/01/04 06:49:27 UTC
[PR] added truncate table support [datafusion]
Nachiket-Roy opened a new pull request, #19633:
URL: https://github.com/apache/datafusion/pull/19633
## Which issue does this PR close?
- Related to #19617
## Rationale for this change
DataFusion recently added TableProvider hooks for row-level DML operations such as DELETE and UPDATE, but TRUNCATE TABLE was still unsupported.
## What changes are included in this PR?
This PR adds end-to-end support for TRUNCATE TABLE, including:
- SQL parsing and logical plan support for TRUNCATE TABLE
- A new WriteOp::Truncate DML operation
- Physical planner routing for TRUNCATE operations
- A new TableProvider::truncate() hook for storage-native implementations
- A reference implementation of TRUNCATE for MemTable
- Protobuf / DML node support for serializing and deserializing TRUNCATE operations
- SQL logic tests covering both planning (truncate.slt) and execution behavior (dml_truncate.slt)
The implementation follows the same structure and conventions as existing DELETE and UPDATE support.
## Are these changes tested?
Yes. The PR includes:
SQL logic tests verifying logical and physical planning of TRUNCATE TABLE
Execution tests validating MemTable behavior, including:
- Removing all rows
- Preserving table schema
- Repeated TRUNCATE operations
- Inserting data after TRUNCATE
All tests pass locally.
## Are there any user-facing changes?
Yes. Users can now execute TRUNCATE TABLE statements in DataFusion for tables whose TableProvider supports the new truncate() hook. Tables that do not support TRUNCATE will return a clear NotImplemented error.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2700593731
##########
datafusion/sql/src/statement.rs:
##########
@@ -1362,6 +1362,53 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ ..
+ } => {
+ if table_names.len() != 1 {
+ return not_impl_err!(
+ "TRUNCATE with multiple tables is not supported"
+ );
+ }
+
+ let target = &table_names[0];
+ if target.only {
+ return not_impl_err!("TRUNCATE with ONLY is not supported");
+ }
+ if partitions.is_some() {
+ return not_impl_err!("TRUNCATE with PARTITION is not supported");
+ }
+ if identity.is_some() {
+ return not_impl_err!(
+ "TRUNCATE with RESTART/CONTINUE IDENTITY is not supported"
+ );
+ }
+ if cascade.is_some() {
+ return not_impl_err!(
+ "TRUNCATE with CASCADE/RESTRICT is not supported"
+ );
+ }
+ if on_cluster.is_some() {
+ return not_impl_err!("TRUNCATE with ON CLUSTER is not supported");
+ }
+ let table = self.object_name_to_table_reference(target.name.clone())?;
+ let source = self.context_provider.get_table_source(table.clone())?;
+
+ Ok(LogicalPlan::Dml(DmlStatement::new(
+ table.clone(),
+ source,
+ WriteOp::Truncate,
+ Arc::new(LogicalPlan::EmptyRelation(EmptyRelation {
+ produce_one_row: false,
+ schema: DFSchemaRef::new(DFSchema::empty()),
Review Comment:
It would be good to add the clarifying comment here on why an empty relation is fine here
##########
datafusion/catalog/src/table.rs:
##########
@@ -353,6 +353,14 @@ pub trait TableProvider: Debug + Sync + Send {
) -> Result<Arc<dyn ExecutionPlan>> {
not_impl_err!("UPDATE not supported for {} table", self.table_type())
}
+
+ /// Remove all rows from the table.
+ ///
+ /// Returns an [`ExecutionPlan`] producing a single row with `count` (UInt64),
+ /// representing the number of rows removed.
Review Comment:
Do we have a way to validate this contract is upheld?
##########
datafusion/sql/src/statement.rs:
##########
@@ -1362,6 +1362,53 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ ..
Review Comment:
Would prefer to destructure this `..` so we'll always be notified if new syntax comes in from sqlparser
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2701161206
##########
datafusion/sql/src/statement.rs:
##########
@@ -1390,6 +1390,56 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate(ast::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ table,
+ }) => {
+ let _ = table; // explicitly handled to satisfy full destructuring
Review Comment:
This comment isn't informative; we should be explaining in the context of the SQL syntax, not in terms of code readability. Why is is alright for us to ignore this parameter in terms of the SQL syntax we accept?
##########
datafusion/sql/src/statement.rs:
##########
@@ -1390,6 +1390,56 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate(ast::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ table,
+ }) => {
+ let _ = table; // explicitly handled to satisfy full destructuring
Review Comment:
This comment isn't informative; we should be explaining in the context of the SQL syntax, not in terms of code structure. Why is is alright for us to ignore this parameter in terms of the SQL syntax we accept?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2701227088
##########
datafusion/sql/src/statement.rs:
##########
@@ -1390,6 +1390,56 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate(ast::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ table,
+ }) => {
+ let _ = table; // explicitly handled to satisfy full destructuring
Review Comment:
would you like me to add this comment here?
> sqlparser supports additional dialect features, but DataFusion does not utilize all of them. In this case, cluster-aware TRUNCATE behavior is not implemented, so the field is ignored (and rejected if users try to use it).
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2700760920
##########
datafusion/sql/src/statement.rs:
##########
@@ -1362,6 +1362,53 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ ..
Review Comment:
Sure -- on it
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2702169355
##########
datafusion/sql/src/statement.rs:
##########
@@ -1390,6 +1390,56 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate(ast::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ table,
+ }) => {
+ let _ = table; // explicitly handled to satisfy full destructuring
Review Comment:
> table: bool
> TABLE - optional keyword
- https://docs.rs/sqlparser/latest/sqlparser/ast/struct.Truncate.html
I'm not sure what you mean, this is what the sqlparser docs describe what the `table` field represents
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2702177557
##########
datafusion/sql/src/statement.rs:
##########
@@ -1390,6 +1390,56 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate(ast::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ table,
+ }) => {
+ let _ = table; // explicitly handled to satisfy full destructuring
Review Comment:
ok i will add a test for TRUNCATE without TABLE keyword
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2702175345
##########
datafusion/sql/src/statement.rs:
##########
@@ -1390,6 +1390,56 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate(ast::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ table,
+ }) => {
+ let _ = table; // explicitly handled to satisfy full destructuring
Review Comment:
A simple comment like
> Support `TRUNCATR TABLE` and `TRUNCATE` syntax
Would be sufficient. As well as adding a test for the syntax omitting the table keyword.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2662181047
##########
datafusion/sql/src/statement.rs:
##########
@@ -1362,6 +1362,53 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ ..
+ } => {
+ if table_names.len() != 1 {
+ return not_impl_err!(
+ "TRUNCATE with multiple tables is not supported"
+ );
+ }
+
+ let target = &table_names[0];
+ if target.only {
+ return not_impl_err!("TRUNCATE with ONLY is not supported");
+ }
+ if partitions.is_some() {
+ return not_impl_err!("TRUNCATE with PARTITION is not supported");
+ }
+ if identity.is_some() {
+ return not_impl_err!(
+ "TRUNCATE with RESTART/CONTINUE IDENTITY is not supported"
+ );
+ }
+ if cascade.is_some() {
+ return not_impl_err!(
+ "TRUNCATE with CASCADE/RESTRICT is not supported"
+ );
+ }
+ if on_cluster.is_some() {
+ return not_impl_err!("TRUNCATE with ON CLUSTER is not supported");
+ }
+ let table = self.object_name_to_table_reference(target.name.clone())?;
+ let source = self.context_provider.get_table_source(table.clone())?;
+
+ Ok(LogicalPlan::Dml(DmlStatement::new(
+ table.clone(),
+ source,
+ WriteOp::Truncate,
+ Arc::new(LogicalPlan::EmptyRelation(EmptyRelation {
+ produce_one_row: false,
+ schema: DFSchemaRef::new(DFSchema::empty()),
Review Comment:
Unlike DELETE or UPDATE, TRUNCATE is a table-level operation with no filters, assignments. The input relation is only a placeholder to satisfy the DML plan shape and is never inspected or executed for row data. Because no rows or columns are read, the usual “input schema must match table schema” invariant does not apply here.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "ethan-tyler (via GitHub)" <gi...@apache.org>.
ethan-tyler commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2660374811
##########
datafusion/sql/src/statement.rs:
##########
@@ -1362,6 +1362,28 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate { table_names, .. } => {
+ if table_names.len() != 1 {
+ return not_impl_err!(
+ "TRUNCATE with multiple tables is not supported"
+ );
+ }
Review Comment:
If you add the option rejection, some negative tests would round it out:
```
statement error TRUNCATE with CASCADE/RESTRICT is not supported
TRUNCATE TABLE t1 CASCADE;
statement error TRUNCATE with multiple tables is not supported
TRUNCATE TABLE t1, t2;
```
##########
datafusion/sql/src/statement.rs:
##########
@@ -1362,6 +1362,28 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate { table_names, .. } => {
+ if table_names.len() != 1 {
+ return not_impl_err!(
+ "TRUNCATE with multiple tables is not supported"
+ );
+ }
+
+ let target = &table_names[0]; // TruncateTableTarget
+ let table = self.object_name_to_table_reference(target.name.clone())?;
+ let source = self.context_provider.get_table_source(table.clone())?;
+
+ Ok(LogicalPlan::Dml(DmlStatement {
+ table_name: table.clone(),
+ target: source,
+ op: WriteOp::Truncate,
+ input: Arc::new(LogicalPlan::EmptyRelation(EmptyRelation {
+ produce_one_row: false,
+ schema: DFSchemaRef::new(DFSchema::empty()),
+ })),
+ output_schema: DFSchemaRef::new(DFSchema::empty()),
+ }))
+ }
Review Comment:
This should use DmlStatement::new() rather than constructing the struct directly. The constructor sets output_schema: make_count_schema() which gives you the {count: UInt64} that DML ops return.
Also noticed this causes a proto roundtrip mismatch: encode uses the struct directly but decode uses the constructor (at mod.rs:978), so you'd get different plans before/after serialization.
Something like:
```suggestion
Ok(LogicalPlan::Dml(DmlStatement::new(
table.clone(),
source,
WriteOp::Truncate,
Arc::new(LogicalPlan::EmptyRelation(EmptyRelation {
produce_one_row: false,
schema: DFSchemaRef::new(DFSchema::empty()),
})),
)))
```
##########
datafusion/sql/src/statement.rs:
##########
@@ -1362,6 +1362,28 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate { table_names, .. } => {
+ if table_names.len() != 1 {
+ return not_impl_err!(
+ "TRUNCATE with multiple tables is not supported"
+ );
+ }
Review Comment:
The .. here silently drops partitions, identity, cascade, on_cluster, and target.only. If someone writes TRUNCATE TABLE t CASCADE expecting cascade behavior, they'd get a normal truncate with no indication the option was ignored.
Worth rejecting these explicitly? Something like:
```suggestion
Statement::Truncate { table_names, partitions, identity, cascade, on_cluster, .. } => {
if table_names.len() != 1 {
return not_impl_err!("TRUNCATE with multiple tables is not supported");
}
let target = &table_names[0];
if target.only {
return not_impl_err!("TRUNCATE with ONLY is not supported");
}
if partitions.is_some() {
return not_impl_err!("TRUNCATE with PARTITION is not supported");
}
if identity.is_some() {
return not_impl_err!("TRUNCATE with RESTART/CONTINUE IDENTITY is not supported");
}
if cascade.is_some() {
return not_impl_err!("TRUNCATE with CASCADE/RESTRICT is not supported");
}
if on_cluster.is_some() {
return not_impl_err!("TRUNCATE with ON CLUSTER is not supported");
}
// ... rest
}
```
##########
datafusion/catalog/src/table.rs:
##########
@@ -353,6 +353,14 @@ pub trait TableProvider: Debug + Sync + Send {
) -> Result<Arc<dyn ExecutionPlan>> {
not_impl_err!("UPDATE not supported for {} table", self.table_type())
}
+
+ /// Remove all rows from the table.
+ ///
+ /// Returns an [`ExecutionPlan`] producing a single row with `count` (UInt64),
+ /// representing the number of rows removed.
+ async fn truncate(&self, _state: &dyn Session) -> Result<Arc<dyn ExecutionPlan>> {
+ not_impl_err!("TRUNCATE not supported for {}", self.table_type())
Review Comment:
nit: DELETE/UPDATE messages say "... for {} table" but this one drops "table"
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#issuecomment-3713244811
@ethan-tyler Thanks again for the review! I’ve tightened the CaptureTruncateProvider test to return a single {count: UInt64} row, added negative tests covering PARTITION, ONLY, and IDENTITY (in addition to CASCADE and multi-table), and added TRUNCATE to roundtrip_logical_plan_dml to lock in the proto roundtrip behavior.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "ethan-tyler (via GitHub)" <gi...@apache.org>.
ethan-tyler commented on PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#issuecomment-3715079014
> @ethan-tyler I tried tightening the test using `TestMemoryExec` to materialize a {count: UInt64} row as suggested. However this causes the test to fail:
>
> ```shell
> TestMemoryExec::with_new_children not implemented
> ```
>
> This happens because physical optimizer rules run and require `ExecutionPlan::with_new_children`, which `TestMemoryExec` does not implement. Could you please look into this?
@Nachiket-Roy My bad - should have validated that suggestion first. TestMemoryExec doesn't implement with_new_children, so it panics during filter pushdown.
Better to stick with EmptyExec:
```
async fn truncate(&self, _state: &dyn Session) -> Result<Arc<dyn ExecutionPlan>> {
*self.truncate_called.lock().unwrap() = true;
Ok(Arc::new(EmptyExec::new(Arc::new(Schema::new(vec![
Field::new("count", DataType::UInt64, false),
])))))
}
```
This matches what CaptureDeleteProvider and CaptureUpdateProvider already do. These tests just verify the hook gets called and validating provider output is the implementor's responsibility.
The "proper" way to return an actual count is DmlResultExec, but it's private to the memory table module. Making it public would be a separate discussion. Sorry for the detour, the original implementation was fine.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#issuecomment-3713520586
how about this? @ethan-tyler I tried tightening the test using TestMemoryExec to materialize a {count: UInt64} row as suggested.
However this causes the test to fail:
```bash
TestMemoryExec::with_new_children not implemented
```
This happens because physical optimizer rules run and require ExecutionPlan::with_new_children, which TestMemoryExec does not implement. Could you please look into this?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2702173733
##########
datafusion/sql/src/statement.rs:
##########
@@ -1390,6 +1390,56 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate(ast::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ table,
+ }) => {
+ let _ = table; // explicitly handled to satisfy full destructuring
Review Comment:
ok so there is no diff in TRUNCATE and TRUNCATE TABLE we can ignore table? is this correct ?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#issuecomment-3776001514
Thanks @Nachiket-Roy, @ethan-tyler & @martin-g
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey merged PR #19633:
URL: https://github.com/apache/datafusion/pull/19633
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#issuecomment-3709413296
@ethan-tyler Thanks for the thorough review. I've addressed all your comments, Please take another look.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "ethan-tyler (via GitHub)" <gi...@apache.org>.
ethan-tyler commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2662814486
##########
datafusion/sql/src/statement.rs:
##########
@@ -1362,6 +1362,53 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ ..
+ } => {
+ if table_names.len() != 1 {
+ return not_impl_err!(
+ "TRUNCATE with multiple tables is not supported"
+ );
+ }
+
+ let target = &table_names[0];
+ if target.only {
+ return not_impl_err!("TRUNCATE with ONLY is not supported");
+ }
+ if partitions.is_some() {
+ return not_impl_err!("TRUNCATE with PARTITION is not supported");
+ }
+ if identity.is_some() {
+ return not_impl_err!(
+ "TRUNCATE with RESTART/CONTINUE IDENTITY is not supported"
+ );
+ }
+ if cascade.is_some() {
+ return not_impl_err!(
+ "TRUNCATE with CASCADE/RESTRICT is not supported"
+ );
+ }
+ if on_cluster.is_some() {
+ return not_impl_err!("TRUNCATE with ON CLUSTER is not supported");
+ }
+ let table = self.object_name_to_table_reference(target.name.clone())?;
+ let source = self.context_provider.get_table_source(table.clone())?;
+
+ Ok(LogicalPlan::Dml(DmlStatement::new(
+ table.clone(),
+ source,
+ WriteOp::Truncate,
+ Arc::new(LogicalPlan::EmptyRelation(EmptyRelation {
+ produce_one_row: false,
+ schema: DFSchemaRef::new(DFSchema::empty()),
Review Comment:
I verified this and you're right that TRUNCATE doesn't use the input.
Physical planner shows:
- DELETE/UPDATE: extract filters/assignments from input
- TRUNCATE: ignores input entirely, just calls provider.truncate()
So functionally the empty schema is fine. The doc comment is aspirational. Could either keep as-is and update the DmlStatement docs to note TRUNCATE is special, or use the table schema for consistency (extra code, no functional benefit). Former seems fine.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "ethan-tyler (via GitHub)" <gi...@apache.org>.
ethan-tyler commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2662853240
##########
datafusion/core/tests/custom_sources_cases/dml_planning.rs:
##########
@@ -165,6 +165,66 @@ impl TableProvider for CaptureUpdateProvider {
}
}
+/// A TableProvider that captures whether truncate() was called.
+struct CaptureTruncateProvider {
+ schema: SchemaRef,
+ truncate_called: Arc<Mutex<bool>>,
+}
+
+impl CaptureTruncateProvider {
+ fn new(schema: SchemaRef) -> Self {
+ Self {
+ schema,
+ truncate_called: Arc::new(Mutex::new(false)),
+ }
+ }
+
+ fn was_truncated(&self) -> bool {
+ *self.truncate_called.lock().unwrap()
+ }
+}
+
+impl std::fmt::Debug for CaptureTruncateProvider {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ f.debug_struct("CaptureTruncateProvider")
+ .field("schema", &self.schema)
+ .finish()
+ }
+}
+
+#[async_trait]
+impl TableProvider for CaptureTruncateProvider {
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn schema(&self) -> SchemaRef {
+ Arc::clone(&self.schema)
+ }
+
+ fn table_type(&self) -> TableType {
+ TableType::Base
+ }
+
+ async fn scan(
+ &self,
+ _state: &dyn Session,
+ _projection: Option<&Vec<usize>>,
+ _filters: &[Expr],
+ _limit: Option<usize>,
+ ) -> Result<Arc<dyn ExecutionPlan>> {
+ Ok(Arc::new(EmptyExec::new(Arc::clone(&self.schema))))
+ }
+
+ async fn truncate(&self, _state: &dyn Session) -> Result<Arc<dyn ExecutionPlan>> {
+ *self.truncate_called.lock().unwrap() = true;
+
+ Ok(Arc::new(EmptyExec::new(Arc::new(Schema::new(vec![
+ Field::new("count", DataType::UInt64, false),
+ ])))))
+ }
+}
Review Comment:
TableProvider::truncate docs say it returns an ExecutionPlan producing {count: UInt64}. This test returns EmptyExec which produces zero rows, it validates the hook was called, but not the contract.
Could tighten this to return an actual row and assert on it:
```suggestion
// Add these imports at the top of the file:
use arrow::array::UInt64Array;
use arrow::record_batch::RecordBatch;
use datafusion_physical_plan::test::TestMemoryExec;
// Then update the truncate impl:
async fn truncate(&self, _state: &dyn Session) -> Result<Arc<dyn ExecutionPlan>> {
*self.truncate_called.lock().unwrap() = true;
let schema = Arc::new(Schema::new(vec![
Field::new("count", DataType::UInt64, false),
]));
let batch = RecordBatch::try_new(
Arc::clone(&schema),
vec![Arc::new(UInt64Array::from(vec![0u64]))],
)?;
Ok(Arc::new(TestMemoryExec::try_new(&[vec![batch]], schema, None)?))
}
```
##########
datafusion/proto/src/logical_plan/to_proto.rs:
##########
@@ -728,6 +728,7 @@ impl From<&WriteOp> for protobuf::dml_node::Type {
WriteOp::Delete => protobuf::dml_node::Type::Delete,
WriteOp::Update => protobuf::dml_node::Type::Update,
WriteOp::Ctas => protobuf::dml_node::Type::Ctas,
+ WriteOp::Truncate => protobuf::dml_node::Type::Truncate,
Review Comment:
You added the proto enum and conversions and worth adding a roundtrip test to lock it in.
In `datafusion/proto/tests/cases/roundtrip_logical_plan.rs`, the `roundtrip_logical_plan_dml` test covers INSERT, DELETE, UPDATE, CTAS.
Adding TRUNCATE there would catch any future regressions:
```
// In the queries array:
"TRUNCATE TABLE test_table",
```
##########
datafusion/sqllogictest/test_files/truncate.slt:
##########
@@ -0,0 +1,64 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+
+# http://www.apache.org/licenses/LICENSE-2.0
+
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+##########
+## Truncate Tests
+##########
+
+statement ok
+create table t1(a int, b varchar, c double, d int);
+
+statement ok
+insert into t1 values (1, 'abc', 3.14, 4), (2, 'def', 2.71, 5);
+
+# Truncate all rows from table
+query TT
+explain truncate table t1;
+----
+logical_plan
+01)Dml: op=[Truncate] table=[t1]
+02)--EmptyRelation: rows=0
+physical_plan_error
+01)TRUNCATE operation on table 't1'
+02)caused by
+03)This feature is not implemented: TRUNCATE not supported for Base table
+
+# Test TRUNCATE with fully qualified table name
+statement ok
+create schema test_schema;
+
+statement ok
+create table test_schema.t5(a int);
+
+query TT
+explain truncate table test_schema.t5;
+----
+logical_plan
+01)Dml: op=[Truncate] table=[test_schema.t5]
+02)--EmptyRelation: rows=0
+physical_plan_error
+01)TRUNCATE operation on table 'test_schema.t5'
+02)caused by
+03)This feature is not implemented: TRUNCATE not supported for Base table
+
+# Test TRUNCATE with CASCADE option
+statement error TRUNCATE with CASCADE/RESTRICT is not supported
+TRUNCATE TABLE t1 CASCADE;
+
+# Test TRUNCATE with multiple tables
+statement error TRUNCATE with multiple tables is not supported
+TRUNCATE TABLE t1, t2;
Review Comment:
You have negative tests for CASCADE and multi-table. The planner also rejects PARTITION, ONLY, IDENTITY, and ON CLUSTER.
Might be worth covering those too:
```
statement error TRUNCATE with PARTITION is not supported
TRUNCATE TABLE t1 PARTITION (p1);
statement error TRUNCATE with ONLY is not supported
TRUNCATE ONLY t1;
statement error TRUNCATE with RESTART/CONTINUE IDENTITY is not supported
TRUNCATE TABLE t1 RESTART IDENTITY;
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2701226611
##########
datafusion/sql/src/statement.rs:
##########
@@ -1390,6 +1390,56 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate(ast::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ table,
+ }) => {
+ let _ = table; // explicitly handled to satisfy full destructuring
Review Comment:
sqlparser supports additional dialect features, but DataFusion does not utilize all of them. In this case, cluster-aware TRUNCATE behavior is not implemented, so the field is ignored (and rejected if users try to use it).
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2702173733
##########
datafusion/sql/src/statement.rs:
##########
@@ -1390,6 +1390,56 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate(ast::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ table,
+ }) => {
+ let _ = table; // explicitly handled to satisfy full destructuring
Review Comment:
ok so there is no diff in TRUNCATE and TRUNCATE TABLE, we can ignore table? is this correct ?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2701218155
##########
datafusion/catalog/src/table.rs:
##########
@@ -353,6 +353,14 @@ pub trait TableProvider: Debug + Sync + Send {
) -> Result<Arc<dyn ExecutionPlan>> {
not_impl_err!("UPDATE not supported for {} table", self.table_type())
}
+
+ /// Remove all rows from the table.
+ ///
+ /// Returns an [`ExecutionPlan`] producing a single row with `count` (UInt64),
+ /// representing the number of rows removed.
Review Comment:
I’m not aware of any explicit validator for this in the current planner. If an implementation returns a schema that doesn’t match what the planning or execution stages expect, it surfaces as a planning or execution error rather than passing silently. This is effectively enforced by the physical planning layer
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2702063336
##########
datafusion/sql/src/statement.rs:
##########
@@ -1390,6 +1390,56 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate(ast::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ table,
+ }) => {
+ let _ = table; // explicitly handled to satisfy full destructuring
Review Comment:
I'm referring to `table` here not `on_cluster`
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2701158565
##########
datafusion/catalog/src/table.rs:
##########
@@ -353,6 +353,14 @@ pub trait TableProvider: Debug + Sync + Send {
) -> Result<Arc<dyn ExecutionPlan>> {
not_impl_err!("UPDATE not supported for {} table", self.table_type())
}
+
+ /// Remove all rows from the table.
+ ///
+ /// Returns an [`ExecutionPlan`] producing a single row with `count` (UInt64),
+ /// representing the number of rows removed.
Review Comment:
I don't mean as a test within datafusion; I mean, what stops an implementer of this function from just returning something other than a UInt64 row? Are there any checks within datafusions planners that it returns the correct type/result, otherwise this seems more like a guideline if we can't enforce it
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2702163497
##########
datafusion/sql/src/statement.rs:
##########
@@ -1390,6 +1390,56 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate(ast::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ table,
+ }) => {
+ let _ = table; // explicitly handled to satisfy full destructuring
Review Comment:
oops sorry about that. The reason table is ignore is because we only supports `TRUNCATE` for a single resolved table name. The table field does not influence planning or execution
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2662134791
##########
datafusion/sql/src/statement.rs:
##########
@@ -1362,6 +1362,53 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ ..
+ } => {
+ if table_names.len() != 1 {
+ return not_impl_err!(
+ "TRUNCATE with multiple tables is not supported"
+ );
+ }
+
+ let target = &table_names[0];
+ if target.only {
+ return not_impl_err!("TRUNCATE with ONLY is not supported");
+ }
+ if partitions.is_some() {
+ return not_impl_err!("TRUNCATE with PARTITION is not supported");
+ }
+ if identity.is_some() {
+ return not_impl_err!(
+ "TRUNCATE with RESTART/CONTINUE IDENTITY is not supported"
+ );
+ }
+ if cascade.is_some() {
+ return not_impl_err!(
+ "TRUNCATE with CASCADE/RESTRICT is not supported"
+ );
+ }
+ if on_cluster.is_some() {
+ return not_impl_err!("TRUNCATE with ON CLUSTER is not supported");
+ }
+ let table = self.object_name_to_table_reference(target.name.clone())?;
+ let source = self.context_provider.get_table_source(table.clone())?;
+
+ Ok(LogicalPlan::Dml(DmlStatement::new(
+ table.clone(),
+ source,
+ WriteOp::Truncate,
+ Arc::new(LogicalPlan::EmptyRelation(EmptyRelation {
+ produce_one_row: false,
+ schema: DFSchemaRef::new(DFSchema::empty()),
Review Comment:
https://github.com/apache/datafusion/blob/47df535d2cd5aac5ad5a92bdc837f38e05ea0f0f/datafusion/expr/src/logical_plan/dml.rs#L147 says `the schema must match with table_schema`. Is this important here ?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "ethan-tyler (via GitHub)" <gi...@apache.org>.
ethan-tyler commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2662853240
##########
datafusion/core/tests/custom_sources_cases/dml_planning.rs:
##########
@@ -165,6 +165,66 @@ impl TableProvider for CaptureUpdateProvider {
}
}
+/// A TableProvider that captures whether truncate() was called.
+struct CaptureTruncateProvider {
+ schema: SchemaRef,
+ truncate_called: Arc<Mutex<bool>>,
+}
+
+impl CaptureTruncateProvider {
+ fn new(schema: SchemaRef) -> Self {
+ Self {
+ schema,
+ truncate_called: Arc::new(Mutex::new(false)),
+ }
+ }
+
+ fn was_truncated(&self) -> bool {
+ *self.truncate_called.lock().unwrap()
+ }
+}
+
+impl std::fmt::Debug for CaptureTruncateProvider {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ f.debug_struct("CaptureTruncateProvider")
+ .field("schema", &self.schema)
+ .finish()
+ }
+}
+
+#[async_trait]
+impl TableProvider for CaptureTruncateProvider {
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn schema(&self) -> SchemaRef {
+ Arc::clone(&self.schema)
+ }
+
+ fn table_type(&self) -> TableType {
+ TableType::Base
+ }
+
+ async fn scan(
+ &self,
+ _state: &dyn Session,
+ _projection: Option<&Vec<usize>>,
+ _filters: &[Expr],
+ _limit: Option<usize>,
+ ) -> Result<Arc<dyn ExecutionPlan>> {
+ Ok(Arc::new(EmptyExec::new(Arc::clone(&self.schema))))
+ }
+
+ async fn truncate(&self, _state: &dyn Session) -> Result<Arc<dyn ExecutionPlan>> {
+ *self.truncate_called.lock().unwrap() = true;
+
+ Ok(Arc::new(EmptyExec::new(Arc::new(Schema::new(vec![
+ Field::new("count", DataType::UInt64, false),
+ ])))))
+ }
+}
Review Comment:
TableProvider::truncate docs say it returns an ExecutionPlan producing {count: UInt64}. This test returns EmptyExec which produces zero rows, it validates the hook was called, but not the contract.
Could tighten this to return an actual row and assert on it:
```suggestion
// Add these imports:
use arrow::array::UInt64Array;
use arrow::record_batch::RecordBatch;
use datafusion_physical_plan::test::TestMemoryExec;
// Then update the truncate impl:
async fn truncate(&self, _state: &dyn Session) -> Result<Arc<dyn ExecutionPlan>> {
*self.truncate_called.lock().unwrap() = true;
let schema = Arc::new(Schema::new(vec![
Field::new("count", DataType::UInt64, false),
]));
let batch = RecordBatch::try_new(
Arc::clone(&schema),
vec![Arc::new(UInt64Array::from(vec![0u64]))],
)?;
Ok(Arc::new(TestMemoryExec::try_new(&[vec![batch]], schema, None)?))
}
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#issuecomment-3707853176
@ethan-tyler, please review this PR. Thank you!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2702066012
##########
datafusion/catalog/src/table.rs:
##########
@@ -353,6 +353,14 @@ pub trait TableProvider: Debug + Sync + Send {
) -> Result<Arc<dyn ExecutionPlan>> {
not_impl_err!("UPDATE not supported for {} table", self.table_type())
}
+
+ /// Remove all rows from the table.
+ ///
+ /// Returns an [`ExecutionPlan`] producing a single row with `count` (UInt64),
+ /// representing the number of rows removed.
Review Comment:
I suppose we should slightly reword it to be like
> Should return an [`ExecutionPlan`] producing a single row with `count` (UInt64), representing the number of rows removed.
Just to make it clear this is an expectation of the contract 🤔
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2702162661
##########
datafusion/catalog/src/table.rs:
##########
@@ -353,6 +353,14 @@ pub trait TableProvider: Debug + Sync + Send {
) -> Result<Arc<dyn ExecutionPlan>> {
not_impl_err!("UPDATE not supported for {} table", self.table_type())
}
+
+ /// Remove all rows from the table.
+ ///
+ /// Returns an [`ExecutionPlan`] producing a single row with `count` (UInt64),
+ /// representing the number of rows removed.
Review Comment:
ok
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2702165184
##########
datafusion/sql/src/statement.rs:
##########
@@ -1390,6 +1390,56 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
exec_err!("Function name not provided")
}
}
+ Statement::Truncate(ast::Truncate {
+ table_names,
+ partitions,
+ identity,
+ cascade,
+ on_cluster,
+ table,
+ }) => {
+ let _ = table; // explicitly handled to satisfy full destructuring
Review Comment:
Should I update the comment to:
> // ignored because the table field does not influence planning or execution. `TRUNCATE` currently only supports a single resolved table.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Feat : added truncate table support [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19633:
URL: https://github.com/apache/datafusion/pull/19633#discussion_r2700758268
##########
datafusion/catalog/src/table.rs:
##########
@@ -353,6 +353,14 @@ pub trait TableProvider: Debug + Sync + Send {
) -> Result<Arc<dyn ExecutionPlan>> {
not_impl_err!("UPDATE not supported for {} table", self.table_type())
}
+
+ /// Remove all rows from the table.
+ ///
+ /// Returns an [`ExecutionPlan`] producing a single row with `count` (UInt64),
+ /// representing the number of rows removed.
Review Comment:
I attempted tightening the test using TestMemoryExec to materialize a {count: UInt64} row.
However, this triggers a physical optimizer rewrite, which calls `with_new_children` on `TestMemoryExec`, and that method is not implemented:
```
TestMemoryExec::with_new_children not implemented
```
Ethan pointed this out as well. Because the optimizer rewrites the plan, the test can’t reliably validate the actual output row. This follows the same pattern as the existing DELETE/UPDATE tests, which validate the schema and hook invocation rather than the concrete row content.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org