Make alias_symbol more human-readable

[JasonLi-cn]

Is your feature request related to a problem or challenge?

DataFusion CLI v37.1.0
> select * from number;
+----+----+----+
| c0 | c1 | c2 |
+----+----+----+
| 1  | 2  | 3  |
+----+----+----+
1 row(s) fetched.
Elapsed 0.005 seconds.

> explain select c0 + 1, count(c0 + 1) from number group by c0 + 1;
+---------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| plan_type     | plan                                                                                                                                                                                                                                              |
+---------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| logical_plan  | Aggregate: groupBy=[[CAST(number.c0 AS Int64) + Int64(1)Int64(1)CAST(number.c0 AS Int64)number.c0 AS number.c0 + Int64(1)]], aggr=[[COUNT(CAST(number.c0 AS Int64) + Int64(1)Int64(1)CAST(number.c0 AS Int64)number.c0 AS number.c0 + Int64(1))]] |
|               |   Projection: CAST(number.c0 AS Int64) + Int64(1) AS CAST(number.c0 AS Int64) + Int64(1)Int64(1)CAST(number.c0 AS Int64)number.c0                                                                                                                 |
|               |     TableScan: number projection=[c0]                                                                                                                                                                                                             |
| physical_plan | AggregateExec: mode=FinalPartitioned, gby=[number.c0 + Int64(1)@0 as number.c0 + Int64(1)], aggr=[COUNT(number.c0 + Int64(1))]                                                                                                                    |
|               |   CoalesceBatchesExec: target_batch_size=8192                                                                                                                                                                                                     |
|               |     RepartitionExec: partitioning=Hash([number.c0 + Int64(1)@0], 8), input_partitions=1                                                                                                                                                           |
|               |       AggregateExec: mode=Partial, gby=[CAST(number.c0 AS Int64) + Int64(1)Int64(1)CAST(number.c0 AS Int64)number.c0@0 as number.c0 + Int64(1)], aggr=[COUNT(number.c0 + Int64(1))]                                                               |
|               |         ProjectionExec: expr=[CAST(c0@0 AS Int64) + 1 as CAST(number.c0 AS Int64) + Int64(1)Int64(1)CAST(number.c0 AS Int64)number.c0]                                                                                                            |
|               |           MemoryExec: partitions=1, partition_sizes=[1]                                                                                                                                                                                           |
|               |                                                                                                                                                                                                                                                   |
+---------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
2 row(s) fetched.
Elapsed 0.015 seconds.

ProjectionExec: expr=[CAST(c0@0 AS Int64) + 1 as CAST(number.c0 AS Int64) + Int64(1)Int64(1)CAST(number.c0 AS Int64)number.c0]                                                                                                            |

The expr of ProjectionExec is confusing.

The related code:

datafusion/datafusion/optimizer/src/common_subexpr_eliminate.rs

Lines 676 to 687 in b41ef20

	fn f_up(&mut self, expr: &Expr) -> Result<TreeNodeRecursion> {
	let (_idx, sub_expr_identifier) = self.pop_enter_mark();
	// skip exprs should not be recognize.
	if self.expr_mask.ignores(expr) {
	let curr_expr_identifier = ExprSet::expr_identifier(expr);
	self.visit_stack
	.push(VisitRecord::ExprItem(curr_expr_identifier));
	return Ok(TreeNodeRecursion::Continue);
	}
	let curr_expr_identifier = ExprSet::expr_identifier(expr);
	let alias_symbol = format!("{curr_expr_identifier}{sub_expr_identifier}");

Maybe we should make it more human-readable.

Describe the solution you'd like

ProjectionExec: expr=[CAST(c0@0 AS Int64) + 1 as CAST(number.c0 AS Int64) + Int64(1)]                                                                                                            

or

ProjectionExec: expr=[CAST(c0@0 AS Int64) + 1 as CAST(number.c0 AS Int64) + Int64(1)<$SUB_IDEN...>]                                                                                              

Describe alternatives you've considered

No response

Additional context

No response

[MohamedAbdeen21]

Hey, this optimizer rule is used by so many test cases, so I wanted to collect opinions before proceeding with implementing the change.

My suggestion

Using let alias_symbol = format!("${{{curr_expr_id}}}"); to mark common expressions. So the query above would be:

 AggregateExec: mode=Partial, gby=[${CAST(number.c0 AS Int64) + Int64(1)}@0 as number.c0 + Int64(1)], aggr=[COUNT(number.c0 + Int64(1))] 
    ProjectionExec: expr=[CAST(c0@0 AS Int64) + 1 as ${CAST(number.c0 AS Int64) + Int64(1)}] 

Or to make plans shorter, $0, $1, etc...

Some naming alternatives I've considered

• Using @0, @1, etc ... However, the @ symbol is already used in the physical plan (check the query above), which means that you may have columns like @0@0 which makes no sense.

• let alias_symbol = format!("{curr_expr_id}").replace(" ", "_");. Sound like a good idea until you see it.

AggregateExec: mode=Partial, gby=[CAST(number.c0_AS_Int64)_+_Int64(1)@0 as number.c0 + Int64(1)], aggr=[COUNT(number.c0 + Int64(1))] 
    ProjectionExec: expr=[CAST(c0@0 AS Int64) + 1 as CAST(number.c0_AS_Int64)_+_Int64(1)

It's not immediately obvious + what happens if the column doesn't have a space?

Please feel free to share other alternatives.

[alamb]

#10333 is a really nice PR 💯

[alamb]

We unfortunately found issues in #10333 so we are going to revert it in #10436

See discussion here: #10436 (review)