morrySnow commented on code in PR #64366:
URL: https://github.com/apache/doris/pull/64366#discussion_r3679602103
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java:
##########
@@ -336,6 +336,9 @@ public void executeQuery(String originStmt) throws
Exception {
StatementBase parsedStmt = stmts.get(i);
parsedStmt.setOrigStmt(new OriginStatement(auditStmt,
usingOrigSingleStmt ? 0 : i));
parsedStmt.setUserInfo(ctx.getCurrentUserIdentity());
+ if (parsedStmt instanceof LogicalPlanAdapter) {
+ ((LogicalPlanAdapter)
parsedStmt).getStatementContext().resetStatementStartTime();
Review Comment:
Fixed in 7aaba2bfff0. ConnectProcessor now captures the statement Instant
before SQL-cache validation and keeps that exact instant when the cached result
is accepted, instead of resetting the context afterward. Non-cache executions
still receive a fresh execution-start instant. Added an injected-clock
ConnectProcessorStatementTimeTest; the targeted FE run passed.
##########
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:
Confirmed fixed on the rebased head. NereidsStreamLoadPlanner saves the
original StatementContext, installs the task-scoped context only while
planning, and restores the original context in finally. The collector and
pruner receive the task instant without leaking the temporary context back to
the outer statement. The existing NereidsStreamLoadPlannerTest covers
preservation and restoration. This thread is outdated after the rebase, so I am
resolving it.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/load/NereidsBrokerLoadTask.java:
##########
@@ -25,12 +25,15 @@
import org.apache.doris.thrift.TFileFormatType;
import org.apache.doris.thrift.TFileType;
+import java.time.Instant;
import java.util.List;
/**
* Nereids Broker Load Task
*/
public class NereidsBrokerLoadTask implements NereidsLoadTaskInfo {
+ private final Instant statementStartTime = Instant.now();
Review Comment:
Fixed in 7aaba2bfff0. BrokerLoadJob.createLoadingTask captures one Instant
before iterating FileGroupAggKeys and passes it through both shared-nothing and
cloud loading tasks into every NereidsBrokerLoadTask. BrokerLoadJobTest now
asserts that three sibling planners receive one distinct instant; the targeted
FE run passed.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java:
##########
@@ -336,6 +336,9 @@ public void executeQuery(String originStmt) throws
Exception {
StatementBase parsedStmt = stmts.get(i);
parsedStmt.setOrigStmt(new OriginStatement(auditStmt,
usingOrigSingleStmt ? 0 : i));
parsedStmt.setUserInfo(ctx.getCurrentUserIdentity());
+ if (parsedStmt instanceof LogicalPlanAdapter) {
+ ((LogicalPlanAdapter)
parsedStmt).getStatementContext().resetStatementStartTime();
Review Comment:
Fixed in 7aaba2bfff0. LogicalPlanBuilder.visitMultiStatements now restores
scoped SET_VAR values after each eagerly parsed statement, including
exceptional exits, so a later statement hint cannot affect an earlier statement
execution-time refresh. Added the requested +08:00 and later
SET_VAR(time_zone=+00:00) two-statement test; the targeted FE run passed.
##########
be/src/service/point_query_executor.cpp:
##########
@@ -308,6 +331,8 @@ Status PointQueryExecutor::init(const
PTabletKeyLookupRequest* request,
_tablet = DORIS_TRY(ExecEnv::get_tablet(request->tablet_id()));
if (cache_handle != nullptr) {
_reusable = cache_handle;
+ _reusable_execution_lock = _reusable->acquire_execution_lock();
Review Comment:
Fixed in 7aaba2bfff0. The per-Reusable execution lease is removed. Cache
hits that need request globals now create execution-local RuntimeState and
expression contexts, so overlapping UUID executions do not mutate shared state.
tablet_fetch_data runs on the heavy pool and checks Controller::IsCanceled
before initialization and before lookup. Added barrier-based
independent-context coverage. Per request, BE UT was not run locally;
clang-format and check-format passed.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -227,6 +229,22 @@ public class StmtExecutor {
// this constructor is mainly for proxy
public StmtExecutor(ConnectContext context, OriginStatement originStmt,
boolean isProxy) {
+ this(context, originStmt, isProxy, Instant.now());
Review Comment:
Fixed in 7aaba2bfff0. After raw Nereids parsing, StmtExecutor adopts the
parsed adapter StatementContext and copies the caller-supplied statement
instant and zone into it, keeping the executor and owning ConnectContext on one
context. Added a parse-to-CoordinatorContext.createQueryGlobals test for
now(6), now(kint) with an explicit instant; the targeted FE run passed.
##########
be/src/service/point_query_executor.cpp:
##########
@@ -216,6 +214,28 @@ void Reusable::return_block(std::unique_ptr<Block>& block)
{
}
}
+void Reusable::_update_runtime_state(const PTabletKeyLookupRequest& request) {
+ if (request.has_time_zone() && !request.time_zone().empty()) {
+ _runtime_state->set_timezone(request.time_zone());
+ }
+ if (request.has_timestamp_ms()) {
+ _runtime_state->set_query_time(request.timestamp_ms(),
request.nano_seconds());
+ }
+}
+
+Status Reusable::_rebuild_output_exprs() {
+ RETURN_IF_ERROR(VExpr::create_expr_trees(_output_exprs,
_output_exprs_ctxs));
+ RowDescriptor row_desc(tuple_desc());
+ RETURN_IF_ERROR(VExpr::prepare(_output_exprs_ctxs, _runtime_state.get(),
row_desc));
+ return VExpr::open(_output_exprs_ctxs, _runtime_state.get());
+}
+
+Status Reusable::refresh(const PTabletKeyLookupRequest& request) {
Review Comment:
Fixed in 7aaba2bfff0. Reusable initialization records whether the output
Thrift tree contains function or info nodes. Cache hits rebuild execution-local
contexts only for those runtime-dependent trees; slot-only projections continue
using prepared cached contexts. Added a build-counter guard showing the
representative slot-only path stays at one expression build. Per request, BE UT
was not run locally; clang-format and check-format passed.
--
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]
