zstan commented on code in PR #3264:
URL: https://github.com/apache/ignite-3/pull/3264#discussion_r1502902599
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionContext.java:
##########
@@ -310,6 +311,16 @@ public void execute(RunnableX task, Consumer<Throwable>
onError) {
} catch (Throwable e) {
onError.accept(e);
+ if (e instanceof IgniteException) {
+ return;
+ }
+
+ if (e instanceof AssertionError) {
Review Comment:
still not clear to me ((( if we want to defense here from some calcite
assertions probably we can implement it somehow more accurate ? What test will
fail if i comment this part ? i can`t find it quick ((
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/rel/ExecutionTest.java:
##########
@@ -621,25 +621,25 @@ static class CorruptedNode<T> implements Node<T> {
/** {@inheritDoc} */
@Override
public ExecutionContext<T> context() {
- throw new AssertionError();
+ throw new IllegalAccessError();
Review Comment:
seems you also need to fix class javadoc
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExchangeServiceImpl.java:
##########
@@ -170,6 +168,10 @@ private void onMessage(String nodeName, QueryBatchMessage
msg) {
} catch (Throwable e) {
inbox.onError(e);
+ if (e instanceof IgniteException) {
Review Comment:
why do we need to play such games ? Exceptions is global system machinery
thus such code will bring more harm for further debugging if someone somewhere
will decide to change one exception into another ? can you point me test which
fail if i comment this part plz ?
--
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]
