martin-g commented on code in PR #19893:
URL: https://github.com/apache/datafusion/pull/19893#discussion_r2712159041
##########
datafusion/physical-plan/src/projection.rs:
##########
@@ -498,6 +504,132 @@ impl RecordBatchStream for ProjectionStream {
}
}
+/// Describes an option immutable reference counted shared projection.
+#[derive(Debug, Clone, Default, PartialEq, Eq)]
+pub struct OptionProjectionRef {
+ inner: Option<Arc<[usize]>>,
+}
+
+impl OptionProjectionRef {
+ /// Make a new [`OptionProjectionRef`].
+ pub fn new(inner: Option<impl Into<Arc<[usize]>>>) -> Self {
+ Self {
+ inner: inner.map(Into::into),
+ }
+ }
+
+ /// Project inner.
+ pub fn as_inner(&self) -> &Option<Arc<[usize]>> {
+ &self.inner
+ }
+
+ /// Consume self and return inner.
+ pub fn into_inner(self) -> Option<Arc<[usize]>> {
+ self.inner
+ }
+
+ /// Represent this projection as option slice.
+ pub fn as_ref(&self) -> Option<&[usize]> {
+ self.inner.as_deref()
+ }
+
+ /// Check if the projection is set.
+ pub fn is_some(&self) -> bool {
+ self.inner.is_some()
+ }
+
+ /// Check if the projection is not set.
+ pub fn is_none(&self) -> bool {
+ self.inner.is_none()
+ }
+
+ /// Apply passed `projection` to inner one.
+ ///
+ /// If inner projection is [`None`] then there are no changes.
+ /// Otherwise, if passed `projection` is not [`None`] then it is remapped
+ /// according to the stored one. Otherwise, there are no changes.
+ ///
+ /// # Example
+ ///
+ /// If stored projection is [0, 2] and we call `apply_projection([0, 2,
3])`,
+ /// then the resulting projection will be [0, 3].
Review Comment:
What guarantees that the new projection has enough items ? E.g. if the
stored projection was `[0, 3]` then `new_projection[3]` would fail with out of
bounds error.
https://github.com/apache/datafusion/pull/19893/changes#diff-ae492510820d46b07e2d7420f4555fcb005408682bf3e8aed85faad7925d4545R145
uses `can_project(schema, new_projection)` before trying to apply.
##########
datafusion/physical-plan/src/filter.rs:
##########
@@ -681,7 +675,7 @@ impl ExecutionPlan for FilterExec {
self.default_selectivity,
self.projection.as_ref(),
)?,
- projection: None,
+ projection: None.into(),
Review Comment:
Shouldn't this be:
```suggestion
projection: self.projection.clone(),
```
--
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]
