Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion datafusion-testing
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this change as a separate PR to datafusion-testing first?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged

13 changes: 13 additions & 0 deletions datafusion/expr/src/logical_plan/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2495,6 +2495,19 @@ pub struct Filter {
}

impl Filter {
/// Create a new filter operator.
///
/// Skips the type-checking and dealiasing done in [Self::try_new].
/// For internal use in DataFusion only.
///
/// **Preconditions:**
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

/// - the `predicate` expression returns a boolean value
/// - the `predicate` expression is not aliased
#[doc(hidden)]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just call this new ? unchecked makes it sound like it should be unsafe, e.g. https://docs.rs/datafusion/latest/datafusion/common/arrow/array/struct.GenericByteArray.html#method.new_unchecked

Though it does have the same properties that the the caller needs to ensure some invariants

Could you please document what the properties are? I think predicate needs to only refer to columns that came from the output schema of input

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I named it new initially, then switched to new_unchecked but I don't have a strong opinion on the name. Makes sense to document the preconditions 👍

pub fn new(predicate: Expr, input: Arc<LogicalPlan>) -> Self {
Self { predicate, input }
}

/// Create a new filter operator.
///
/// Notes: as Aliases have no effect on the output of a filter operator,
Expand Down
Loading
Loading