Skip to content

Deduplicate spark_ceil and spark_floor and port floor's decimal boundary tests to ceil #6974

Open

Description

@andygrove

What is the problem the feature request solves?

native/spark-expr/src/math_funcs/ceil.rs and floor.rs are the same module with ceil/floor and div_ceil/div_floor swapped. The spark_* dispatch, decimal_*_f, decimal_*_pow10::<EXP> with its i64 fast path, and the cold *_wide fallback are identical, as are the first seven tests.

The copies have already diverged in their tests. floor.rs has test_floor_decimal128_i64_boundary_array and test_floor_decimal128_large_scale_array, which exercise the i64/i128 switch and the runtime fallback. ceil.rs has neither, even though decimal_ceil_pow10 has the same boundary logic. Any future fix to the fast path has to be made twice, and today only floor's version is tested at the boundary.

Describe the potential solution

  1. Port floor's boundary and large-scale tests to ceil. This is the quick win and could land on its own.
  2. Move the shared decimal rounding into math_funcs/utils.rs, generic over a rounding mode (e.g. a trait with Ceil/Floor marker types providing the i64 and i128 division), and make spark_ceil/spark_floor thin wrappers over one generic function. Parameterise the tests by mode.

Additional context

Line references are pinned to commit 7d8d9b1.

Activity

  1. med9110 commented on Oct 15, 2026

    @med9110

    Hi @andygrove, I'd like to contribute to #6974.

    I noticed that ceil.rs is missing the decimal boundary and large-scale regression tests already present in floor.rs.

    Would you be open to a small, test-only PR that ports these two cases to ceil, with the appropriate expected results?

    I'd leave the larger ceil/floor refactoring for a separate contribution.

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions