Skip to content

Commit

Permalink
Fix the exception thrown in the case that a specified table name does…
Browse files Browse the repository at this point in the history
… not exist (apache#6328)
  • Loading branch information
malbx committed May 4, 2021
1 parent 1c09b78 commit d73f838
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,12 @@ private BrokerResponseNative handleSQLRequest(long requestId, String query, Json
}
if ((offlineTableName == null) && (realtimeTableName == null)) {
// No table matches the request
if (_tableCache.getTableConfig(TableNameBuilder.REALTIME.tableNameWithType(rawTableName)) == null
&& _tableCache.getTableConfig(TableNameBuilder.OFFLINE.tableNameWithType(rawTableName)) == null) {
LOGGER.info("Table not found for request {}: {}", requestId, query);
requestStatistics.setErrorCode(QueryException.TABLE_DOES_NOT_EXIST_ERROR_CODE);
return BrokerResponseNative.TABLE_DOES_NOT_EXIST;
}
LOGGER.info("No table matches for request {}: {}", requestId, query);
requestStatistics.setErrorCode(QueryException.BROKER_RESOURCE_MISSING_ERROR_CODE);
_brokerMetrics.addMeteredGlobalValue(BrokerMeter.RESOURCE_MISSING_EXCEPTIONS, 1);
Expand Down Expand Up @@ -643,6 +649,12 @@ private BrokerResponseNative handlePQLRequest(long requestId, String query, Json
}
if ((offlineTableName == null) && (realtimeTableName == null)) {
// No table matches the request
if (_tableCache.getTableConfig(TableNameBuilder.REALTIME.tableNameWithType(rawTableName)) == null
&& _tableCache.getTableConfig(TableNameBuilder.OFFLINE.tableNameWithType(rawTableName)) == null) {
LOGGER.info("Table not found for request {}: {}", requestId, query);
requestStatistics.setErrorCode(QueryException.TABLE_DOES_NOT_EXIST_ERROR_CODE);
return BrokerResponseNative.TABLE_DOES_NOT_EXIST;
}
LOGGER.info("No table matches for request {}: {}", requestId, query);
requestStatistics.setErrorCode(QueryException.BROKER_RESOURCE_MISSING_ERROR_CODE);
_brokerMetrics.addMeteredGlobalValue(BrokerMeter.RESOURCE_MISSING_EXCEPTIONS, 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public static void setMaxLinesOfStackTrace(int maxLinesOfStackTrace) {
public static final int SEGMENT_PLAN_EXECUTION_ERROR_CODE = 160;
public static final int COMBINE_SEGMENT_PLAN_TIMEOUT_ERROR_CODE = 170;
public static final int ACCESS_DENIED_ERROR_CODE = 180;
public static final int TABLE_DOES_NOT_EXIST_ERROR_CODE = 190;
public static final int QUERY_EXECUTION_ERROR_CODE = 200;
// TODO: Handle these errors in broker
public static final int SERVER_SHUTTING_DOWN_ERROR_CODE = 210;
Expand Down Expand Up @@ -72,6 +73,8 @@ public static void setMaxLinesOfStackTrace(int maxLinesOfStackTrace) {
new ProcessingException(SEGMENT_PLAN_EXECUTION_ERROR_CODE);
public static final ProcessingException COMBINE_SEGMENT_PLAN_TIMEOUT_ERROR =
new ProcessingException(COMBINE_SEGMENT_PLAN_TIMEOUT_ERROR_CODE);
public static final ProcessingException TABLE_DOES_NOT_EXIST_ERROR =
new ProcessingException(TABLE_DOES_NOT_EXIST_ERROR_CODE);
public static final ProcessingException QUERY_EXECUTION_ERROR = new ProcessingException(QUERY_EXECUTION_ERROR_CODE);
public static final ProcessingException SERVER_SCHEDULER_DOWN_ERROR =
new ProcessingException(SERVER_SHUTTING_DOWN_ERROR_CODE);
Expand Down Expand Up @@ -108,6 +111,7 @@ public static void setMaxLinesOfStackTrace(int maxLinesOfStackTrace) {
PQL_PARSING_ERROR.setMessage("PQLParsingError");
SEGMENT_PLAN_EXECUTION_ERROR.setMessage("SegmentPlanExecutionError");
COMBINE_SEGMENT_PLAN_TIMEOUT_ERROR.setMessage("CombineSegmentPlanTimeoutError");
TABLE_DOES_NOT_EXIST_ERROR.setMessage("TableDoesNotExistError");
QUERY_EXECUTION_ERROR.setMessage("QueryExecutionError");
SERVER_SCHEDULER_DOWN_ERROR.setMessage("ServerShuttingDown");
SERVER_OUT_OF_CAPACITY_ERROR.setMessage("ServerOutOfCapacity");
Expand Down Expand Up @@ -160,16 +164,17 @@ public static String getTruncatedStackTrace(Exception exception) {
*/
public static boolean isClientError(int errorCode) {
switch (errorCode) {
// NOTE: QueryException.BROKER_RESOURCE_MISSING_ERROR can be triggered either due to
// client error (incorrect table name) or due to issues with EV updates. For cases where
// access to tables is controlled via ACLs, for an incorrect table name we expect ACCESS_DENIED_ERROR to be
// thrown. Hence, we currently don't treat BROKER_RESOURCE_MISSING_ERROR as client error.
// NOTE: QueryException.BROKER_RESOURCE_MISSING_ERROR can be triggered due to issues
// with EV updates. For cases where access to tables is controlled via ACLs, for an
// incorrect table name we expect ACCESS_DENIED_ERROR to be thrown. Hence, we currently
// don't treat BROKER_RESOURCE_MISSING_ERROR as client error.
case QueryException.ACCESS_DENIED_ERROR_CODE:
case QueryException.JSON_COMPILATION_ERROR_CODE:
case QueryException.JSON_PARSING_ERROR_CODE:
case QueryException.QUERY_VALIDATION_ERROR_CODE:
case QueryException.PQL_PARSING_ERROR_CODE:
case QueryException.TOO_MANY_REQUESTS_ERROR_CODE:
case QueryException.TABLE_DOES_NOT_EXIST_ERROR_CODE:
return true;
default:
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public class BrokerResponseNative implements BrokerResponse {
public static final BrokerResponseNative EMPTY_RESULT = BrokerResponseNative.empty();
public static final BrokerResponseNative NO_TABLE_RESULT =
new BrokerResponseNative(QueryException.BROKER_RESOURCE_MISSING_ERROR);
public static final BrokerResponseNative TABLE_DOES_NOT_EXIST =
new BrokerResponseNative(QueryException.TABLE_DOES_NOT_EXIST_ERROR);

private int _numServersQueried = 0;
private int _numServersResponded = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.apache.helix.model.InstanceConfig;
import org.apache.pinot.client.ResultSet;
import org.apache.pinot.client.ResultSetGroup;
import org.apache.pinot.common.exception.QueryException;
import org.apache.pinot.core.query.utils.idset.IdSet;
import org.apache.pinot.core.query.utils.idset.IdSets;
import org.apache.pinot.spi.data.DimensionFieldSpec;
Expand Down Expand Up @@ -463,16 +464,16 @@ private void testGeneratedQueries(boolean withMultiValues)
*/
public void testQueryExceptions()
throws Exception {
testQueryException("POTATO");
testQueryException("SELECT COUNT(*) FROM potato");
testQueryException("SELECT POTATO(ArrTime) FROM mytable");
testQueryException("SELECT COUNT(*) FROM mytable where ArrTime = 'potato'");
testQueryException("POTATO", QueryException.PQL_PARSING_ERROR_CODE);
testQueryException("SELECT COUNT(*) FROM potato", QueryException.TABLE_DOES_NOT_EXIST_ERROR_CODE);
testQueryException("SELECT POTATO(ArrTime) FROM mytable", QueryException.QUERY_EXECUTION_ERROR_CODE);
testQueryException("SELECT COUNT(*) FROM mytable where ArrTime = 'potato'", QueryException.QUERY_EXECUTION_ERROR_CODE);
}

private void testQueryException(String query)
private void testQueryException(String query, int errorCode)
throws Exception {
JsonNode jsonObject = postQuery(query);
assertTrue(jsonObject.get("exceptions").size() > 0);
assertEquals(jsonObject.get("exceptions").get(0).get("errorCode").asInt(), errorCode);
}

/**
Expand Down

0 comments on commit d73f838

Please sign in to comment.