Jackie-Jiang commented on pull request #6466: URL: https://github.com/apache/incubator-pinot/pull/6466#issuecomment-777040075
> > Mostly good. > > Another way to handle the server size empty segments is to not load them. In `ImmutableSegmentLoader`, return `null` if the segment is empty. The caller can handle the `null` segment accordingly. > > Currently the query engine won't work on the empty `ImmutableSegmentLoader` if the pruner let it pass because the data source is not set. > > wdyt? > > I found that there were too many callers of ImmutableSegmentLoader#load, and felt it would be error prone if I tried to find them all and handle the null case. If you think that's the better approach, I'm open to doing that. > The case you mention should be handled by the broker side pruner though rt? I checked the code and the `ImmutableSegmentLoader#load` is only called in 3 different classes (regardless of the tests). There are way more callers to `IndexSegment#getDataSource`, so I feel it might be harder to keep it correct if we allow empty `DataSource`. Another small benefit of not loading the empty segment is that we can remove the `ValidSegmentPruner` and reduce the query overhead a little bit. ---------------------------------------------------------------- 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
