alamb commented on code in PR #14631: URL: https://github.com/apache/datafusion/pull/14631#discussion_r1953445303
########## datafusion/core/src/datasource/memory.rs: ########## @@ -648,9 +649,14 @@ mod tests { // Create a table scan logical plan to read from the source table let scan_plan = LogicalPlanBuilder::scan("source", source, None)?.build()?; // Create an insert plan to insert the source data into the initial table - let insert_into_table = - LogicalPlanBuilder::insert_into(scan_plan, "t", &schema, InsertOp::Append)? - .build()?; + let insert_into_table = LogicalPlanBuilder::insert_into( + scan_plan, + "t", + table_sink, + &schema, Review Comment: Given the table_source already has the target schema, we could avoid storing it again ########## datafusion/expr/src/logical_plan/dml.rs: ########## @@ -91,10 +91,12 @@ impl Hash for CopyTo { /// The operator that modifies the content of a database (adapted from /// substrait WriteRel) -#[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[derive(Clone)] pub struct DmlStatement { /// The table name pub table_name: TableReference, + /// this is sink table which corresponds to table_name reference Review Comment: ```suggestion /// this is target table to insert into ``` ########## datafusion/core/src/datasource/memory.rs: ########## @@ -648,9 +649,14 @@ mod tests { // Create a table scan logical plan to read from the source table let scan_plan = LogicalPlanBuilder::scan("source", source, None)?.build()?; // Create an insert plan to insert the source data into the initial table - let insert_into_table = - LogicalPlanBuilder::insert_into(scan_plan, "t", &schema, InsertOp::Append)? - .build()?; + let insert_into_table = LogicalPlanBuilder::insert_into( + scan_plan, + "t", + table_sink, Review Comment: I found the name of `table_sink` to be confusing here (as the type is called `TableSource` I would expect it to be called `table_source` for consistency) ########## datafusion/core/src/datasource/memory.rs: ########## @@ -648,9 +649,14 @@ mod tests { // Create a table scan logical plan to read from the source table let scan_plan = LogicalPlanBuilder::scan("source", source, None)?.build()?; // Create an insert plan to insert the source data into the initial table - let insert_into_table = - LogicalPlanBuilder::insert_into(scan_plan, "t", &schema, InsertOp::Append)? - .build()?; + let insert_into_table = LogicalPlanBuilder::insert_into( + scan_plan, + "t", + table_sink, Review Comment: But now I see that it is a sink because it is the target of the DML operations Maybe `target` would be clearer 🤔 -- 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-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org