jackjlli commented on a change in pull request #6361:
URL: https://github.com/apache/incubator-pinot/pull/6361#discussion_r546005399



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java
##########
@@ -259,6 +260,18 @@ private DataTable processQuery(List<IndexSegment> 
indexSegments, QueryContext qu
       ExecutorService executorService, @Nullable 
StreamObserver<Server.ServerResponse> responseObserver, long endTimeMs,
       boolean enableStreaming)
       throws Exception {
+
+    // Validate whether column names in the query are valid
+    Set<String> columnNamesFromSchema = 
_instanceDataManager.getColumnNamesByTable(queryContext.getTableName());
+    Set<String> columnNamesFromQuery = queryContext.getColumns();
+    if (!columnNamesFromSchema.isEmpty() && 
!columnNamesFromSchema.containsAll(columnNamesFromQuery)) {
+      columnNamesFromQuery.removeAll(columnNamesFromSchema);

Review comment:
       The `containsAll` method has already been used in 
`DataSchemaSegmentPruner` class as well. And it gets called for every selected 
segment. And here it is just done once. 
   The method `removeAll` here is invoked only when the if statement is 
satisfied, so no extra execution on querying actual segments. 
   




----------------------------------------------------------------
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

Reply via email to