github-actions[bot] commented on code in PR #64366:
URL: https://github.com/apache/doris/pull/64366#discussion_r3418715609
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/load/NereidsStreamLoadPlanner.java:
##########
@@ -213,10 +214,10 @@ public TPipelineFragmentParams plan(TUniqueId loadId, int
fragmentInstanceIdInde
// make sure StatementContext is set in ConnectContext
ConnectContext connectContext = ConnectContext.get();
- if (connectContext != null && connectContext.getStatementContext() ==
null) {
- StatementContext statementContext = new StatementContext();
+ if (connectContext != null) {
Review Comment:
This now replaces any existing `ConnectContext` statement context, while the
old code only installed one when it was missing. That is unsafe for callers
that invoke this planner inside another statement:
`InsertUtils.beginTransaction()` ->
`InsertStreamTxnExecutor.beginTransaction()` and the first `GroupCommitPlanner`
build both enter with the outer insert/prepared `StatementContext` already on
the connection. After this assignment returns,
`ConnectContext.get().getStatementContext()` points at a fresh load-planning
context instead of the outer statement context, so later code using the
thread-local context can lose placeholders, statement flags, hints/snapshots,
and cleanup/audit state. Please preserve and restore the original context, or
avoid installing the task context globally and pass the task timestamp only to
the collector/query globals that need it.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnFE.java:
##########
@@ -170,6 +178,7 @@ public List<ExpressionPatternMatcher<? extends Expression>>
buildRules() {
matches(Or.class, this::visitOr),
matches(TryCast.class, this::visitTryCast),
matches(Cast.class, this::visitCast),
+ matches(Now.class, this::visitNow),
Review Comment:
This only routes `Now` through `StatementContext`; the other statement-time
acquisition functions still fall through `BoundFunction` ->
`ExpressionEvaluator` -> `DateTimeAcquire` and read the clock again
(`unix_timestamp()` uses `System.currentTimeMillis()`, while
`current_date()`/`current_time()`/`curtime()` use `LocalDateTime.now()`). A
statement such as `select now(6), from_unixtime(unix_timestamp())`, or a load
predicate/partition pruning expression using `current_date()`, can still mix
the captured statement instant with a later FE fold-time instant while BE
runtime uses the statement timestamp. Please route all current-time acquisition
functions through the same statement instant, or leave them unfolded for
BE/runtime evaluation.
--
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]
