alamb commented on code in PR #7778:
URL: https://github.com/apache/arrow-datafusion/pull/7778#discussion_r1350774878
##########
datafusion/physical-plan/src/insert.rs:
##########
@@ -46,6 +47,23 @@ use datafusion_execution::TaskContext;
/// output.
#[async_trait]
pub trait DataSink: DisplayAs + Debug + Send + Sync {
+ /// Returns the data sink as [`Any`](std::any::Any) so that it can be
+ /// downcast to a specific implementation.
+ fn as_any(&self) -> &dyn Any;
+
+ /// Return a snapshot of the [`MetricsSet`] for this
+ /// [`ExecutionPlan`].
+ ///
+ /// While the values of the metrics in the returned
+ /// [`MetricsSet`] may change as execution progresses, the
+ /// specific metrics will not.
+ ///
+ /// Once `self.execute()` has returned (technically the future is
+ /// resolved) for all available partitions, the set of metrics
+ /// should be complete. If this function is called prior to
+ /// `execute()` new metrics may appear in subsequent calls.
Review Comment:
This appears to be copy/paste from
https://docs.rs/datafusion/latest/datafusion/physical_plan/trait.ExecutionPlan.html#method.metrics
I wonder if we should just leave a doc link (like "Return a snapshot of the
[`MetricsSet`] for this [`DataSink`]. See [`ExecutionPlan::metrics()`] for more
details
##########
datafusion/physical-plan/src/insert.rs:
##########
@@ -46,6 +47,23 @@ use datafusion_execution::TaskContext;
/// output.
#[async_trait]
pub trait DataSink: DisplayAs + Debug + Send + Sync {
+ /// Returns the data sink as [`Any`](std::any::Any) so that it can be
+ /// downcast to a specific implementation.
+ fn as_any(&self) -> &dyn Any;
+
+ /// Return a snapshot of the [`MetricsSet`] for this
+ /// [`ExecutionPlan`].
+ ///
+ /// While the values of the metrics in the returned
+ /// [`MetricsSet`] may change as execution progresses, the
+ /// specific metrics will not.
+ ///
+ /// Once `self.execute()` has returned (technically the future is
+ /// resolved) for all available partitions, the set of metrics
+ /// should be complete. If this function is called prior to
+ /// `execute()` new metrics may appear in subsequent calls.
+ fn metrics(&self) -> Option<MetricsSet>;
Review Comment:
🤔 I wonder if there is value in making some sort of End to End example for
this feature (so we don't accidentally break it in the future0
##########
datafusion/physical-plan/src/insert.rs:
##########
@@ -46,6 +47,23 @@ use datafusion_execution::TaskContext;
/// output.
#[async_trait]
pub trait DataSink: DisplayAs + Debug + Send + Sync {
+ /// Returns the data sink as [`Any`](std::any::Any) so that it can be
+ /// downcast to a specific implementation.
+ fn as_any(&self) -> &dyn Any;
+
+ /// Return a snapshot of the [`MetricsSet`] for this
+ /// [`ExecutionPlan`].
+ ///
+ /// While the values of the metrics in the returned
+ /// [`MetricsSet`] may change as execution progresses, the
+ /// specific metrics will not.
+ ///
+ /// Once `self.execute()` has returned (technically the future is
+ /// resolved) for all available partitions, the set of metrics
+ /// should be complete. If this function is called prior to
+ /// `execute()` new metrics may appear in subsequent calls.
+ fn metrics(&self) -> Option<MetricsSet>;
Review Comment:
🤔 I wonder if there is value in making some sort of End to End example for
this feature (so we don't accidentally break it in the future0
##########
datafusion/physical-plan/src/insert.rs:
##########
@@ -46,6 +47,23 @@ use datafusion_execution::TaskContext;
/// output.
#[async_trait]
pub trait DataSink: DisplayAs + Debug + Send + Sync {
+ /// Returns the data sink as [`Any`](std::any::Any) so that it can be
+ /// downcast to a specific implementation.
+ fn as_any(&self) -> &dyn Any;
+
+ /// Return a snapshot of the [`MetricsSet`] for this
+ /// [`ExecutionPlan`].
Review Comment:
```suggestion
/// [`DataSink`].
```
--
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]
