feat(binary): the 32- and 64-bit tiers, and decimal-typed columns - #24
Merged
Conversation
Phase 1 covered Int128 and Int256, the tiers a caller cannot hand-roll safely. Parquet also stores small decimals in INT32 and INT64, and in FIXED_LEN_BYTE_ARRAY fields narrower than either — a DECIMAL(6,2) is three bytes — so the narrow tiers need the same width-parameterised treatment rather than a BinaryPrimitives call at the natural width. Add ReadInt32/ReadInt64 and their Try, Write, and column forms, with the same semantics as the wide tiers: the span length is the field width, reads sign-extend, writes sign-extend across the field and report values it cannot hold. A column at the mantissa's own width skips the fit check and the scratch buffer entirely — a wholesale copy on a little-endian host, one byte swap per element otherwise. Add ReadDecimal32 through WriteDecimal256 for columns of the wrapper types. Each Decimal* is a single-field Sequential struct over its mantissa, so these are the mantissa overloads with the reinterpret kept inside the package; a test pins the sizes so a field added to a wrapper fails there rather than silently halving a column's length. Closes #16.
There was a problem hiding this comment.
Pull request overview
Extends Clast.DatabaseDecimal.Binary.DecimalBinary to support 32- and 64-bit mantissa tiers (including sub-width Parquet-style fields) and adds bulk read/write overloads that operate directly on Decimal32/64/128/256 wrapper columns, keeping the layout reinterpretation inside the library boundary.
Changes:
- Added scalar
Read/TryRead/Write/TryWriteAPIs forInt32andInt64using the same sign-extension + fit-check semantics as the wide tiers. - Added bulk
ReadInt32/WriteInt32andReadInt64/WriteInt64with natural-width fast paths and generalized-width handling. - Added bulk
ReadDecimal*/WriteDecimal*column overloads implemented viaMemoryMarshal.Castto the underlying mantissa spans, plus tests and README updates covering the new surface.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Clast.DatabaseDecimal.Tests/ReadmeExampleTests.cs | Updates compiled README example test to use the new ReadDecimal128 column overload. |
| tests/Clast.DatabaseDecimal.Tests/DecimalBinaryTests.NarrowTiers.cs | Adds comprehensive tests for Int32/Int64 width/order behavior and decimal-wrapper column overload equivalence. |
| tests/Clast.DatabaseDecimal.Tests/DecimalBinaryTests.cs | Makes the existing test class partial to share helpers with the new narrow-tier test file. |
| src/Clast.DatabaseDecimal/Binary/DecimalBinary.cs | Implements Int32/Int64 scalar + bulk APIs and adds ReadDecimal*/WriteDecimal* column overloads. |
| README.md | Expands the binary section to describe and demonstrate the new decimal-typed column APIs and tier coverage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2 of #16, completing the design proposal. Phase 1 (#22) covered
Int128andInt256.Why the narrow tiers need the same treatment
Parquet stores small decimals in
INT32andINT64, and inFIXED_LEN_BYTE_ARRAYfields narrower than either — aDECIMAL(6,2)is three bytes. ABinaryPrimitives.ReadInt32BigEndianat the natural width does not cover that, so the 32- and 64-bit tiers get the same width-parameterised API as the wide ones.Added
Semantics are unchanged from phase 1: the span length is the field width, reads sign-extend from the top bit of the field, writes sign-extend across it and reject what will not fit, and
DecimalOverflow.Ignoretruncates for callers that have already proven range.Natural-width columns skip the general path. A field of the mantissa's own width always fits, so there is no fit check and no scratch buffer — a wholesale copy on a little-endian host, and one
BinaryPrimitivesbyte swap per element otherwise. That is the dominant ParquetINT32/INT64case.The
Decimal*overloads are the mantissa overloads with the reinterpret kept inside the package — each wrapper is a single-fieldSequentialstruct over its mantissa, which is exactly the layout knowledge the issue asked to move to this side of the boundary.Tests
28 new cases, 971 passing across net8.0/net10.0/net472.
Int32andInt64round-tripped against the BigInteger oracle at every width (1..4 and 1..8) in both orders, with each width's own edges.0xFFFFFFreads as-1,2^23 - 1and-2^23fit,2^23does not.Decimal*column overload asserted byte-identical to its mantissa overload, at Arrow's width and at a narrower Parquet one.Unsafe.SizeOfpinned for all four wrappers, so a field added to one fails a test rather than silently halving a column's length.The README's binary section and its compiled example now cover the full surface.