Skip to content

fix: re-enable Comet abs #2595

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.

Sign up for GitHub

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

Closed
wants to merge 8 commits into from
Closed

fix: re-enable Comet abs #2595

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
58d8c9a
Removed old ABS implementation
hsiang-c Oct 16, 2025
d5b70c3
Define Comet's ABS in Scala
hsiang-c Oct 16, 2025
7b1965a
Implement Comet's ABS in rust
hsiang-c Oct 16, 2025
eb94cb6
Enable ABS tests in legacy/ANSI mode
hsiang-c Oct 16, 2025
ce1c1c2
Fix bit position b/c schema change in CometTestBase
hsiang-c Oct 16, 2025
9cb2f6c
Update docs
hsiang-c Oct 16, 2025
6016f69
Fix style
hsiang-c Oct 17, 2025
1ddff98
Update doc
hsiang-c Oct 17, 2025
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 docs/source/user-guide/latest/configs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ These settings can be used to determine which parts of the plan are accelerated
<!--BEGIN:CONFIG_TABLE[enable_expr]-->
| Config | Description | Default Value |
|--------|-------------|---------------|
| `spark.comet.expression.Abs.enabled` | Enable Comet acceleration for `Abs` | true |
| `spark.comet.expression.Acos.enabled` | Enable Comet acceleration for `Acos` | true |
| `spark.comet.expression.Add.enabled` | Enable Comet acceleration for `Add` | true |
| `spark.comet.expression.Alias.enabled` | Enable Comet acceleration for `Alias` | true |
Expand Down
13 changes: 0 additions & 13 deletions native/core/src/execution/planner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -675,19 +675,6 @@ impl PhysicalPlanner {
let op = DataFusionOperator::BitwiseShiftLeft;
Ok(Arc::new(BinaryExpr::new(left, op, right)))
}
// https://github.com/apache/datafusion-comet/issues/666
// ExprStruct::Abs(expr) => {
// let child = self.create_expr(expr.child.as_ref().unwrap(), Arc::clone(&input_schema))?;
// let return_type = child.data_type(&input_schema)?;
// let args = vec![child];
// let eval_mode = from_protobuf_eval_mode(expr.eval_mode)?;
// let comet_abs = Arc::new(ScalarUDF::new_from_impl(Abs::new(
// eval_mode,
// return_type.to_string(),
// )?));
// let expr = ScalarFunctionExpr::new("abs", comet_abs, args, return_type);
// Ok(Arc::new(expr))
// }
ExprStruct::CaseWhen(case_when) => {
let when_then_pairs = case_when
.when
Expand Down
6 changes: 0 additions & 6 deletions native/proto/src/proto/expr.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ message Expr {
IfExpr if = 44;
NormalizeNaNAndZero normalize_nan_and_zero = 45;
TruncTimestamp truncTimestamp = 47;
Abs abs = 49;
Subquery subquery = 50;
UnboundReference unbound = 51;
BloomFilterMightContain bloom_filter_might_contain = 52;
Expand Down Expand Up @@ -351,11 +350,6 @@ message TruncTimestamp {
string timezone = 3;
}

message Abs {
Expr child = 1;
EvalMode eval_mode = 2;
}

message Subquery {
int64 id = 1;
DataType datatype = 2;
Expand Down
5 changes: 5 additions & 0 deletions native/spark-expr/src/comet_scalar_funcs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// under the License.

use crate::hash_funcs::*;
use crate::math_funcs::abs::abs;
use crate::math_funcs::checked_arithmetic::{checked_add, checked_div, checked_mul, checked_sub};
use crate::math_funcs::modulo_expr::spark_modulo;
use crate::{
Expand Down Expand Up @@ -180,6 +181,10 @@ pub fn create_comet_physical_fun_with_eval_mode(
let func = Arc::new(spark_modulo);
make_comet_scalar_udf!("spark_modulo", func, without data_type, fail_on_error)
}
"abs" => {
let func = Arc::new(abs);
make_comet_scalar_udf!("abs", func, without data_type)
}
_ => registry.udf(fun_name).map_err(|e| {
DataFusionError::Execution(format!(
"Function {fun_name} not found in the registry: {e}",
Expand Down
Loading