Describe the bug
EXPLAIN DELETE and EXPLAIN UPDATE change the rows of an in-memory table. The plan is printed, and the statement has also run.
Two causes combine.
-
handle_explain() builds the physical plan of the statement in order to render the physical_plan section (datafusion/core/src/physical_planner.rs:2729-2733 for the indent format, :2678-2686 for the tree format). Only the PostgresJSON and Graphviz formats stop at the logical plan.
-
The physical planner calls the provider hook while it builds the plan. The WriteOp::Delete arm awaits TableProvider::delete_from() (:791-812), and the WriteOp::Update arm awaits TableProvider::update() (:813-838).
MemTable then does the whole row change inside the hook. delete_from_inner() takes a write lock on each partition and overwrites it with the surviving batches, then returns DmlResultExec::new(total_deleted) (datafusion/catalog/src/memory/table.rs:367-428). update_inner() does the same (:440-572). DmlResultExec is a constant node: its execute() only emits the count that the hook already computed (:702-720). The hook also clears the declared sort order of the table (:376, :483), which is a second observable effect of the EXPLAIN.
delete.slt carries the evidence. Every explain delete case prints 02)--DmlResultExec: rows_affected=0, and the count is baked into the plan text because the delete already ran against the empty table t1.
To Reproduce
> create table t as values (1), (2), (3);
> explain delete from t where column1 > 1;
+---------------+-------------------------------------+
| plan_type | plan |
+---------------+-------------------------------------+
| logical_plan | Dml: op=[Delete] table=[t] |
| | Filter: t.column1 > Int64(1) |
| | TableScan: t |
| physical_plan | CooperativeExec |
| | DmlResultExec: rows_affected=2 |
+---------------+-------------------------------------+
> select * from t;
+---------+
| column1 |
+---------+
| 1 |
+---------+
The rows are gone, and the count appears in the plan text rather than in a result.
Expected behavior
EXPLAIN DELETE and EXPLAIN UPDATE print the plan and change nothing. The three rows survive.
EXPLAIN ANALYZE DELETE is expected to change the rows: it runs the plan by design, and it must run the statement exactly once.
The provider documentation states the rule that the fix should follow: do the row change in the execute() method of the plan that the hook returns, so the hook stays lightweight. MemTable should follow its own guidance, since the documentation names it as the reference implementation.
Additional context
The documentation added by pull request #24567 records the behaviour as a warning in two places, and neither cites a tracker issue:
docs/source/user-guide/sql/dml.md: "EXPLAIN executes a DELETE or an UPDATE on an in-memory table ... Use a copy of the table if you want to read the plan only."
docs/source/library-user-guide/custom-table-providers.md: "The hooks run during physical planning, like scan(). A hook that changes rows before it returns its plan therefore changes them during planning, and EXPLAIN DELETE or EXPLAIN UPDATE also changes them. MemTable works this way."
Notes for whoever takes the fix:
- No test asserts that
EXPLAIN leaves the data alone. delete.slt and update.slt hold the EXPLAIN cases and pass only because their tables are empty. dml_delete.slt and dml_update.slt hold the behavioural cases and use no EXPLAIN.
- The mutation moves out of the hook, so the row count is no longer known while the plan is built. The display text of the new plan node cannot carry
rows_affected, and the eight expectations in delete.slt and update.slt need regenerating.
- The two fields of
MemTable that the operation needs, batches and sort_order, already sit behind an Arc, so the plan node can hold clones and apply the change in execute().
- A plan that changes rows in
execute() changes them once per run, so a caller that runs the plan twice applies the statement twice. DataSinkExec behaves the same way for an INSERT.
- Keep every planning error in the hook. The
UPDATE check for an unknown column raises a plan error today (datafusion/catalog/src/memory/table.rs:460-466), and EXPLAIN should still report it.
insert_into() clears the sort order inside the hook as well (:344), so EXPLAIN INSERT keeps that smaller effect. Worth a follow-up.
Describe the bug
EXPLAIN DELETEandEXPLAIN UPDATEchange the rows of an in-memory table. The plan is printed, and the statement has also run.Two causes combine.
handle_explain()builds the physical plan of the statement in order to render thephysical_plansection (datafusion/core/src/physical_planner.rs:2729-2733for the indent format,:2678-2686for the tree format). Only thePostgresJSONandGraphvizformats stop at the logical plan.The physical planner calls the provider hook while it builds the plan. The
WriteOp::Deletearm awaitsTableProvider::delete_from()(:791-812), and theWriteOp::Updatearm awaitsTableProvider::update()(:813-838).MemTablethen does the whole row change inside the hook.delete_from_inner()takes a write lock on each partition and overwrites it with the surviving batches, then returnsDmlResultExec::new(total_deleted)(datafusion/catalog/src/memory/table.rs:367-428).update_inner()does the same (:440-572).DmlResultExecis a constant node: itsexecute()only emits the count that the hook already computed (:702-720). The hook also clears the declared sort order of the table (:376,:483), which is a second observable effect of theEXPLAIN.delete.sltcarries the evidence. Everyexplain deletecase prints02)--DmlResultExec: rows_affected=0, and the count is baked into the plan text because the delete already ran against the empty tablet1.To Reproduce
The rows are gone, and the count appears in the plan text rather than in a result.
Expected behavior
EXPLAIN DELETEandEXPLAIN UPDATEprint the plan and change nothing. The three rows survive.EXPLAIN ANALYZE DELETEis expected to change the rows: it runs the plan by design, and it must run the statement exactly once.The provider documentation states the rule that the fix should follow: do the row change in the
execute()method of the plan that the hook returns, so the hook stays lightweight.MemTableshould follow its own guidance, since the documentation names it as the reference implementation.Additional context
The documentation added by pull request #24567 records the behaviour as a warning in two places, and neither cites a tracker issue:
docs/source/user-guide/sql/dml.md: "EXPLAINexecutes aDELETEor anUPDATEon an in-memory table ... Use a copy of the table if you want to read the plan only."docs/source/library-user-guide/custom-table-providers.md: "The hooks run during physical planning, likescan(). A hook that changes rows before it returns its plan therefore changes them during planning, andEXPLAIN DELETEorEXPLAIN UPDATEalso changes them. MemTable works this way."Notes for whoever takes the fix:
EXPLAINleaves the data alone.delete.sltandupdate.slthold theEXPLAINcases and pass only because their tables are empty.dml_delete.sltanddml_update.slthold the behavioural cases and use noEXPLAIN.rows_affected, and the eight expectations indelete.sltandupdate.sltneed regenerating.MemTablethat the operation needs,batchesandsort_order, already sit behind anArc, so the plan node can hold clones and apply the change inexecute().execute()changes them once per run, so a caller that runs the plan twice applies the statement twice.DataSinkExecbehaves the same way for anINSERT.UPDATEcheck for an unknown column raises a plan error today (datafusion/catalog/src/memory/table.rs:460-466), andEXPLAINshould still report it.insert_into()clears the sort order inside the hook as well (:344), soEXPLAIN INSERTkeeps that smaller effect. Worth a follow-up.