Skip to content

refactor: share negative-scale decimal checks across Scala serdes - #7334

Draft
Abhishek-Mohan wants to merge 1 commit into
apache:mainfrom
Abhishek-Mohan:contrib/7021-negative-scale-checks
Draft

Abhishek-Mohan wants to merge 1 commit into
apache:mainfrom
Abhishek-Mohan:contrib/7021-negative-scale-checks

Conversation

@Abhishek-Mohan

Copy link
Copy Markdown

Which issue does this PR close?

Closes #7021.

Rationale for this change

Negative-scale decimal checks are repeated across the cast, arithmetic, and math serdes. Sharing the predicate and unsupported-native reason keeps those checks consistent while preserving each expression's existing execution route.

The decimal-to-string reason remains specific: it describes a configuration prerequisite and returns Incompatible when the legacy flag is disabled, rather than an unconditional lack of native support. This is the intentional exception to the issue's proposal for one reason everywhere.

What changes are included in this PR?

  • Add a pure, non-recursive isNegativeScaleDecimal(DataType) predicate and shared reason in the DataTypeSupport companion object.
  • Replace the existing checks in place for casts, division, round, ceil, and floor. Preserve division's left, right, and result checks, branch ordering, support levels, configuration gates, and native/codegen-dispatch/Spark-fallback behavior.
  • Consolidate the unsupported-native reasons, retain the decimal-to-string configuration reason, and expose the shared restriction through ceil/floor's getUnsupportedReasons metadata.
  • Add predicate and ceil/floor regression assertions, strengthen round's materialized-input dispatch assertion, and update existing cast/division reason references.

How are these changes tested?

Draft: compilation and runtime validation are pending. No Maven, Cargo, formatter, or local-CI runs have been performed. Local builds remain paused to coordinate shared caches.

Completed: git diff --check and static review of the six-file patch.

Tests added or updated cover:

  • Negative, zero, and positive decimal scales; non-decimal and nested-container inputs; predicate independence from the legacy flag.
  • Ceil/floor support levels and Spark fallback on materialized negative-scale inputs, with non-negative controls and explicit native execution assertions.
  • Explicit round codegen dispatch on a materialized negative-scale decimal column.
  • Existing cast/division reason checks, retaining the materialized-input, nested-cast, and safe-operation regression tests.

Before marking ready: run compilation and formatting/lint checks, CometExpressionSuite, and CometNativeCastSuite. Since this touches serdes, obtain a run-spark-4.1-tests verdict before queueing. Generated documentation output also remains unverified.

Council review

Rounds: 2. These are static-review scores, conditional on pending compilation and runtime validation.

Delegate Round 1 Round 2
Claude Opus 5.5 8/10 10/10
Gemini 4 Argon 10/10 10/10
GPT-6 Astra 9/10 10/10

Round 1 identified two test lines exceeding the configured formatting width; both were wrapped. All round-2 reviewers reported no actionable findings. Interrupted review attempts were retried and are not counted as extra rounds.

AI assistance: implementation by Claude Opus 5.5, orchestration by GPT-6 Astra, and council review by the models above using the tstack workflow in T3 Code / OpenCode.

@github-actions github-actions Bot added enhancement New feature or request area:expressions Expression evaluation labels Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:expressions Expression evaluation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use one shared check and reason string for negative-scale decimals across the serdes

1 participant