Copilot commented on code in PR #24281:
URL: https://github.com/apache/datafusion/pull/24281#discussion_r3764655294
##########
datafusion/optimizer/src/optimize_projections/mod.rs:
##########
@@ -1174,6 +1230,80 @@ mod tests {
}
}
+ /// `project_schema_by_indices` must produce exactly the schema that
+ /// recomputing it from scratch (`projection_schema`) would, for every
subset
+ /// of a projection's expressions. This is the correctness invariant that
lets
+ /// `rewrite_projection_given_requirements` reuse the parent schema
instead of
+ /// re-deriving each field's type via `Expr::to_field`.
+ #[test]
+ fn project_schema_by_indices_matches_recompute() -> Result<()> {
+ use super::project_schema_by_indices;
+ use datafusion_expr::logical_plan::projection_schema;
+
+ let input = Arc::new(test_table_scan()?); // columns: a, b, c (UInt32,
NOT NULL)
+
+ // A deliberately mixed expression list: plain column, computed binary
+ // expr, alias, nullable literal, and a qualified column.
+ let exprs = vec![
+ col("a"),
+ binary_expr(col("b"), Operator::Plus, col("c")),
+ col("c").alias("c_alias"),
+ lit(1_i64).alias("one"),
+ Expr::Column(Column::new(Some(TableReference::bare("test")), "b")),
Review Comment:
The test comment says this expression list includes a nullable literal, but
`lit(1_i64)` is non-nullable. Making the literal actually NULL improves
coverage for field nullability propagation (one of the key invariants this test
is meant to validate).
This issue also appears on line 1279 of the same file.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
