pepijnve commented on code in PR #21739:
URL: https://github.com/apache/datafusion/pull/21739#discussion_r3222022355
##########
datafusion/proto/src/physical_plan/mod.rs:
##########
@@ -118,6 +118,58 @@ use crate::{convert_required, into_required};
pub mod from_proto;
pub mod to_proto;
+const HUMAN_DISPLAY_ALIAS_PREFIX: &str =
"\u{1f}datafusion_human_display_alias_v1:";
+
+fn encode_human_display_alias(human_display: &str, alias: &str) -> String {
+ format!(
+ "{HUMAN_DISPLAY_ALIAS_PREFIX}{}:{alias}{human_display}",
Review Comment:
An alternative here that may be kinder to other consumers would be to encode
using `{human_display} as {alias}` kind of like the code was doing before. The
split logic is less robust, but maybe good enough?
##########
datafusion/sqllogictest/test_files/tpch/plans/q1.slt.part:
##########
@@ -50,9 +50,9 @@ physical_plan
01)SortPreservingMergeExec: [l_returnflag@0 ASC NULLS LAST, l_linestatus@1 ASC
NULLS LAST]
02)--SortExec: expr=[l_returnflag@0 ASC NULLS LAST, l_linestatus@1 ASC NULLS
LAST], preserve_partitioning=[true]
03)----ProjectionExec: expr=[l_returnflag@0 as l_returnflag, l_linestatus@1 as
l_linestatus, sum(lineitem.l_quantity)@2 as sum_qty,
sum(lineitem.l_extendedprice)@3 as sum_base_price, sum(lineitem.l_extendedprice
* Int64(1) - lineitem.l_discount)@4 as sum_disc_price,
sum(lineitem.l_extendedprice * Int64(1) - lineitem.l_discount * Int64(1) +
lineitem.l_tax)@5 as sum_charge, avg(lineitem.l_quantity)@6 as avg_qty,
avg(lineitem.l_extendedprice)@7 as avg_price, avg(lineitem.l_discount)@8 as
avg_disc, count(Int64(1))@9 as count_order]
-04)------AggregateExec: mode=FinalPartitioned, gby=[l_returnflag@0 as
l_returnflag, l_linestatus@1 as l_linestatus], aggr=[sum(lineitem.l_quantity),
sum(lineitem.l_extendedprice), sum(lineitem.l_extendedprice * Int64(1) -
lineitem.l_discount), sum(lineitem.l_extendedprice * Int64(1) -
lineitem.l_discount * Int64(1) + lineitem.l_tax), avg(lineitem.l_quantity),
avg(lineitem.l_extendedprice), avg(lineitem.l_discount), count(Int64(1))]
+04)------AggregateExec: mode=FinalPartitioned, gby=[l_returnflag@0 as
l_returnflag, l_linestatus@1 as l_linestatus], aggr=[sum(lineitem.l_quantity),
sum(lineitem.l_extendedprice), sum(__common_expr_1) as
sum(lineitem.l_extendedprice * Int64(1) - lineitem.l_discount),
sum(__common_expr_1 * Some(1),20,0 + lineitem.l_tax) as
sum(lineitem.l_extendedprice * Int64(1) - lineitem.l_discount * Int64(1) +
lineitem.l_tax), avg(lineitem.l_quantity), avg(lineitem.l_extendedprice),
avg(lineitem.l_discount), count(Int64(1))]
Review Comment:
Based on @adriangb's comment I think this change may be controversial. Do we
want more input on this?
##########
datafusion/expr/src/expr.rs:
##########
@@ -677,6 +677,7 @@ impl Hash for Alias {
self.expr.hash(state);
self.relation.hash(state);
self.name.hash(state);
+ self.metadata.hash(state);
Review Comment:
Does including metadata in equality and hash calculations risk any
unintended side effects? If it's not required for the plan change itself, it
might be safer to not include this in this PR.
--
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]
