github-actions[bot] commented on code in PR #64366:
URL: https://github.com/apache/doris/pull/64366#discussion_r3578532382


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/load/NereidsStreamLoadPlanner.java:
##########
@@ -324,20 +331,41 @@ public TPipelineFragmentParams plan(TUniqueId loadId, int 
fragmentInstanceIdInde
         queryOptions.setNewVersionUnixTimestamp(true);
         queryOptions.setNewVersionPercentile(true);
         params.setQueryOptions(queryOptions);
+        params.setQueryGlobals(createQueryGlobals());

Review Comment:
   These globals are serialized into 
`GroupCommitPlanner.execPlanFragmentParamsBytes` when a prepared group-commit 
insert first builds the stream-load plan, but the prepared path reuses that 
same `GroupCommitPlanner` on later executions when table/schema match. Only the 
row bytes are rebuilt, so later executions keep the first execution's 
timestamp/timezone in the cached fragment. That matters for omitted columns 
with defaults such as `DEFAULT CURRENT_TIMESTAMP`: 
`NereidsLoadPlanInfoCollector` serializes those default expressions into the 
file scan params, and BE evaluates them against the fragment `TQueryGlobals`. A 
prepared group-commit insert that omits the timestamp column can therefore load 
later rows with the first execution's time or timezone. Please refresh/rebuild 
the cached group-commit fragment globals per execution, or invalidate the 
cached planner when the execution statement time/timezone changes.



##########
be/src/service/point_query_executor.cpp:
##########
@@ -345,10 +354,7 @@ Status PointQueryExecutor::init(const 
PTabletKeyLookupRequest* request,
                                                *_tablet->tablet_schema(), 1));
         }
     }
-    // Set timezone from request for functions like from_unixtime()
-    if (request->has_time_zone() && !request->time_zone().empty()) {
-        _reusable->runtime_state()->set_timezone(request->time_zone());
-    }
+    _reusable->update_runtime_state(*request);

Review Comment:
   This still installs the request globals after a cache-miss 
`Reusable::init()` has opened the output expressions. `Reusable::init()` calls 
`VExpr::open(_output_exprs_ctxs, _runtime_state.get())`, and `VExpr::open()` 
materializes all-constant expressions into `_constant_col` at fragment-local 
scope. So a prepared point query such as `SELECT /*+ 
SET_VAR(debug_skip_fold_constant=true,time_zone='+00:00') */ from_unixtime(20) 
FROM t WHERE k=?` can serialize the correct timezone/timestamp in 
`PTabletKeyLookupRequest`, but the cache-miss open already folded 
`from_unixtime(20)` using the default/stale runtime state; later 
`update_runtime_state()` cannot change the cached constant. Please apply the 
request timezone/timestamp before `Reusable::init()` prepares/opens the exprs, 
or pass the globals into `Reusable::init()`, and add coverage with an 
all-constant timezone-sensitive output.



-- 
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]

Reply via email to