fix: Improve ergonomics for the Decimal type by Divyanshu-s13 · Pull Request #341 · apache/arrow-js

Divyanshu-s13 · 2025-11-18T21:52:56Z

What's Changed

Added a new decimal.ts module that provides high-level utilities for working with Decimal128 and Decimal256 values in JavaScript. These utilities make decimal handling more ergonomic by removing the need for low-level bit manipulation and aligning behavior with Arrow C++ semantics. The module is exported through the util namespace and integrates cleanly without introducing breaking changes.

Closes #81.

…bindings

Divyanshu-s13 · 2025-11-18T21:55:30Z

@kou Fix #81: Improve ergonomics for the Decimal type in JavaScript bindings.

domoritz

Thanks for the pull request. This definitely needs tests.

Also, does this address #100 and #77 in some way?

- Comprehensive tests for isNegativeDecimal, negateDecimal - Tests for toDecimalString and toDecimalNumber conversions - Tests for fromDecimalString parsing with roundtrips - Integration tests for negation and edge cases

Divyanshu-s13 · 2025-11-20T14:46:22Z

Thanks for the pull request. This definitely needs tests.

Also, does this address #100 and #77 in some way?

thanks for review!
I have added a test case for this.

This partially addresses Issues #77 and #100 by providing comprehensive utility functions for working with Decimal types in Apache Arrow JavaScript.

domoritz · 2025-11-24T17:12:15Z

Thanks for these utilities. I think we should try to use them in the default converters so that #77 gets fixed. Either you could do this here or we can do it in a follow up when you fixed the CI errors and we merged.

Divyanshu-s13 · 2025-11-24T18:04:09Z

Thanks for these utilities. I think we should try to use them in the default converters so that #77 gets fixed. Either you could do this here or we can do it in a follow up when you fixed the CI errors and we merged.

I have fixed CI error can you merge it.

kou · 2025-11-25T04:47:03Z

Could you check lint failures?

https://github.com/apache/arrow-js/actions/runs/19644239168/job/56300291328?pr=341#step:5:8

/home/runner/work/arrow-js/arrow-js/src/util/decimal.ts
Error:    84:9  error  'n' is never reassigned. Use 'const' instead                 prefer-const
Error:    89:5  error  This `if` statement can be replaced by a ternary expression  unicorn/prefer-ternary
Error:   128:5  error  This `if` statement can be replaced by a ternary expression  unicorn/prefer-ternary

FYI: You can run CI on your fork by enabling GitHub Actions on your fork.

- Change 'let n' to 'const n' (prefer-const) - Use ternary for magnitude assignment (unicorn/prefer-ternary) - Use ternary for result composition (unicorn/prefer-ternary)

Divyanshu-s13 · 2025-11-25T07:37:19Z

Could you check lint failures?

https://github.com/apache/arrow-js/actions/runs/19644239168/job/56300291328?pr=341#step:5:8
/home/runner/work/arrow-js/arrow-js/src/util/decimal.ts
Error:    84:9  error  'n' is never reassigned. Use 'const' instead                 prefer-const
Error:    89:5  error  This `if` statement can be replaced by a ternary expression  unicorn/prefer-ternary
Error:   128:5  error  This `if` statement can be replaced by a ternary expression  unicorn/prefer-ternary
FYI: You can run CI on your fork by enabling GitHub Actions on your fork.

I have fix this can you merge it.

Divyanshu-s13 · 2025-11-25T18:10:51Z

@kou @domoritz I have fix all CI failure and all checks are passed please merge it.

domoritz · 2025-11-25T21:33:46Z

src/util/decimal.ts

+    }
+
+    // Calculate divisor as 10^scale
+    // Using a loop instead of BigInt exponentiation (**) for ES2015 compatibility


Should we bump to something beyond es2015 for better code?

ilyabo · 2026-01-13T20:47:29Z

It might be worth it to include Decimal256 support (as suggested by Coderabbit)

Divyanshu-s13 · 2026-02-02T07:49:52Z

@kou @domoritz please review it.

domoritz · 2026-02-04T16:30:21Z

I started looking at this in November. Have you seen my question in #341 (comment)?

domoritz

Thanks for making these utilities. I wonder, can we also use some of these utilities in other tests that already exist?

domoritz · 2026-02-04T16:35:04Z

src/util/decimal.ts

+ * ```
+ * @ignore
+ */
+export function toDecimalString(value: Uint32Array, scale: number): string {


Can you test this with Decimal256? I'm not 100% sure this method support it.

domoritz · 2026-02-04T16:36:30Z

src/util/decimal.ts

+    }
+
+    // Trim trailing zeros in fractional part
+    fracPart = fracPart.replace(/0+$/, '');


Wouldn't this mean that a scale of 5 would still return "0.0" rather than "0.00000"?

domoritz · 2026-02-04T16:37:12Z

src/util/decimal.ts

+ * ```
+ * @ignore
+ */
+export function fromDecimalString(str: string, scale: number): Uint32Array {


If toDecimalString supports Decimal256, shouldn't this support it as well?

domoritz · 2026-02-04T16:37:59Z

src/util/decimal.ts

+    const [wholePart = '0', fracPart = ''] = str.split('.');
+
+    // Pad or truncate fractional part to match scale
+    const adjustedFrac = (fracPart + '0'.repeat(scale)).slice(0, scale);


If the input string has more fractional digits than scale, are they silently truncated without rounding?

domoritz · 2026-02-04T16:39:30Z

src/util/decimal.ts

+    };
+
+    // Detect sign via MSB of most-significant word
+    const isNegative = (value[3] & 0x80000000) !== 0;


Why is this not the same as isNegativeDecimal?

Fix apache#81: Improve ergonomics for the Decimal type in JavaScript …

27b8784

…bindings

domoritz requested changes Nov 20, 2025

View reviewed changes

test: add decimal utilities test suite

f95f882

- Comprehensive tests for isNegativeDecimal, negateDecimal - Tests for toDecimalString and toDecimalNumber conversions - Tests for fromDecimalString parsing with roundtrips - Integration tests for negation and edge cases

kou changed the title ~~Fix #81: Improve ergonomics for the Decimal type in JavaScript bindings~~ fix: Improve ergonomics for the Decimal type Nov 22, 2025

fix:CI failure

ef3acf9

fix: resolve ESLint errors in decimal.ts

a13dcda

- Change 'let n' to 'const n' (prefer-const) - Use ternary for magnitude assignment (unicorn/prefer-ternary) - Use ternary for result composition (unicorn/prefer-ternary)

domoritz reviewed Nov 25, 2025

View reviewed changes

ilyabo mentioned this pull request Jan 13, 2026

SQLRooms Issue: DATE and DECIMAL types display incorrectly in QueryResultPanel sqlrooms/sqlrooms#286

Closed

domoritz reviewed Feb 4, 2026

View reviewed changes

Conversation

Divyanshu-s13 commented Nov 18, 2025 • edited by kou Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's Changed

Uh oh!

Divyanshu-s13 commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

domoritz left a comment

Choose a reason for hiding this comment

Uh oh!

Divyanshu-s13 commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

domoritz commented Nov 24, 2025

Uh oh!

Divyanshu-s13 commented Nov 24, 2025

Uh oh!

kou commented Nov 25, 2025

Uh oh!

Divyanshu-s13 commented Nov 25, 2025

Uh oh!

Divyanshu-s13 commented Nov 25, 2025

Uh oh!

domoritz Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

ilyabo commented Jan 13, 2026

Uh oh!

Divyanshu-s13 commented Feb 2, 2026

Uh oh!

domoritz commented Feb 4, 2026

Uh oh!

domoritz left a comment

Choose a reason for hiding this comment

Uh oh!

domoritz Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

domoritz Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

domoritz Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

domoritz Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

domoritz Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Divyanshu-s13 commented Nov 18, 2025 •

edited by kou

Loading

Divyanshu-s13 commented Nov 18, 2025 •

edited

Loading

Divyanshu-s13 commented Nov 20, 2025 •

edited

Loading