Skip to content

[python] Preserve high-precision DECIMAL values in the row file format - #10124

Merged
JingsongLi merged 2 commits into
apache:masterfrom
jackylee-ch:py-row-decimal-precision
Sep 24, 2026
Merged

JingsongLi merged 2 commits into
apache:masterfrom
jackylee-ch:py-row-decimal-precision

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

Purpose

Reading and writing a DECIMAL column in the row file format both use Python decimal arithmetic under the default context (28 significant digits). For DECIMAL(p) with p > 28, the unscaled integer has up to 38 digits, so:

  • the writer's int(value * (10 ** scale)) rounds to 28 digits before encoding, and
  • the reader's Decimal(unscaled) / Decimal(10 ** scale) rounds to 28 digits after decoding.

A DECIMAL(38, 0) value 12345678901234567890123456789012345678 round-trips as 12345678901234567890123456790000000000 — silent data corruption.

The fix computes the unscaled value (write) and rescales it (read) under a context wide enough for the column — max(precision + abs(scale), 38) with scaleb — mirroring the existing pypaimon/data/decimal.py. DECIMAL(p <= 18) is unchanged.

Tests

test_format_row_reader_writer.test_high_precision_decimal round-trips positive, negative and null values across DECIMAL(38,0), DECIMAL(38,10), DECIMAL(28,4) and DECIMAL(18,2) and asserts exact values. Fails on master (DECIMAL(38,0) reads back rounded to 28 digits); passes here.

finally:
os.unlink(path)

def test_high_precision_decimal(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, not a blocker
This test changes the writer and reader together, so a symmetric scaling mistake in both paths could still round-trip successfully. Since row files are shared with the Java implementation, could we add one independent assertion - either decode a hand-built signed unscaled byte sequence for  DECIMAL(38, 10) , or inspect the writer’s encoded bytes and compare them with the exact expected unscaled integer? That would directly protect cross-language wire compatibility rather than only the Python writer/reader pair

Reading and writing a DECIMAL column in the row file format both used Python
decimal arithmetic under the default context, whose precision is 28 significant
digits. For DECIMAL(p) with p greater than 28, the unscaled value has up to 38
significant digits, so the writer's value * 10^scale and the reader's
Decimal(unscaled) / Decimal(10^scale) silently rounded it to 28 digits: a
DECIMAL(38, 0) value 12345678901234567890123456789012345678 round-tripped as
12345678901234567890123456790000000000.

Compute the unscaled value on write and rescale it on read under a context wide
enough for the column (max(precision + abs(scale), 38)) using scaleb, mirroring
pypaimon/data/decimal.py. DECIMAL(p) with p <= 18 is unchanged.
… DECIMAL

Decode a hand-built signed unscaled byte sequence for DECIMAL(38, 10) and assert
the exact value, so a symmetric scaling mistake in the writer and reader cannot
round-trip undetected and the row-file wire form stays compatible with the Java
implementation.
@jackylee-ch
jackylee-ch force-pushed the py-row-decimal-precision branch from 98e78ae to c6d02e2 Compare September 23, 2026 04:52
@JingsongLi

Copy link
Copy Markdown
Contributor

This fixes silent high-precision DECIMAL corruption in the row format. I reviewed both write-side unscaled conversion and read-side rescaling under localcontext; the context width covers the declared precision/scale, and the low-precision wire branch remains intact. The independent hand-built wire test addresses the earlier cross-language compatibility comment.

Local verification on the PR patch: all 27 test_format_row_reader_writer.py tests passed, including the positive/negative/null DECIMAL round-trips and raw-wire assertion. The current PR checks remain red in Python 3.10–3.13 (plus the aggregate result). Please resolve those matrix failures or establish an unrelated cause before production merge. This has clear end-to-end data-integrity value, and I found no blocking defect in the changed decimal conversion.

@JingsongLi
JingsongLi merged commit 414080b into apache:master Sep 24, 2026
9 of 14 checks passed
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.

3 participants