github-actions[bot] commented on code in PR #68672:
URL: https://github.com/apache/doris/pull/68672#discussion_r4144328668
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Cast.java:
##########
@@ -93,6 +93,17 @@ protected Cast(List<Expression> child, DataType targetType,
boolean isExplicitTy
this.isStrict = isStrict;
}
+ @Override
+ public boolean isDeterministic() {
+ // TIME has no date. These casts supply the date from the current
query's clock.
+ return child().isNullLiteral() || !(child().getDataType().isTimeType()
&& targetType.isDateLikeType());
Review Comment:
[P1] Classify nested TIME-to-date casts as volatile.
`CAST(ARRAY(TIMEDIFF(dt1,dt2)) AS ARRAY<DATETIME>)` passes the recursive cast
check, and BE's array element cast uses the query timestamp to supply each
element's date. Here the source and target are ARRAY types, so
`isDeterministic()` returns true and SQL/query cache can reuse yesterday's
result for unchanged rows. The same path exists for MAP and STRUCT members;
check nested source/target type pairs before declaring this cast cacheable.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java:
##########
@@ -949,6 +947,8 @@ private PlanFragment
computePhysicalOlapScan(PhysicalOlapScan olapScan, PlanTran
Review Comment:
[P1] Check runtime-filter probes before enabling scan condition cache. A
hash join on `CAST(CAST(p.s AS TIME) AS DATETIME) = b.d` can push a runtime
filter on `p.s` through a cast-wrapped slot, while this scan has no
`PhysicalFilter` or virtual column and is set cacheable. BE hashes the filter's
cast and payload, but the TIME-to-DATETIME result also depends on the query
date. With an unchanged build-side `b.d` for tomorrow, a granule cached as all
false before midnight can contain matches after midnight and be skipped.
Disable the scan cache for such RF targets or include the query-clock
dependency in the digest.
##########
gensrc/thrift/PlanNodes.thrift:
##########
@@ -1809,6 +1809,9 @@ struct TPlanNode {
106: optional list<i32> topn_filter_source_node_ids
107: optional i32 nereids_id
+ // FE expression eligibility, independent of the session switch. An old FE
has not checked
+ // volatility, so absence must disable condition cache on a new BE.
+ 108: optional bool enable_condition_cache = false
Review Comment:
[P1] Protect volatile scans on older BEs during rolling upgrades. A new FE
still sends a nonzero query-wide `condition_cache_digest` when the session
switch is on, but an old BE ignores this new optional plan-node field and its
scan-open path checks only that digest. For `Filter(k < 0 OR rand() < 0.0001)
-> DUP Scan`, the old BE can reuse an all-false segment granule and skip rows
that would match on a later execution. Suppress the digest for plans with
unsafe scans until every scheduled BE understands this flag, or gate cache use
by BE capability.
--
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]
