alamb commented on code in PR #14631:
URL: https://github.com/apache/datafusion/pull/14631#discussion_r1956042470
##########
datafusion/sql/src/statement.rs:
##########
@@ -1709,18 +1709,22 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
// Do a table lookup to verify the table exists
let table_ref =
self.object_name_to_table_reference(table_name.clone())?;
let table_source =
self.context_provider.get_table_source(table_ref.clone())?;
- let schema = (*table_source.schema()).clone();
- let schema = DFSchema::try_from(schema)?;
- let scan =
- LogicalPlanBuilder::scan(table_ref.clone(), table_source,
None)?.build()?;
+ let schema = table_source.schema().to_dfschema_ref()?;
+ let scan = LogicalPlanBuilder::scan(
+ //object_name_to_string(&table_name),
Review Comment:
left over?
##########
datafusion/expr/src/logical_plan/dml.rs:
##########
@@ -91,31 +91,64 @@ 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,
- /// The schema of the table (must align with Rel input)
- pub table_schema: DFSchemaRef,
+ /// this is target table to insert into
+ pub target: Arc<dyn TableSource>,
/// The type of operation to perform
pub op: WriteOp,
/// The relation that determines the tuples to add/remove/modify the
schema must match with table_schema
pub input: Arc<LogicalPlan>,
/// The schema of the output relation
pub output_schema: DFSchemaRef,
}
+impl Eq for DmlStatement {}
+impl Hash for DmlStatement {
+ fn hash<H: Hasher>(&self, state: &mut H) {
+ self.table_name.hash(state);
+ self.target.schema().hash(state);
Review Comment:
should it also look at the target name 🤔
##########
datafusion/core/src/dataframe/mod.rs:
##########
@@ -1536,10 +1537,20 @@ impl DataFrame {
.build()?
};
+ let table_ref: TableReference = table_name.into();
+ let table = table_ref.table().to_string();
+ let table_schema = self.session_state.schema_for_ref(table_ref)?;
+ let target = match table_schema.table(&table).await? {
+ Some(ref provider) => Ok(Arc::clone(provider)),
+ _ => plan_err!("No table named '{table}'"),
+ }?;
+
+ let target = Arc::new(DefaultTableSource::new(target));
Review Comment:
I wasn't sure why it was required to go from `table_name` --> `table_ref`
--> `table` (another name) again. I tried this locally and it seemed to work
well (all tests passed)
```suggestion
let table_ref: TableReference = table_name.into();
let table_schema =
self.session_state.schema_for_ref(table_ref.clone())?;
let target = match table_schema.table(table_ref.table()).await? {
Some(ref provider) => Ok(Arc::clone(provider)),
_ => plan_err!("No table named '{table_name}'"),
}?;
let target = Arc::new(DefaultTableSource::new(target));
```
--
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
