You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2021/05/20 02:20:44 UTC
[incubator-pinot] branch master updated: Fix the exception thrown
in the case that a specified table name does not exist (#6328) (#6765)
This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 7b44624 Fix the exception thrown in the case that a specified table name does not exist (#6328) (#6765)
7b44624 is described below
commit 7b44624f2c955ae080439d3ef0c6d6354a1b62b3
Author: malbx <30...@users.noreply.github.com>
AuthorDate: Wed May 19 22:20:15 2021 -0400
Fix the exception thrown in the case that a specified table name does not exist (#6328) (#6765)
## Description
Fix the exception thrown in the case that a specified table name does not exist (#6328). As opposed to returning `BROKER_RESOURCE_MISSING_ERROR`, which can be a bit misleading, the code has been modified to return `TABLE_DOES_NOT_EXIST_ERROR`.
## Release Notes
The error code BROKER_RESOURCE_MISSING_ERROR (410) is no longer returned in the case that a table does not exist (e.g., misspelled table name). In such cases the error code TABLE_DOES_NOT_EXIST_ERROR (190) will now be returned.
---
.../broker/requesthandler/BaseBrokerRequestHandler.java | 12 ++++++++++++
.../org/apache/pinot/common/exception/QueryException.java | 13 +++++++++----
.../pinot/common/response/broker/BrokerResponseNative.java | 2 ++
.../integration/tests/BaseClusterIntegrationTestSet.java | 13 +++++++------
4 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
index bbeff49..256b31c 100644
--- a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
+++ b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
@@ -303,6 +303,12 @@ public abstract class BaseBrokerRequestHandler implements BrokerRequestHandler {
}
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);
@@ -642,6 +648,12 @@ public abstract class BaseBrokerRequestHandler implements BrokerRequestHandler {
}
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);
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java b/pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
index 10a1544..9080d2b 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
@@ -41,6 +41,7 @@ public class QueryException {
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;
@@ -72,6 +73,8 @@ public class QueryException {
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);
@@ -108,6 +111,7 @@ public class QueryException {
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");
@@ -160,16 +164,17 @@ public class QueryException {
*/
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;
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNative.java b/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNative.java
index 8b41228..504896b 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNative.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNative.java
@@ -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;
diff --git a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java
index d899a58..b1e2b95 100644
--- a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java
+++ b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java
@@ -32,6 +32,7 @@ import org.apache.commons.lang3.StringUtils;
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;
@@ -463,16 +464,16 @@ public abstract class BaseClusterIntegrationTestSet extends BaseClusterIntegrati
*/
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);
}
/**
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org