b41sh commented on code in PR #10445:
URL: https://github.com/apache/datafusion/pull/10445#discussion_r1597433886
##########
datafusion/expr/src/logical_plan/builder.rs:
##########
@@ -188,37 +181,61 @@ impl LogicalPlanBuilder {
n_cols
);
}
- field_types = row
- .iter()
- .enumerate()
- .map(|(j, expr)| {
- if let Expr::Literal(ScalarValue::Null) = expr {
- nulls.push((i, j));
- Ok(field_types[j].clone())
- } else {
- let data_type = expr.get_type(&empty_schema)?;
- if let Some(prev_data_type) = &field_types[j] {
- if prev_data_type != &data_type {
- return plan_err!("Inconsistent data type
across values list at row {i} column {j}. Was {prev_data_type} but found
{data_type}")
+ }
+
+ let empty_schema = DFSchema::empty();
Review Comment:
I understand what you mean, using the schema that the table already has can
avoid redundant checking and casting, but there is a problem,
`LogicalPlan::Values` needs a schema, and if we don't check the type at build
time, we can't get a usable schema, which might be a bit problematic. Also,
values can be executed separately without creating a table, for this scenario,
it might be better to determine the schema at build time.
--
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
