adriangb commented on code in PR #16433: URL: https://github.com/apache/datafusion/pull/16433#discussion_r2286456332
########## datafusion/physical-plan/src/topk/mod.rs: ########## @@ -121,13 +122,37 @@ pub struct TopK { /// Common sort prefix between the input and the sort expressions to allow early exit optimization common_sort_prefix: Arc<[PhysicalSortExpr]>, /// Filter matching the state of the `TopK` heap used for dynamic filter pushdown - filter: Option<Arc<DynamicFilterPhysicalExpr>>, + filter: TopKDynamicFilters, /// If true, indicates that all rows of subsequent batches are guaranteed /// to be greater (by byte order, after row conversion) than the top K, /// which means the top K won't change and the computation can be finished early. pub(crate) finished: bool, } +#[derive(Debug, Clone)] +pub struct TopKDynamicFilters { Review Comment: Yes because it gets passed into `TopK::try_new` which is currently public. Maybe `TopK` should be `pub(crate)` then we could make the whole thing `pub(crate)` but as it stands that's a breaking change. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org