datafusion icon indicating copy to clipboard operation
datafusion copied to clipboard

Better Cast name for display

Open yyy1000 opened this issue 9 months ago • 5 comments

Which issue does this PR close?

Closes #10274 .

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

yyy1000 avatar Apr 29 '24 01:04 yyy1000

I encountered a problem for type_coercion and need help🥲 Here TypeCoercion would rewrite an Expr thus brings a 'Cast', and the name for Cast would not just be the expr name in the Cast in this PR, thus rewrite_preserving_name will introduce an Alias, which is not expected. https://github.com/apache/datafusion/blob/b41ef20c5dad7bdd674e3cc5f35a9c99efae676c/datafusion/optimizer/src/analyzer/type_coercion.rs#L102-L116

yyy1000 avatar Apr 29 '24 05:04 yyy1000

I think we should change the display_name not create_name, as the comment said, name for casting should preserve

See https://github.com/apache/datafusion/pull/3727 for why preserving name

jayzhan211 avatar Apr 29 '24 07:04 jayzhan211

Looks like this one has some test failures to review before it is ready for review

alamb avatar Apr 29 '24 19:04 alamb

I encountered an issue: the plan's schema would change during rewrite stage.👀 In

explain verbose SELECT CAST(2 AS INTEGER);

+----------------------------------------------------------------------------+--------------------------------------------------------------------------------------+
| plan_type                                                                  | plan                                                                                 |
+----------------------------------------------------------------------------+--------------------------------------------------------------------------------------+
| initial_logical_plan                                                       | Projection: CAST(Int64(2) AS Int32)                                                  |
|                                                                            |   EmptyRelation                                                                      |
| logical_plan after apply_function_rewrites                                 | SAME TEXT AS ABOVE                                                                   |
| logical_plan after inline_table_scan                                       | SAME TEXT AS ABOVE                                                                   |
| logical_plan after type_coercion                                           | SAME TEXT AS ABOVE                                                                   |
| logical_plan after count_wildcard_rule                                     | SAME TEXT AS ABOVE                                                                   |
| analyzed_logical_plan                                                      | SAME TEXT AS ABOVE                                                                   |
| logical_plan after eliminate_nested_union                                  | SAME TEXT AS ABOVE                                                                   |
| logical_plan after simplify_expressions                                    | Projection: Int32(2) AS Int64(2)                                                     |
|                                                                            |   EmptyRelation                                                                      |
| logical_plan after unwrap_cast_in_comparison                               | SAME TEXT AS ABOVE                                                                   |
| logical_plan after replace_distinct_aggregate                              | SAME TEXT AS ABOVE                                                                   |
| logical_plan after eliminate_join                                          | SAME TEXT AS ABOVE                                                                   |
| logical_plan after decorrelate_predicate_subquery                          | SAME TEXT AS ABOVE                                                                   |
| logical_plan after scalar_subquery_to_join                                 | SAME TEXT AS ABOVE                                                                   |
| logical_plan after extract_equijoin_predicate                              | SAME TEXT AS ABOVE                                                                   |
| logical_plan after simplify_expressions                                    | SAME TEXT AS ABOVE                                                                   |
| logical_plan after rewrite_disjunctive_predicate                           | SAME TEXT AS ABOVE                                                                   |
| logical_plan after eliminate_duplicated_expr                               | SAME TEXT AS ABOVE                                                                   |
| logical_plan after eliminate_filter                                        | SAME TEXT AS ABOVE                                                                   |
| logical_plan after eliminate_cross_join                                    | SAME TEXT AS ABOVE                                                                   |
| logical_plan after Optimizer rule 'common_sub_expression_eliminate' failed | Schema error: No field named "CAST(Int64(2) AS Int32)". Valid fields are "Int64(2)". |
+----------------------------------------------------------------------------+--------------------------------------------------------------------------------------+
20 row(s) fetched. 
Elapsed 0.008 seconds.

After simplify_expressions, ConstEvaluator will rewrite the Cast to an Alias to ScalarValue, and the name for the Alias is different from the Field name in the original schema (Because in the past the Cast's name is just the expr, which is the same as the Alias's name, but now it's not.

I will dive into related code but it may take some time. :) I'm not so familiar with related code base but I will try to learn it.

yyy1000 avatar Apr 30 '24 01:04 yyy1000

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Jun 29 '24 01:06 github-actions[bot]