924060929 commented on code in PR #68283:
URL: https://github.com/apache/doris/pull/68283#discussion_r4059466730
##########
fe/fe-core/src/main/java/org/apache/doris/qe/NereidsCoordinator.java:
##########
@@ -154,9 +154,15 @@ private void syncLocalShufflePlannerOption() {
@Override
public void exec() throws Exception {
+ if (isQueryTerminated()) {
+ throw new UserException("Query was terminated before execution");
Review Comment:
Fixed in b6c7b9dfc95. DictionaryInsertExecutor.onFail() no longer assumes
DdlException: it handles arbitrary Throwable, preserves a UserException MySQL
error code when available, and falls back to ERR_UNKNOWN_ERROR otherwise. Added
a test that drives this path with the coordinator-fence UserException and
verifies the original message is reported.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -1449,6 +1453,14 @@ public void cancel(Status cancelReason, boolean
needWaitCancelComplete) {
Review Comment:
Fixed in b6c7b9dfc95. Coordinator publication is now owned by
AbstractInsertExecutor.executeSingleInsert(), the common boundary used by
regular INSERT, row-level DML, RewriteTableCommand, and
ConnectorRewriteGroupTask. INSERT INTO TVF remains a separate path and now
publishes its coordinator immediately after creation, before FE-side file
deletion or fragment dispatch. Added coverage for the common boundary and the
TVF pre-cancel path.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/Coordinator.java:
##########
@@ -767,6 +767,9 @@ private boolean shouldQueue() {
// A call to Exec() must precede all other member function calls.
@Override
public void exec() throws Exception {
+ if (isQueryTerminated()) {
+ throw new UserException("Query was terminated before execution");
Review Comment:
Fixed in b6c7b9dfc95. The execution and dispatch fences now snapshot the
stored Status and throw UserException with its original error message. This
keeps timeout/cancellation details instead of replacing them with a generic
termination string; the coordinator and pipeline tests assert the original
messages.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -1419,6 +1422,7 @@ public void clearCancelDelegate() {
}
Review Comment:
Fixed in b6c7b9dfc95. StmtExecutor.cancel() now snapshots the retained first
terminal status after compareAndSet and always forwards that retained status to
the coordinator. A deterministic latch-based race test verifies that a later
CANCELLED signal cannot overtake an earlier TIMEOUT during coordinator
publication.
--
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]
