You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "gortiz (via GitHub)" <gi...@apache.org> on 2023/07/28 15:33:10 UTC

[GitHub] [pinot] gortiz commented on a diff in pull request #11205: [Draft] Remove noop

gortiz commented on code in PR #11205:
URL: https://github.com/apache/pinot/pull/11205#discussion_r1277704105


##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -211,6 +208,13 @@ public void testSqlWithExceptionMsgChecker(String sql, String exceptionMsg) {
           _reducerScheduler, null, false);
     } catch (RuntimeException rte) {
       Assert.assertTrue(rte.getMessage().contains("Received error query execution result block"));
+      // TODO: The actual message is (usually) something like:

Review Comment:
   I don't know if this is actually what we want to test or not. 
   
   I think there is a race condition and sometimes the message we get is directly the one from the original error block (Illegal Json Path...) and other times we receive a new error block created during the op chain cancellation, which did not contain the message. In order to pass the test I added the original message to that cancellation block, but it may just a hack that makes the test pass but does not implement the logic we actually wanted to 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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org