timsaucer commented on code in PR #22157:
URL: https://github.com/apache/datafusion/pull/22157#discussion_r3243790822
##########
datafusion/ffi/src/execution_plan.rs:
##########
@@ -74,6 +75,15 @@ pub struct FFI_ExecutionPlan {
/// underlying [`ExecutionPlan::metrics`] returned `None`.
pub metrics: unsafe extern "C" fn(plan: &Self) ->
FFI_Option<FFI_MetricsSet>,
+ /// Snapshot partition statistics. `partition == None` corresponds to
+ /// statistics over all partitions; `Some(idx)` corresponds to a specific
+ /// partition. The returned bytes are a prost-encoded
+ /// `datafusion_proto_common::Statistics`.
+ pub partition_statistics: unsafe extern "C" fn(
+ plan: &Self,
+ partition: FFI_Option<u64>,
Review Comment:
why FFI_Option<u64> instead of FFI_Option<usize>?
##########
datafusion/ffi/src/execution_plan.rs:
##########
@@ -454,10 +476,24 @@ impl ExecutionPlan for ForeignExecutionPlan {
unsafe { (self.plan.metrics)(&self.plan) }.into();
ffi.map(MetricsSet::from)
}
+
+ fn partition_statistics(&self, partition: Option<usize>) ->
Result<Arc<Statistics>> {
+ let bytes = df_result!(unsafe {
+ (self.plan.partition_statistics)(
+ &self.plan,
+ partition.map(|p| p as u64).into(),
+ )
+ })?;
+ Ok(Arc::new(deserialize_statistics(bytes.as_slice())?))
+ }
}
#[cfg(any(test, feature = "integration-tests"))]
pub mod tests {
+ #[cfg(test)]
+ use datafusion_common::stats::Precision;
+ #[cfg(test)]
+ use datafusion_common::{ColumnStatistics, ScalarValue};
Review Comment:
If this is here because you get clippy warnings otherwise, then it's
marginally cleaner to remove the `use` statements and just fully qualify these
below.
##########
datafusion/ffi/src/table_provider.rs:
##########
@@ -179,6 +188,16 @@ unsafe extern "C" fn schema_fn_wrapper(provider:
&FFI_TableProvider) -> WrappedS
provider.inner().schema().into()
}
+unsafe extern "C" fn statistics_fn_wrapper(
+ provider: &FFI_TableProvider,
+) -> FFI_Result<FFI_Option<SVec<u8>>> {
+ let serialized: Option<SVec<u8>> = provider
+ .inner()
+ .statistics()
+ .map(|s| serialize_statistics(&s).into_iter().collect());
+ FFI_Result::Ok(serialized.into())
Review Comment:
Why return a result? It looks infallible unless I missed something
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
