alamb commented on code in PR #6595: URL: https://github.com/apache/arrow-datafusion/pull/6595#discussion_r1225777088
########## datafusion/common/src/dfschema.rs: ########## @@ -384,8 +384,7 @@ impl DFSchema { let self_fields = self.fields().iter(); let other_fields = other.fields().iter(); self_fields.zip(other_fields).all(|(f1, f2)| { - f1.qualifier() == f2.qualifier() - && f1.name() == f2.name() + f1.qualified_name() == f2.qualified_name() Review Comment: I am not sure if it matters, but I believe that `qualified_name` creates a new `String` where the previous version avoids that allocation. ########## datafusion/core/tests/sqllogictests/test_files/array.slt: ########## @@ -110,10 +111,10 @@ select array_position(['h', 'e', 'l', 'l', 'o'], 'l', 4), array_position([1, 2, 4 5 2 # array_positions scalar function -query III +query error DataFusion error: SQL error: ParserError\("Expected an SQL statement, found: caused"\) Review Comment: these errors look not quite right -- I think there is something wrong with sqllogictest `--complete` with multi-line errors 🤔 ########## datafusion/core/tests/sql/group_by.rs: ########## @@ -18,6 +18,8 @@ use super::*; #[tokio::test] +#[ignore] +// TODO: issue: https://github.com/apache/arrow-datafusion/issues/6623 Review Comment: Here is a proposed fix: https://github.com/apache/arrow-datafusion/pull/6632 ########## datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs: ########## @@ -206,7 +206,16 @@ impl<'a> TreeNodeRewriter for ConstEvaluator<'a> { fn mutate(&mut self, expr: Expr) -> Result<Expr> { match self.can_evaluate.pop() { - Some(true) => Ok(Expr::Literal(self.evaluate_to_scalar(expr)?)), + Some(true) => { + // After simplifying the expression, data_type may change, so we need to cast it. Review Comment: This seems wrong to me -- the type of the expression should always be the same before/after simplification. If it isn't the same that is a bug (perhaps we could raise an internal error here?) -- 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...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org