Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move create_physical_expr to phy-expr-common #1 #10144

Closed
wants to merge 10 commits into from
Closed
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
1 change: 1 addition & 0 deletions datafusion-cli/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions datafusion/physical-expr-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,4 @@ path = "src/lib.rs"
arrow = { workspace = true }
datafusion-common = { workspace = true, default-features = true }
datafusion-expr = { workspace = true }
paste = "^1.0"
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ use std::{any::Any, sync::Arc};
use crate::expressions::datum::{apply, apply_cmp};
Copy link
Contributor

Choose a reason for hiding this comment

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

My hope / dream for the binary operators are that over time we would be able to extract them into functions (e.g so we would have a + b --> add(a, b)

However, I can't think if any good justification for this yet and I think we have bigger things to focus on than the semantics of operators

use crate::intervals::cp_solver::{propagate_arithmetic, propagate_comparison};
use crate::physical_expr::down_cast_any_ref;
use crate::physical_expr::PhysicalExpr;
use crate::sort_properties::SortProperties;
use crate::PhysicalExpr;

use arrow::array::*;
use arrow::compute::kernels::boolean::{and_kleene, not, or_kleene};
Expand Down Expand Up @@ -622,8 +622,13 @@ pub fn binary(
#[cfg(test)]
mod tests {
use super::*;
use crate::expressions::{col, lit, try_cast, Literal};
use datafusion_common::plan_datafusion_err;
use crate::expressions::column::col;
use crate::expressions::literal::{lit, Literal};
use crate::expressions::try_cast::try_cast;
use arrow::datatypes::{
ArrowNumericType, Decimal128Type, Field, Int32Type, SchemaRef,
};
use datafusion_common::{plan_datafusion_err, Result};
use datafusion_expr::type_coercion::binary::get_input_types;

/// Performs a binary operation, applying any type coercion necessary
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@
// specific language governing permissions and limitations
// under the License.

use arrow::array::BooleanArray;
use arrow::array::{ArrayRef, Datum};
use arrow::error::ArrowError;
use arrow_array::BooleanArray;
use datafusion_common::{Result, ScalarValue};
use datafusion_expr::ColumnarValue;
use std::sync::Arc;

/// Applies a binary [`Datum`] kernel `f` to `lhs` and `rhs`
///
/// This maps arrow-rs' [`Datum`] kernels to DataFusion's [`ColumnarValue`] abstraction
pub(crate) fn apply(
pub fn apply(
lhs: &ColumnarValue,
rhs: &ColumnarValue,
f: impl Fn(&dyn Datum, &dyn Datum) -> Result<ArrayRef, ArrowError>,
Expand All @@ -49,7 +49,7 @@ pub(crate) fn apply(
}

/// Applies a binary [`Datum`] comparison kernel `f` to `lhs` and `rhs`
pub(crate) fn apply_cmp(
pub fn apply_cmp(
lhs: &ColumnarValue,
rhs: &ColumnarValue,
f: impl Fn(&dyn Datum, &dyn Datum) -> Result<BooleanArray, ArrowError>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ use std::hash::{Hash, Hasher};
use std::sync::Arc;

use crate::physical_expr::down_cast_any_ref;
use crate::physical_expr::PhysicalExpr;
use crate::sort_properties::SortProperties;
use crate::PhysicalExpr;

use arrow::{
datatypes::{DataType, Schema},
Expand Down
7 changes: 7 additions & 0 deletions datafusion/physical-expr-common/src/expressions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,11 @@
// specific language governing permissions and limitations
// under the License.

//! Defines physical expressions that can evaluated at runtime during query execution

#[macro_use]
pub mod binary;
pub mod column;
pub mod datum;
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree it makes sense to have the datum / scalars / cast stuff in physical expr common

pub mod literal;
pub mod try_cast;
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use std::hash::{Hash, Hasher};
use std::sync::Arc;

use crate::physical_expr::down_cast_any_ref;
use crate::PhysicalExpr;
use crate::physical_expr::PhysicalExpr;
use arrow::compute;
use arrow::compute::{cast_with_options, CastOptions};
use arrow::datatypes::{DataType, Schema};
Expand Down Expand Up @@ -148,7 +148,7 @@ pub fn try_cast(
#[cfg(test)]
mod tests {
use super::*;
use crate::expressions::col;
use crate::expressions::column::col;
use arrow::array::{
Decimal128Array, Decimal128Builder, StringArray, Time64NanosecondArray,
};
Expand Down
Loading