github-actions[bot] commented on code in PR #65264:
URL: https://github.com/apache/doris/pull/65264#discussion_r3589179298
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/OrderKey.java:
##########
@@ -112,4 +112,8 @@ public boolean equals(Object o) {
OrderKey that = (OrderKey) o;
return isAsc == that.isAsc() && nullFirst == that.isNullFirst() &&
expr.equals(that.getExpr());
}
+
+ public String shapeInfo() {
Review Comment:
This helper fixes callers that use it, but the detailed sort shape path
still bypasses it. `PhysicalQuickSort.shapeInfo()` renders detailed order keys
with `orderKeys.stream().map(OrderKey::toSql)` when `detail_shape_nodes`
contains `PhysicalQuickSort`, so a joined query sorted by qualified slots can
still print ambiguous keys there even though adjacent project/filter
expressions now use `shapeInfo()`. Please switch that call site to
`OrderKey::shapeInfo` and add a `detail_shape_nodes='PhysicalQuickSort'`
regression with qualified sort keys.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/BoundFunction.java:
##########
@@ -99,6 +99,20 @@ public String computeToSql() throws UnboundException {
return sql.append(")").toString();
}
+ @Override
+ public String shapeInfo() {
+ StringBuilder sql = new StringBuilder(getName()).append("(");
+ int arity = arity();
+ for (int i = 0; i < arity; i++) {
+ Expression arg = child(i);
+ sql.append(arg.shapeInfo());
Review Comment:
This generic renderer skips special rendering that previously came through
`toSql()`, and there are at least two current regressions.
`CryptoFunction.computeToSql()`/`toString()` intentionally mask argument 1 as
`'***'`, but AES/SM4 functions inherit this new `shapeInfo()` and now render
the real key literal through `arg.shapeInfo()`. High-order functions have the
same recursion gap: expressions such as `array_map` and
`array_filter`/`array_sort`-style paths can reach a `Lambda` child, `Lambda`
has no `shapeInfo()` override, and `Lambda.computeToSql()` renders its body and
array arguments with `toSql()`, so qualifiers inside lambda expressions are
still lost. Please keep the generic recursion, but add the needed `shapeInfo()`
overrides for these special renderers and cover both cases with shape-info
tests.
##########
regression-test/data/shape_check/tpcds_sf100/no_stats_shape/query58.out:
##########
@@ -6,9 +6,9 @@ PhysicalResultSink
------PhysicalDistribute[DistributionSpecGather]
--------PhysicalTopN[LOCAL_SORT]
----------PhysicalProject
-------------hashJoin[INNER_JOIN colocated] hashCondition=((ss_items.item_id =
ws_items.item_id)) otherCondition=((cast(cs_item_rev as DECIMALV3(38, 3)) <=
(1.1 * ws_items.ws_item_rev)) and (cast(cs_item_rev as DECIMALV3(38, 3)) >=
(0.9 * ws_items.ws_item_rev)) and (cast(ss_item_rev as DECIMALV3(38, 3)) <=
(1.1 * ws_items.ws_item_rev)) and (cast(ss_item_rev as DECIMALV3(38, 3)) >=
(0.9 * ws_items.ws_item_rev)) and (cast(ws_item_rev as DECIMALV3(38, 3)) <=
(1.1 * cs_items.cs_item_rev)) and (cast(ws_item_rev as DECIMALV3(38, 3)) <=
(1.1 * ss_items.ss_item_rev)) and (cast(ws_item_rev as DECIMALV3(38, 3)) >=
(0.9 * cs_items.cs_item_rev)) and (cast(ws_item_rev as DECIMALV3(38, 3)) >=
(0.9 * ss_items.ss_item_rev))) build RFs:RF13 item_id->i_item_id;RF14
item_id->i_item_id
+------------hashJoin[INNER_JOIN colocated] hashCondition=((ss_items.item_id =
ws_items.item_id)) otherCondition=((cast(cs_items.cs_item_rev as DECIMALV3(38,
3)) <= (1.1 * ws_items.ws_item_rev)) and (cast(cs_items.cs_item_rev as
DECIMALV3(38, 3)) >= (0.9 * ws_items.ws_item_rev)) and
(cast(ss_items.ss_item_rev as DECIMALV3(38, 3)) <= (1.1 *
ws_items.ws_item_rev)) and (cast(ss_items.ss_item_rev as DECIMALV3(38, 3)) >=
(0.9 * ws_items.ws_item_rev)) and (cast(ws_items.ws_item_rev as DECIMALV3(38,
3)) <= (1.1 * cs_items.cs_item_rev)) and (cast(ws_items.ws_item_rev as
DECIMALV3(38, 3)) <= (1.1 * ss_items.ss_item_rev)) and
(cast(ws_items.ws_item_rev as DECIMALV3(38, 3)) >= (0.9 *
cs_items.cs_item_rev)) and (cast(ws_items.ws_item_rev as DECIMALV3(38, 3)) >=
(0.9 * ss_items.ss_item_rev))) build RFs:RF13 item_id->i_item_id;RF14
item_id->i_item_id
Review Comment:
This output still exposes another shape renderer that did not move from
`toSql()` to `shapeInfo()`: the join predicates are now qualified, but the
`build RFs` endpoints remain `item_id->i_item_id`.
`PhysicalHashJoin.shapeInfo()` gets this text from `RuntimeFilter.shapeInfo()`,
which currently formats both `getSrcExpr()` and `targetExpression` with
`toSql()`. Please change that runtime-filter renderer to use `shapeInfo()` for
both sides and refresh/add coverage with qualified RF endpoints.
--
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]
