Skip to content

feat: Add native support for scalar Math expressions#4028

Open
pchintar wants to merge 1 commit intoapache:mainfrom
pchintar:scalar-expr-support
Open

feat: Add native support for scalar Math expressions#4028
pchintar wants to merge 1 commit intoapache:mainfrom
pchintar:scalar-expr-support

Conversation

@pchintar
Copy link
Copy Markdown

Which issue does this PR close?

Closes #3840 .

Rationale for this change

Several scalar mathematical expressions are currently unsupported in Comet and fall back to Spark JVM execution. This PR adds native support for these expressions to improve coverage while preserving full semantic parity with Spark, including correct handling of nulls and edge cases.

What changes are included in this PR?

Native support is added for the following expressions:

  • pi, e, degrees, radians
  • sec, csc, cbrt, hypot
  • factorial, shiftrightunsigned

Implementation and integration

  • Native implementations

    • Added in:
      native/spark-expr/src/math_funcs/math_expr.rs
  • Function registration

    • Updated:
      native/spark-expr/src/comet_scalar_funcs.rs
  • Module exports

    • Updated:
      native/spark-expr/src/math_funcs/mod.rs
      native/spark-expr/src/lib.rs
  • Spark → Comet expression mapping

    • Updated:
      spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala

These changes ensure the expressions are resolved and executed natively without fallback.

How are these changes tested?

Tests are added/extended in:

spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala

  • Added sec, csc, cbrt, hypot to existing math coverage

  • Added dedicated tests for:

    • factorial
    • shiftrightunsigned (both Int32 and Int64)
    • pi and e
  • degrees and radians validated via existing scalar test patterns

All tests verify:

  • correctness against Spark results
  • native execution (no fallback)

All tests pass successfully.

@pchintar
Copy link
Copy Markdown
Author

pchintar commented Apr 22, 2026

This is weird, The only failing CI test is TestRewriteDataFilesAction.testParallelPartialProgressWithMaxFailedCommitsLargerThanTotalFileGroup() in the Iceberg Spark 3.4 / Scala 2.13 job. I investigated it separately and reproduced that targeted test locally with ENABLE_COMET=true multiple times, and it passed each time. So I have not been able to reproduce the failure outside CI. From the artifacts available to me, I can only see the outer Iceberg partial-progress wrapper exception, not the inner commit-failure cause. Since this failure is outside the scalar expression changes made in this PR, could you please rerun the failed job or share any more other test-report feedback if possible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add native support for Sec, Csc, Cbrt, Hypot, Factorial, and ShiftRightUnsigned

1 participant