[FEA] Implement decimal casts for AST Interpreter by lamarrr · Pull Request #23982 · NVIDIA/cudf

lamarrr · 2026-09-04T03:06:14Z

Description

AST expressions currently expose only int64, uint64 and float64 casts. This PR adds casts to bool, the remaining integer widths, float32, and decimal32/64/128, plus a RESCALE operation with an explicit target scale. Expressions can perform these conversions within AST evaluation and JIT transforms without materializing a separate cast column.

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.

copy-pr-bot · 2026-09-04T03:06:17Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

Keep upstream checked-arithmetic fixes and the updated JIT transform calling convention while retaining AST cast and rescale support. Preserve target scale metadata when normalizing Parquet expressions.

coderabbitai · 2026-10-21T23:05:25Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: NVIDIA/cudf/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 53f97aa3-d54d-496e-82f9-90291ecbd8d4

📥 Commits

Reviewing files that changed from the base of the PR and between f6225c4 and 3ad5653.

📒 Files selected for processing: 1

cpp/include/cudf/ast/detail/operators.cuh

🚧 Files skipped from review as they are similar to previous changes: 1

cpp/include/cudf/ast/detail/operators.cuh

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

📝 Summary

Summary by CodeRabbit

New Features
- Added AST support for casting to boolean, narrower integer, unsigned integer, float32, and decimal types.
- Added scale-aware rescaling for fixed-point and decimal values, including null preservation.
- Extended expression processing across Parquet filters and Row IR to retain target-scale information.
Bug Fixes
- Added validation for missing or invalid target scales.
- Improved decimal casting and rescaling behavior across supported execution paths.

The AST adds boolean, integer, floating-point, and decimal cast operators, plus scale-aware RESCALE. Expression parsing stores target scales in device data, and unary evaluation forwards them to rescaling. Expression transforms and Row IR conversion preserve scale metadata.

Changes: AST casts and fixed-point rescaling

Layer / File(s)	Summary
Operator and operation contracts `cpp/include/cudf/ast/ast_operator.hpp`, `cpp/include/cudf/ast/expressions.hpp`, `cpp/src/ast/expressions.cpp`, `cpp/src/ast/operators.cpp`	The AST adds cast operators and `RESCALE`. `operation` stores an optional target scale, and return-type handling validates RESCALE operands.
Parse and expose target-scale metadata `cpp/include/cudf/ast/detail/expression_parser.hpp`, `cpp/src/ast/expression_parser.cpp`	The parser requires target-scale metadata for RESCALE, applies that scale to its result type, and includes per-operator scales in device expression data.
Dispatch casts and evaluate RESCALE `cpp/include/cudf/ast/detail/operator_functor.cuh`, `cpp/include/cudf/ast/detail/operators.cuh`, `cpp/include/cudf/ast/detail/expression_evaluator.cuh`	Operator mappings and dispatch cover the added casts and RESCALE. Unary evaluation passes the target scale to RESCALE and retains single-argument invocation for other unary operators.
Propagate scale metadata and test operators `cpp/src/io/parquet/expression_transform_helpers.cpp`, `cpp/src/jit/row_ir.cpp`, `cpp/tests/ast/transform_tests.cpp`	Parquet expression transforms and Row IR conversion preserve target-scale metadata. Tests cover cast parity, decimal rescaling, null preservation, and scale-argument validation.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Possibly related issues

NVIDIA/cudf#22507: The issue requests decimal AST casts with a target scale; this change adds decimal casts and explicit RESCALE support.

Merge Risk: ⚪ Minimal · up to 3ad56

No actionable issue remains from this review; the change is mergeable after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 9 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description check		The description accurately summarizes the added AST casts, RESCALE operation, evaluation support, tests, and documentation updates.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly describes a major part of the changes: adding decimal casts to the AST interpreter. It does not mention the additional casts or RESCALE support, but the title remains relevant and sp…

Full details: Title check

Explanation

The title clearly describes a major part of the changes: adding decimal casts to the AST interpreter. It does not mention the additional casts or RESCALE support, but the title remains relevant and specific.

Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 9 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

mhaseeb123 · 2026-10-27T01:52:30Z

+  template <typename T>
+  __device__ static inline T eval(T a)
+    requires(cudf::is_fixed_point<T>())
+  {
+    return a;
+  }
+


This eval overload is a silent no-op for is_valid_unary_op. Let's drop it and update the unary dispatch in expression_evaluator.cuh (L727/L758) on the two-arg form for RESCALE instead, e.g.:

template <ast_operator op, typename T> constexpr bool is_valid_ast_unary_op = op == ast_operator::RESCALE ? requires(operator_functor<op> f, T v) { f(v, int32_t{}); } : is_valid_unary_op<operator_functor<op>, T>;

Suggested change

template <typename T>

__device__ static inline T eval(T a)

requires(cudf::is_fixed_point<T>())

{

return a;

}

mhaseeb123 · 2026-10-27T01:52:31Z

-      auto const op    = plan.operators[operator_index];
-      auto const arity = plan.operator_arities[operator_index];
+      auto const op           = plan.operators[operator_index];
+      auto const target_scale = plan.operator_target_scales[operator_index];


AI's nit: plan is in global memory so this adds a load per operator per row, though only RESCALE needs it.

Suggested change

auto const target_scale = plan.operator_target_scales[operator_index];

auto const target_scale =

op == ast_operator::RESCALE ? plan.operator_target_scales[operator_index] : 0;

mhaseeb123 · 2026-10-27T01:52:34Z

  });
  // Resolve expression type
-  auto const data_type = cudf::ast::detail::ast_operator_return_type(op, operand_types);
+  auto data_type = cudf::ast::detail::ast_operator_return_type(op, operand_types);


AI pointed out that CAST_TO_DECIMAL128 is unusable here as it throws at root (L275). As an intermediate with nulls, it passes the size check at L286, then resolve_output memcpys 16 bytes into an int64_t and resolve_input truncates the rep. Let’s reject it upfront, and ideally L286 should compare against

Suggested change

auto data_type = cudf::ast::detail::ast_operator_return_type(op, operand_types);

CUDF_EXPECTS(op != ast_operator::CAST_TO_DECIMAL128,

"CAST_TO_DECIMAL128 is not supported by the AST interpreter.",

cudf::data_type_error);

auto data_type = cudf::ast::detail::ast_operator_return_type(op, operand_types);

mhaseeb123 · 2026-10-27T01:52:35Z

+    CUDF_EXPECTS(operand_types.size() == 1 && cudf::is_fixed_point(operand_types.front()),
+                 "RESCALE requires one fixed-point operand.",
+                 cudf::data_type_error);
+    return operand_types.front();


This returns the input scale, not the target scale (the parser patches it).

Suggested change

return operand_types.front();

// Scale is the input's; callers must apply the operation's target scale

return operand_types.front();

mhaseeb123 · 2026-10-27T01:52:36Z

+   * @param input Input fixed-point expression
+   * @param target_scale Desired output scale
+   */
+  operation(ast_operator op, expression const& input, int32_t target_scale);


Use numeric::scale_type to match cudf::data_type (same for the deleted overload, member and get_target_scale()).

Suggested change

operation(ast_operator op, expression const& input, int32_t target_scale);

operation(ast_operator op, expression const& input, numeric::scale_type target_scale);

mhaseeb123 · 2026-10-27T01:52:37Z

+  CAST_TO_DECIMAL128,  ///< Cast a decimal value to decimal128, preserving its scale
+  RESCALE              ///< Rescale a fixed-point value to operation target scale


nit: Document the AST limitation and rounding behavior.

Suggested change

CAST_TO_DECIMAL128, ///< Cast a decimal value to decimal128, preserving its scale

RESCALE ///< Rescale a fixed-point value to operation target scale

CAST_TO_DECIMAL128, ///< Cast a decimal value to decimal128, preserving its scale (JIT only)

RESCALE ///< Rescale a fixed-point value to the target scale (truncates toward zero)

mhaseeb123 · 2026-10-27T01:52:38Z

+  auto rescale_expected = cudf::test::fixed_point_column_wrapper<int32_t>(
+    {1200, -4500, 7800}, {1, 0, 1}, numeric::scale_type{-3});
+  auto rescale_result = Executor::compute_column(table, rescale);
+  CUDF_TEST_EXPECT_COLUMNS_EQUAL(rescale_expected, rescale_result->view(), verbosity);


Optional: Let's also cover rescaling to a coarser scale and RESCALE as an intermediate (the main use case, and the only path where the target scale reaches a non-root operator).

Suggested change

CUDF_TEST_EXPECT_COLUMNS_EQUAL(rescale_expected, rescale_result->view(), verbosity);

CUDF_TEST_EXPECT_COLUMNS_EQUAL(rescale_expected, rescale_result->view(), verbosity);

auto rescale_coarse = cudf::ast::operation{cudf::ast::ast_operator::RESCALE, ref, -1};

auto rescale_coarse_expected = cudf::test::fixed_point_column_wrapper<int32_t>(

{12, -45, 78}, {1, 0, 1}, numeric::scale_type{-1});

auto rescale_coarse_result = Executor::compute_column(table, rescale_coarse);

CUDF_TEST_EXPECT_COLUMNS_EQUAL(rescale_coarse_expected, rescale_coarse_result->view(), verbosity);

auto literal_value = cudf::fixed_point_scalar<numeric::decimal32>(7800, numeric::scale_type{-3});

auto literal = cudf::ast::literal{literal_value};

auto less = cudf::ast::operation{cudf::ast::ast_operator::LESS, rescale, literal};

auto less_expected = column_wrapper<bool>{{true, false, false}, {1, 0, 1}};

auto less_result = Executor::compute_column(table, less);

CUDF_TEST_EXPECT_COLUMNS_EQUAL(less_expected, less_result->view(), verbosity);

mhaseeb123 · 2026-10-27T01:52:39Z

+{
+  using Executor = TypeParam;
+
+  auto input = column_wrapper<int32_t>{{0, 1, 2, 3}, {1, 0, 1, 1}};


Optional: Only INT32 input is covered. Consider adding float, bool and decimal inputs (float -> int truncation, decimal -> int/float scale handling).

mhaseeb123 · 2026-10-27T22:27:30Z

+  CAST_TO_DECIMAL32,   ///< Cast a decimal value to decimal32, preserving its scale
+  CAST_TO_DECIMAL64,   ///< Cast a decimal value to decimal64, preserving its scale
+  CAST_TO_DECIMAL128,  ///< Cast a decimal value to decimal128, preserving its scale


CAST_TO_DECIMAL* only changes the width and RESCALE only changes the scale, so a full decimal cast needs two chained ops whose order matters for overflow. Can CAST_TO_DECIMAL* take an optional target scale instead of a separate RESCALE?

mhaseeb123 · 2026-10-27T22:28:04Z

 private:
  ast_operator op;
  std::vector<std::reference_wrapper<expression const>> operands;
+  std::optional<int32_t> target_scale;


Same here (along with the getter and the constructor in expressions.cpp)

Suggested change

std::optional<int32_t> target_scale;

std::optional<numeric::scale_type> target_scale;

lamarrr added 2 commits September 3, 2026 21:57

Add precompiled checked arithmetic operators

fe6c79b

Add AST cast and rescale operators

14cf17c

github-actions Bot assigned lamarrr Sep 4, 2026

github-actions Bot added libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue labels Sep 4, 2026

lamarrr added feature request New feature or request non-breaking Non-breaking change labels Sep 4, 2026

lamarrr added 2 commits October 21, 2026 21:40

Merge upstream/main into operator-parity-ast-casts

46b1c09

Keep upstream checked-arithmetic fixes and the updated JIT transform calling convention while retaining AST cast and rescale support. Preserve target scale metadata when normalizing Parquet expressions.

Merge branch 'main' into operator-parity-ast-casts

f6225c4

lamarrr marked this pull request as ready for review October 21, 2026 22:57

lamarrr requested a review from a team as a code owner October 21, 2026 22:57

lamarrr requested review from davidwendt and kingcrimsontianyu October 21, 2026 22:57

Update copyright notice to include affiliates in operators.cuh

3ad5653

lamarrr changed the title ~~[FEA] Implement decimal casts for AST~~ [FEA] Implement decimal casts for AST Interpreter Oct 21, 2026

mhaseeb123 reviewed Oct 27, 2026

View reviewed changes

Navigation Menu

Repository navigation

[FEA] Implement decimal casts for AST Interpreter - #23982

[FEA] Implement decimal casts for AST Interpreter#23982
lamarrr wants to merge 5 commits into
NVIDIA:mainfrom
lamarrr:operator-parity-ast-casts

lamarrr commented Sep 4, 2026 •

edited

Loading

Uh oh!

copy-pr-bot Bot commented Sep 4, 2026

Uh oh!

coderabbitai Bot commented Oct 21, 2026 •

edited

Loading

Summary by CodeRabbit

Uh oh!

mhaseeb123 Oct 27, 2026

Uh oh!

mhaseeb123 Oct 27, 2026

Uh oh!

mhaseeb123 Oct 27, 2026

Uh oh!

mhaseeb123 Oct 27, 2026

Uh oh!

mhaseeb123 Oct 27, 2026

Uh oh!

mhaseeb123 Oct 27, 2026

Uh oh!

mhaseeb123 Oct 27, 2026

Uh oh!

mhaseeb123 Oct 27, 2026

Uh oh!

mhaseeb123 Oct 27, 2026

Uh oh!

mhaseeb123 Oct 27, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	auto const target_scale = plan.operator_target_scales[operator_index];
	auto const target_scale =
	op == ast_operator::RESCALE ? plan.operator_target_scales[operator_index] : 0;

	return operand_types.front();
	// Scale is the input's; callers must apply the operation's target scale
	return operand_types.front();

	operation(ast_operator op, expression const& input, int32_t target_scale);
	operation(ast_operator op, expression const& input, numeric::scale_type target_scale);

		CAST_TO_DECIMAL128, ///< Cast a decimal value to decimal128, preserving its scale
		RESCALE ///< Rescale a fixed-point value to operation target scale

-  CUDF_TEST_EXPECT_COLUMNS_EQUAL(rescale_expected, rescale_result->view(), verbosity);
+  CUDF_TEST_EXPECT_COLUMNS_EQUAL(rescale_expected, rescale_result->view(), verbosity);
+  auto rescale_coarse = cudf::ast::operation{cudf::ast::ast_operator::RESCALE, ref, -1};
+  auto rescale_coarse_expected = cudf::test::fixed_point_column_wrapper<int32_t>(
+    {12, -45, 78}, {1, 0, 1}, numeric::scale_type{-1});
+  auto rescale_coarse_result = Executor::compute_column(table, rescale_coarse);
+  CUDF_TEST_EXPECT_COLUMNS_EQUAL(rescale_coarse_expected, rescale_coarse_result->view(), verbosity);
+  auto literal_value = cudf::fixed_point_scalar<numeric::decimal32>(7800, numeric::scale_type{-3});
+  auto literal       = cudf::ast::literal{literal_value};
+  auto less          = cudf::ast::operation{cudf::ast::ast_operator::LESS, rescale, literal};
+  auto less_expected = column_wrapper<bool>{{true, false, false}, {1, 0, 1}};
+  auto less_result   = Executor::compute_column(table, less);
+  CUDF_TEST_EXPECT_COLUMNS_EQUAL(less_expected, less_result->view(), verbosity);

	std::optional<int32_t> target_scale;
	std::optional<numeric::scale_type> target_scale;

Conversation

lamarrr commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

copy-pr-bot Bot commented Sep 4, 2026

Uh oh!

coderabbitai Bot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lamarrr commented Sep 4, 2026 •

edited

Loading

coderabbitai Bot commented Oct 21, 2026 •

edited

Loading