github-actions[bot] commented on code in PR #64366:
URL: https://github.com/apache/doris/pull/64366#discussion_r3601851923
##########
be/src/service/point_query_executor.cpp:
##########
@@ -308,6 +318,7 @@ Status PointQueryExecutor::init(const
PTabletKeyLookupRequest* request,
_tablet = DORIS_TRY(ExecEnv::get_tablet(request->tablet_id()));
if (cache_handle != nullptr) {
_reusable = cache_handle;
+ _reusable->update_runtime_state(*request);
Review Comment:
Updating only `RuntimeState` here cannot change a constant that the cached
output expression already materialized. `Reusable::init()` opens these contexts
once, `VExpr::open()` stores all-constant trees in `_constant_col`, and
`VExprContext` will not reopen them; meanwhile
`ShortCircuitQueryContext.isReusable()` ignores timezone changes. Thus, with
session `debug_skip_fold_constant=true`, a prepared deterministic point query
can execute `from_unixtime(20)` in `+00:00`, change the same session to
`+08:00`, hit this branch, and still return the first UTC constant. This is
distinct from the earlier cache-miss thread: the first execution is now
correct, but a later execution with different globals is stale. Please
invalidate/rebuild the reusable entry when relevant globals change or safely
rematerialize request-dependent constants, and add a two-execution
timezone-transition test.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/BaseAnalysisTask.java:
##########
@@ -646,7 +650,7 @@ protected void runQuery(String sql) {
String queryId = "";
try (AutoCloseConnectContext a =
StatisticsUtil.buildConnectContext(false)) {
a.connectContext.getState().setPlanWithUnKnownColumnStats(true);
- stmtExecutor = new StmtExecutor(a.connectContext, sql);
+ stmtExecutor = new StmtExecutor(a.connectContext, sql,
statementStartTime);
Review Comment:
Passing only the captured `Instant` here does not keep `NOW()` stable for
the whole analysis task. `runQuery()`, `runInsert()`, and `collectMinMax()`
each build a fresh `ConnectContext`, and this `StmtExecutor` overload derives
its `StatementContext` timezone from that new context. If `SET GLOBAL
time_zone` changes between partition batches, the same instant is converted in
two zones and the task persists different `update_time` wall-clock values.
Please capture a canonical `ZoneId` alongside `statementStartTime` in
`execute()` and pass both into every internal statement context, with coverage
for a timezone change between two task SQL executions.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java:
##########
@@ -189,6 +193,10 @@ public void run() throws JobException {
}
mtmvSchemaChangeVersion = mtmv.getSchemaChangeVersion();
ConnectContext ctx = MTMVPlanUtil.createMTMVContext(mtmv,
MTMVPlanUtil.DISABLE_RULES_WHEN_RUN_MTMV_TASK);
+ StatementContext taskStatementContext = new StatementContext(ctx,
null);
+ ctx.setStatementContext(taskStatementContext);
+ statementStartTime = taskStatementContext.getStatementStartTime();
Review Comment:
These captured task values are reused by refresh SQL, but not by
`partition_sync_limit` alignment. Before `exec()`, `alignMvPartition()` and
partition mapping reach
`MTMVRelatedPartitionDescSyncLimitGenerator.getNowTruncSubSec()`, which calls
the live `DateTimeAcquire.now()` again. If setup crosses a DAY/MONTH/YEAR
boundary, alignment can drop/select partitions for the later period while an
opted-in nondeterministic MV's `current_date()`/`now()` still evaluates at the
task-start period. Please thread this captured `Instant` and `ZoneId` through
sync-limit generation and add an injected-clock boundary test covering both
alignment and refresh 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]
