alamb commented on code in PR #6049:
URL: https://github.com/apache/arrow-datafusion/pull/6049#discussion_r1179619431
##########
datafusion/core/src/physical_plan/memory.rs:
##########
@@ -223,15 +245,365 @@ impl RecordBatchStream for MemoryStream {
}
}
+/// Execution plan for writing record batches to an in-memory table.
+pub struct MemoryWriteExec {
Review Comment:
> All in all, having a distinct ExecutionPlan to handle writes for each data
source mirrors the behavior on the read side and seems much more natural. What
do you think?
I guess I don't fully understand why adding 800 lines of `MemTable` specific
ExecutionPlan that seems to add no new functionality is an improvement
Not only only this this pattern seem to overly complicate this PR, but it
sets us on a course for every other table source to have to do the same (mostly
repeated) things.
Perhaps there is a future use case I don't understand.
If you guys feel strongly that this is the right and maintainable solution
going forward I will defer to your judgement, but from where I see it just adds
complexity / technical debt for no reason.
--
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]
