jackjlli commented on a change in pull request #6942:
URL: https://github.com/apache/incubator-pinot/pull/6942#discussion_r636348514
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##########
@@ -1682,6 +1685,74 @@ static void validateRequest(PinotQuery pinotQuery, int
queryResponseLimit) {
if (!queryOptions.isGroupByModeSQL() ||
!queryOptions.isResponseFormatSQL()) {
throw new IllegalStateException("SQL query should always have response
format and group-by mode set to SQL");
}
+
+ // Validate column names from query
+ Set<String> columnsFromQuery = getColumnsFromQuery(pinotQuery);
+ if (!columnNamesFromSchema.containsAll(columnsFromQuery)) {
+ brokerMetrics.addMeteredTableValue(rawTableName,
BrokerMeter.INVALID_COLUMN_NAMES_IN_QUERY, 1L);
+ }
+ }
+
+ /**
+ * Fetch column names from the SQL query.
+ * @param pinotQuery pinot query
+ */
+ private static Set<String> getColumnsFromQuery(PinotQuery pinotQuery) {
Review comment:
This is a good point. I do have a PR that handles the column name
validation in updateColumnNames
(https://github.com/apache/incubator-pinot/pull/6066), but that doesn't seem
like the correct place to put this logic, since it should only be used to
fix/update the column names. Plus, there are a few places in the same
`handleSQLRequest` method that do the similar operations on broker, and the
overhead is very small comparing to routing the query to the server side.
If we handle the column name validation on broker side, there is no need to
route the query to servers. I've updated this PR in the latest push. Thus,
`DataSchemaSegmentPruner` can be deprecated.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
