gortiz commented on code in PR #15277:
URL: https://github.com/apache/pinot/pull/15277#discussion_r2002941298
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java:
##########
@@ -499,40 +498,6 @@ protected void testGeneratedQueries(boolean
withMultiValues, boolean useMultista
}
}
- /**
- * Test invalid queries which should cause query exceptions.
- *
- * @throws Exception
- */
- public void testQueryExceptions()
- throws Exception {
- testQueryException("POTATO", QueryErrorCode.SQL_PARSING);
-
- // Ideally, we should attempt to unify the error codes returned by the two
query engines if possible
- testQueryException("SELECT COUNT(*) FROM potato",
- useMultiStageQueryEngine()
- ? QueryErrorCode.QUERY_PLANNING :
QueryErrorCode.TABLE_DOES_NOT_EXIST);
-
- testQueryException("SELECT POTATO(ArrTime) FROM mytable",
- useMultiStageQueryEngine()
- ? QueryErrorCode.QUERY_PLANNING : QueryErrorCode.QUERY_VALIDATION);
-
- // ArrTime expects a numeric type
- testQueryException("SELECT COUNT(*) FROM mytable where ArrTime = 'potato'",
- useMultiStageQueryEngine()
- ? QueryErrorCode.QUERY_EXECUTION :
QueryErrorCode.QUERY_VALIDATION);
-
- // Cannot use numeric aggregate function for string column
- testQueryException("SELECT MAX(OriginState) FROM mytable where ArrTime >
5",
- QueryErrorCode.QUERY_VALIDATION);
- }
-
- private void testQueryException(String query, QueryErrorCode errorCode)
- throws Exception {
- JsonNode jsonObject = postQuery(query);
- assertEquals(jsonObject.get("exceptions").get(0).get("errorCode").asInt(),
errorCode.getId());
- }
-
Review Comment:
For an unknown reason these tests were only executed in realtime integration
tests. I don't think that makes sense. Instead now these tests are executed in
ErrorCodesIntegrationTests and their subclasses. They are also split into
different tests so we can have better coverage even if one of them fail. They
are also executed against both controllers and brokers. So there are 2
configuration axes (MSE vs SSE and broker vs controller) and therefore 4
classes that actually run these tests. As a result, we need to startup the
cluster 4 extra times. That is something we need to improve in the future (ie
using JUnit 5 it would be trivial to do)
--
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]
