feat: add column_struct_operation.rs
#333
Conversation
iajoiner
force-pushed
the
feat/column-ops
branch
6 times, most recently
from
9dd1011
to
c115121
Compare
iajoiner
force-pushed
the
feat/column-ops
branch
from
c115121
to
e002ee2
Compare
|
|
||
| /// Element-wise equality check for two columns | ||
| #[allow(clippy::too_many_lines)] | ||
| pub fn element_wise_eq(&self, rhs: &Self, alloc: &'a Bump) -> ColumnOperationResult<Self> { |
There was a problem hiding this comment.
Since all of these functions are identical (I think), could we just replace them all with a single function that we can pass in a few slice_op parameters?
| pub fn element_wise_eq(&self, rhs: &Self, alloc: &'a Bump) -> ColumnOperationResult<Self> { | |
| pub fn element_wise_binary_compare_op<F,G,H>(&self, rhs: &Self, alloc: &'a Bump, slice_op: F, slice_op_with_casting: G, decimal_slice_op: H) -> ColumnOperationResult<Self> { |
Not sure if the generic parameters are needed.
| } | ||
| match (self, rhs) { | ||
| (Self::TinyInt(lhs), Self::TinyInt(rhs)) => Ok(Self::Boolean( | ||
| alloc.alloc_slice_copy(slice_eq(lhs, rhs).as_ref()), |
There was a problem hiding this comment.
Could we change slice_eq to be the following?
pub(super) fn slice_eq<'a, T>(lhs: &'a [T], rhs: &'a [T]) -> impl Iterator<Item = bool> + 'a
That way, we can do this:
| alloc.alloc_slice_copy(slice_eq(lhs, rhs).as_ref()), | |
| alloc.alloc_slice_fill_iter(slice_eq(lhs, rhs)), |
Alternatively, add a slice_eq_iter that is called from slice_eq to avoid refactoring existing code.
|
|
||
| #[allow(clippy::too_many_lines)] | ||
| /// Element-wise + for two columns | ||
| pub fn element_wise_add(&self, rhs: &Self, alloc: &'a Bump) -> ColumnOperationResult<Self> { |
There was a problem hiding this comment.
Another generic function could cover all of these.
I'm not sure if there's a clever way to combine these with the comparison ones, but maybe there is.
| } | ||
| match (&self, &rhs) { | ||
| (Self::TinyInt(lhs), Self::TinyInt(rhs)) => Ok(Self::TinyInt( | ||
| alloc.alloc_slice_copy(try_add_slices(lhs, rhs)?.as_ref()), |
There was a problem hiding this comment.
Same comment here as above. I don't like unnecessary copy. This is how it could look using the process_results method from itertools:
| alloc.alloc_slice_copy(try_add_slices(lhs, rhs)?.as_ref()), | |
| try_add_slices(lhs, rhs).process_results(|iter| alloc.alloc_slice_copy(iter)), |
Please be sure to look over the pull request guidelines here: https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/CONTRIBUTING.md#submit-pr.
Please go through the following checklist
!is used if and only if at least one breaking change has been introduced.source scripts/run_ci_checks.sh.Rationale for this change
The equivalent of
owned_column_operation.rsforColumns.What changes are included in this PR?
Are these changes tested?