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



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

Review comment:
       We've already done the similar check in `DataSchemaSegmentPruner` class 
and the as I mentioned `containsAll` method is called for every indexed 
segment. 
   
   If the query does include invalid column names, those checks in 
`DataSchemaSegmentPruner` would be skipped. 
   If the query doesn't include any invalid columns, the effect here is like to 
add one more pruning check on one extra segment.
   
   I feel that `DataSchemaSegmentPruner` is still needed, in case if there is 
some inconsistence between the new and old segments and the table doesn't get 
reloaded.




----------------------------------------------------------------
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to