github-actions[bot] commented on code in PR #64366:
URL: https://github.com/apache/doris/pull/64366#discussion_r3602820532
##########
be/src/service/point_query_executor.cpp:
##########
@@ -308,6 +321,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:
[P1] Serialize access to the cached runtime and expression tree
Every cache hit mutates the single `RuntimeState` and resets the single
`VExpr` tree returned for this UUID, but the block-pool mutex does not protect
either object through `_output_data()`. `tablet_fetch_data` runs each RPC on
the multi-worker light pool, and an FE timeout only cancels its future; the BE
handler does not observe cancellation, so a subsequent execution of the same
prepared UUID can reach this line while the timed-out request is still
evaluating the same contexts. One thread can then reset shared pointers or
rewrite timezone/query-time while the other reads them, which is a C++ data
race and can also mix statement globals. Isolate `RuntimeState`/expression
contexts per execution or hold a per-`Reusable` lease across update through
output; locking only this call is insufficient. Add barrier-based overlap
coverage.
##########
be/src/exprs/vexpr.cpp:
##########
@@ -836,6 +836,13 @@ Status VExpr::get_const_col(VExprContext* context,
return Status::OK();
}
+void VExpr::reset_constant_col() {
+ _constant_col.reset();
Review Comment:
[P1] Refresh function state along with constant columns
This resets only the `VExpr` result pointer. `Reusable::init()` opened this
tree once, and `VExpr::init_function_context()` copied each constant child's
wrapper into `FunctionContext` and initialized thread-local function state from
it. `FunctionLike::open()`, for example, compiles a constant pattern into
`LikeState`, which execution continues using after this reset. With FE folding
disabled, a prepared point query `from_unixtime(20) LIKE from_unixtime(20)`
returns true in `+00:00`; after changing the same session to `+08:00`, the
children rematerialize as `08:00:20` but LIKE still compares against the
compiled `00:00:20` pattern, so the cache hit returns false. This is distinct
from the earlier `_constant_col` thread: that layer is now cleared, but the
`FunctionContext`-owned state remains stale. Rebuild/reopen all
constant-derived function state per request (or isolate the reusable expression
context), and cover a nested constant function.
##########
fe/fe-core/src/main/java/org/apache/doris/planner/GroupCommitPlanner.java:
##########
@@ -227,6 +250,7 @@ public static void executeGroupCommitInsert(ConnectContext
ctx, PreparedStatemen
List<Expr> valueExprs =
statementContext.getIdToPlaceholderRealExpr().values().stream()
.map(v -> ((Literal)
v).toLegacyLiteral()).collect(Collectors.toList());
List<InternalService.PDataRow> rows =
getRows(groupCommitPlanner.targetColumnSize, valueExprs);
+ groupCommitPlanner.refreshQueryGlobals(statementContext);
Review Comment:
[P1] Apply the statement hint before refreshing cached globals
On the first prepared group-commit execution, planner construction runs
normal analysis, so `EliminateLogicalSelectHint` applies
`SET_VAR(time_zone=...)`. On reuse, `ExecuteCommand.run()` first resets
`StatementContext` from the restored base session zone, then
`fastAnalyzeGroupCommit()` skips analysis and reaches this line without calling
`applySetVarHints()` (that helper is used only by the point-query early
return). This therefore serializes the base zone. With a `+08:00` session and a
prepared group-commit insert hinted to `+00:00` that omits a `DEFAULT
CURRENT_TIMESTAMP` column, the first row uses UTC but cache-hit rows use
`+08:00`. Reapply the hints before the cached group-commit fast path and add a
two-execution default-value test.
--
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]
