alamb commented on code in PR #9926: URL: https://github.com/apache/arrow-datafusion/pull/9926#discussion_r1551793324
########## datafusion/physical-expr-core/README.md: ########## @@ -0,0 +1,27 @@ +<!--- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +--> + +# DataFusion Core Physical Expressions + +[DataFusion][df] is an extensible query execution framework, written in Rust, that uses Apache Arrow as its in-memory format. + +This crate is a submodule of DataFusion that provides the core functionality of physical expressions. +Like `PhysicalExpr` or `PhysicalSortExpr` and related things. Review Comment: Here is a very minor suggestion: ```suggestion This crate is a submodule of DataFusion that provides shared APIs for implementing physical expressions such as `PhysicalExpr` and `PhysicalSortExpr`. ``` ########## datafusion/physical-expr/src/expressions/mod.rs: ########## @@ -80,7 +80,8 @@ pub use crate::PhysicalSortExpr; pub use binary::{binary, BinaryExpr}; pub use case::{case, CaseExpr}; pub use cast::{cast, cast_with_options, CastExpr}; -pub use column::{col, Column, UnKnownColumn}; +pub use column::UnKnownColumn; +pub use datafusion_physical_expr_core::expressions::column::{col, Column}; Review Comment: 👍 ########## datafusion/physical-expr-core/src/physical_expr.rs: ########## @@ -0,0 +1,268 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::any::Any; +use std::fmt::{Debug, Display}; +use std::hash::{Hash, Hasher}; +use std::sync::Arc; + +use arrow::array::BooleanArray; +use arrow::compute::filter_record_batch; +use arrow::datatypes::{DataType, Schema}; +use arrow::record_batch::RecordBatch; +use datafusion_common::utils::DataPtr; +use datafusion_common::{internal_err, not_impl_err, Result}; +use datafusion_expr::interval_arithmetic::Interval; +use datafusion_expr::ColumnarValue; + +use crate::sort_properties::SortProperties; +use crate::utils::scatter; + +/// `PhysicalExpr` evaluate DataFusion expressions such as `A + 1`, or `CAST(c1 +/// AS int)`. +/// +/// `PhysicalExpr` are the physical counterpart to [`Expr`] used in logical +/// planning, and can be evaluated directly on a [`RecordBatch`]. They are +/// normally created from `Expr` by a [`PhysicalPlanner`] and can be created +/// directly using [`create_physical_expr`]. +/// +/// A Physical expression knows its type, nullability and how to evaluate itself. +/// +/// [`Expr`]: datafusion_expr::Expr +/// [`PhysicalPlanner`]: https://docs.rs/datafusion/latest/datafusion/physical_planner/trait.PhysicalPlanner.html +/// [`create_physical_expr`]: https://docs.rs/datafusion/latest/datafusion/physical_expr/fn.create_physical_expr.html +/// +/// # Example: Create `PhysicalExpr` from `Expr` +/// ```ignore Review Comment: I suggest we move the examples to `create_physical_expr` so it continues to run in tests and link to it via html. Something like ``` See [create_physical_expr] for examples of creating `PhysicalExpr` from `Expr`: [create_physical_expr]: https://docs.rs/datafusion/latest/datafusion/physical_expr/fn.create_physical_expr.html ``` ########## datafusion/physical-expr/src/aggregate/mod.rs: ########## @@ -15,16 +15,11 @@ // specific language governing permissions and limitations // under the License. -use std::any::Any; -use std::fmt::Debug; use std::sync::Arc; use crate::expressions::{NthValueAgg, OrderSensitiveArrayAgg}; -use crate::{PhysicalExpr, PhysicalSortExpr}; -use arrow::datatypes::Field; -use datafusion_common::{not_impl_err, Result}; -use datafusion_expr::{Accumulator, GroupsAccumulator}; +pub use datafusion_physical_expr_core::aggregate::AggregateExpr; Review Comment: 💯 for backwards compatibility -- 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]
