shauryachats commented on code in PR #17731:
URL: https://github.com/apache/pinot/pull/17731#discussion_r2843614067
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java:
##########
@@ -430,6 +431,16 @@ protected BrokerResponse doHandleRequest(long requestId,
String query, SqlNodeAn
LogicalTableConfig logicalTableConfig =
_tableCache.getLogicalTableConfig(rawTableName);
String database =
DatabaseUtils.extractDatabaseFromFullyQualifiedTableName(tableName);
long compilationEndTimeNs = System.nanoTime();
+
+ // Validate that physical tables are not queried with multi-cluster
routing enabled
+ try {
+ validatePhysicalTablesWithMultiClusterRouting(Set.of(tableName),
sqlNodeAndOptions.getOptions());
+ } catch (QueryException e) {
Review Comment:
SSE historically has a mixed exception handling strategy - some of them let
the exception propagate and some immediately return the `BrokerResponseNative`
directly. I went with the later because throwing the exception directly does
not convert it to a `BrokerResponseNative` and the caller has to decide how to
handle it unlike MSE, which has a catch on the outermost layer to transform the
exception to a `BrokerResponse`. An additional benefit of returning a
`BrokerResponseNative` is that the relevant error code can be set for each
exception.
--
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]
