Skip to content

feat(parquet): extended-precision timestamps on FIXED_LEN_BYTE_ARRAY(12), behind EWPARQUET0004 - #217

Open
CurtHagenlocher wants to merge 8 commits into
mainfrom
flba12-timestamps
Open

feat(parquet): extended-precision timestamps on FIXED_LEN_BYTE_ARRAY(12), behind EWPARQUET0004#217
CurtHagenlocher wants to merge 8 commits into
mainfrom
flba12-timestamps

Conversation

@CurtHagenlocher

Copy link
Copy Markdown
Collaborator

Read and write support for TIMESTAMP annotating FIXED_LEN_BYTE_ARRAY(12) — a signed 96-bit
little-endian count of the column's declared unit since the epoch, covering the full ANSI SQL
TIMESTAMP(9) range where INT64 nanoseconds stops at 1677-09-21 and 2262-04-11.

Proposed in parquet-format#600. Gated behind
[Experimental("EWPARQUET0004")] and off by default on the write side.

Design record: doc/parquet-extended-precision-timestamps.md.

Read this part before the rest: nothing upstream is merged

parquet-format#601 — the spec change open
parquet-java#3680 — reference implementation open
parquet-testing#123 — the fixture open

And the byte order is not settled. The spec text, parquet-java and the fixture are all
little-endian, but a proposal co-author argued for big-endian on the spec PR so readers could reuse
the DECIMAL comparator, and the approving reviewer signed off with "LGTM, we can bikeshed more on
little endian vs big-endian to finalize this."

Nothing on the wire distinguishes the two. If it flips, files already written do not become
unreadable — they become silently wrong-valued
, which is worse. That is what the experimental gate
is carrying, and it is a stronger reason for the gate than ALP or FSST had. All byte-order code goes
through one type so a flip is a one-file change; the doc lists the four places.

If you would rather wait for ratification, that is a completely reasonable call and this can sit.

What Arrow forces

Arrow's timestamp is hard-coded to int64. timestamp128
(apache/arrow#47848) has been dormant since May 2026
and there is no canonical extension type, so the read mapping is our choice, not a standard:

ExtendedTimestampOutputKind Arrow type
TimestampMicroseconds (default) timestamp[us, tz?] ±292,000 years, so a conforming file always reads
Timestamp timestamp[declared unit, tz?] keeps every digit; reports what int64 cannot hold
FixedSizeBinary fixed_size_binary[12] raw bytes

A deliberate sibling of Int96OutputKind, not the same enum: INT96 carries no annotation so its
unit is genuinely the reader's choice, while this carrier declares its own and reading at another
unit would be a rescale. The default was Timestamp for one commit and the corpus sweep changed
it
— adding the fixture broke ReadRowGroupTests, because a plain ParquetFileReader could not
read a valid file. Reading a valid file should not require knowing in advance what is in it.

I deliberately did not invent an ew.* Arrow extension name for a type Arrow may standardise
differently later.

The promotion can never be automatic

An Arrow timestamp is int64, so any value Arrow can hold already fits INT64. Nothing this
library can be handed needs the wider carrier. ParquetWriteOptions.ExtendedTimestampColumns exists
to produce files in that shape — interop fixtures, and readers being tested against the proposal.

It follows that this library cannot write the values the carrier exists for. The fixture's
timestamp_nanos column is interesting precisely because two rows need more than 64 bits, and those
rows cannot be expressed in Arrow at all. MILLIS and MICROS reproduce byte for byte.

Things that are easy to get wrong, and how they are handled

  • converted_type is omitted. TIMESTAMP_MILLIS/MICROS are INT64-only, so a converted-type-aware
    reader would decode twelve bytes as eight. Not in the spec PR's text — parquet-java found it in
    review — so it has its own test.
  • This is the one FLBA column that is not lexicographically ordered. DECIMAL escapes the problem
    by being rewritten to big-endian before statistics run; this cannot. Statistics use a signed 96-bit
    comparator, and the deprecated min/max are dropped because no reader could reproduce signed
    ordering from little-endian bytes.
  • Nested paths are refused, not ignored. The schema is built by ArrowToSchemaConverter while the
    physical type of the data is decided by NestedLevelWriter, which does not see these options —
    honouring a nested request would put FIXED_LEN_BYTE_ARRAY(12) in the footer over INT64 pages.

One general fix rides along

No predicate on a DATE, TIME or TIMESTAMP column could ever probe a bloom filter, for any
column — the coercion dispatched on the physical type and the statistics layer hands those literals
over as DateOnly/TimeOnly/DateTimeOffset. Enabling a bloom filter on a timestamp column bought
nothing. Fixed for ordinary INT64 columns as well as the carrier; supporting the experimental case
and not the everyday one would have been a strange place to stop.

Verification, and its limits

There is no external oracle. PyArrow, DuckDB and delta-rs cannot read this combination and
parquet-java's implementation is unmerged. This is weaker verification than ALP or FSST had.

⚠️ The conformance tests no-op in CI. They read flba12_timestamp.parquet from the
parquet-testing submodule, and parquet-testing#123 has not merged — so green CI here does not
mean they ran.
They were developed against a local copy of the proposed file, where all eighteen
byte sequences (six timestamps × three units) were confirmed to appear verbatim before any of this
was written. Expectations come from that file's own documented table, not from our decoder.

What is checked locally:

  • Round trip through both writers, compared byte for byte — BufferedParquetWriter is an
    independent implementation that has drifted before, and a carrier encoded two ways would drift
    silently, since both files would be well-formed.
  • A test guarding the fixture's premise — that its two extreme rows really do exceed int64 — so the
    range-refusal case cannot go quiet if the file is regenerated.
  • Bloom-filter tests probe a gap value (inside min/max, absent from the column), because a value
    outside min/max is pruned by statistics and would pass whether or not the filter was consulted.
    Each carries a FilterUseBloomFilters = false control.

Suite: 1108 passed, 37 skipped on net10.0 and 1102 passed, 37 skipped on net472 (the skips are
the cloud-emulator tests, per #79). Solution builds clean including the net10.0 AOT/trim gate.

Dependency

EngineeredWood.Parquet gains Clast.DatabaseDecimalfor System.Int128, not Decimal128.
The carrier needs >64-bit integer math and netstandard2.0 has none; that package ships a public
polyfill. Decimal128 is the wrong type: this is an integer count, its scale would be pinned at 0,
and its CompareTo is scale-aware rather than the byte order the spec defines. The polyfill is
partial (no Parse, no generic math) — reported as
clast-project/database-decimal#18.

🤖 Generated with Claude Code

CurtHagenlocher and others added 7 commits August 22, 2026 14:22
Groundwork for apache/parquet-format#601, which lets TIMESTAMP annotate
FIXED_LEN_BYTE_ARRAY(12): a signed two's-complement LITTLE-ENDIAN count of the
declared TimeUnit since the Unix epoch. Ninety-six bits covers the whole ANSI SQL
TIMESTAMP(9) range; INT64 nanoseconds stops at 1677-09-21 and 2262-04-11.

This is the value layer only -- no schema mapping and no reader or writer wiring,
so nothing observable changes yet.

WHY THE DEPENDENCY IS Clast.DatabaseDecimal AND WHY IT IS NOT Decimal128. The
carrier needs >64-bit integer math and netstandard2.0 has no Int128. That package
ships a PUBLIC System.Int128/UInt128 polyfill -- undocumented in its description,
but there, and one reference covers every TFM here. Decimal128 was the obvious
guess and is the wrong type: this is an integer count of units, its scale would
be pinned at 0 forever, and Decimal128's CompareTo is scale-aware decimal
comparison rather than the byte order the spec defines.

The polyfill is PARTIAL. No Parse, no TryFormat, no generic-math interfaces, and
several conversions that are implicit on the BCL type are not -- `Int128 | ulong`
does not compile, so every widening here is written out. The tests carry their own
ParseInt128 for the same reason.

THE COMPARATOR DELIBERATELY DOES NOT USE Int128. It runs once per value while
collecting statistics, so it reads the high word signed and the low word unsigned
straight out of the bytes -- which is also the shape parquet-java landed after
review. TheByteComparatorAgreesWithInt128 is what keeps that shortcut honest.

CONFORMANCE, NOT SELF-CONSISTENCY. All eighteen encodings in the tests (six
timestamps x three units) were confirmed to appear verbatim in
flba12_timestamp.parquet, the fixture proposed in apache/parquet-testing#123 --
including the two nanosecond values that need more than 64 bits, one in each
direction. So the byte layout is pinned against the reference file rather than
against our own encoder.

The tests run on net472 as well, which is the leg where the polyfill actually
executes rather than the BCL type. Rescaling floors rather than truncates, for the
reason the INT96 path already floors: truncation toward zero would make a
pre-epoch value round the opposite way from a post-epoch one and stop being
monotonic.

THE BYTE ORDER IS NOT SETTLED. The proposal, parquet-java#3680 and the fixture are
all little-endian, but a co-author argued for big-endian on the spec PR and the
approving reviewer said the choice was still open. Nothing on the wire
distinguishes the two, so a flip makes already-written files silently
wrong-valued rather than unreadable. Every entry point goes through one file so
that a flip is a one-file change, and the experimental gate (EWPARQUET0004, still
to come) is what carries the risk.

Parquet suite 1073/1073 on net10.0 and 1067/1067 on net472; solution builds clean
including the net10.0 AOT/trim gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The read half of apache/parquet-format#600, behind EWPARQUET0004.

ExtendedTimestampOutputKind is a SIBLING of Int96OutputKind, not the same enum,
and the reason is not stylistic. INT96 carries no logical annotation, so its unit
is genuinely the reader's choice and TimestampMicroseconds/TimestampNanoseconds
are both meaningful. This carrier declares MILLIS, MICROS or NANOS in the file --
reading at another unit is a rescale, not an output kind. The two also want
opposite defaults, since INT96's default is the one that never throws and this
one's is not. What they DO share is the narrowing machinery: both are twelve
opaque bytes in the value buffer that become eight in place at Build time, so
the hook and the idempotence flag are now shared.

THE DEFAULT REFUSES SOME LEGAL FILES, BY DESIGN. Arrow timestamps are int64, so
`Timestamp` mode cannot represent year 9999 in nanoseconds -- which is not a
corrupt file, it is the case the carrier exists for. It reports the row, the
value and a remedy rather than wrapping into a plausible-looking date.
TimestampMicroseconds spans +/-292,000 years and always produces an answer;
FixedSizeBinary declines to interpret.

That has a consequence worth naming: any corpus-wide sweep now needs
TimestampMicroseconds, because the upstream conformance fixture is unreadable
under the default. ReadRowGroupTests' sweep is updated accordingly -- its
question is "can every file be read at all", and repeating a refusal that
ExtendedTimestampReadTests already covers would only stop it reaching the rest of
the file. The same will apply to the compatibility harness and the parquity
bridge when they meet such a file.

The declared unit is carried on ColumnBuildState because narrowing happens at
Build time, where the column descriptor is long out of scope -- and the target
Arrow unit alone does not say what to rescale FROM.

TESTED AGAINST THE REFERENCE FILE, not against ourselves: flba12_timestamp.parquet
from apache/parquet-testing#123, three columns over six timestamps, expectations
taken from the fixture's own documented table. The raw-bytes test rebuilds the
encoding from those epoch seconds, so it compares the file to the spec rather
than to our decoder. One test guards the fixture's premise -- that its two extreme
rows really do exceed int64 -- so the refusal above cannot go quiet if the file
is ever regenerated. All of it no-ops until #123 merges and the submodule moves;
verified locally against the proposed file, where corrupting one expected value
fails 8 of the 10.

TimestampCarrierGateTests' fall-through assertion for FLBA(12) is replaced rather
than deleted: that width now decodes, and the fall-through it was pinning is now
pinned at every OTHER width, which is where it still holds.

Parquet suite 1096/1096 on net10.0 and 1090/1090 on net472; solution builds clean
including the net10.0 AOT/trim gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nnot fail

The previous default kept the file's declared unit and REFUSED any value int64
could not hold. That is a defensible trade in isolation and a bad default in
practice, and the corpus sweep proved it: adding the upstream conformance fixture
to parquet-testing broke ReadRowGroupTests outright, because a plain
ParquetFileReader could not read a valid, spec-conforming file. The same would
have hit the compatibility harness, the parquity bridge, and anyone calling the
reader with no options.

So the default is now TimestampMicroseconds, matching Int96OutputKind's default
and for the same stated reason: reading a valid file should not require knowing
in advance what is in it. Microseconds span +/-292,000 years, so every date the
carrier exists to hold survives. The cost is the last three digits of a NANOS
column, and Timestamp mode is still there for callers who would rather be told
than lose them.

The sweep's TimestampMicroseconds workaround is reverted with the default that
made it necessary -- it reads the fixture on stock options now, which is the
property worth having.

CORRECTING MYSELF: the docs said TimestampMicroseconds "never reports a range
error". Not true. The carrier holds +/-2^95 units, which in microseconds is far
past int64, so an extreme value still overflows and is reported. Nothing
representing a date can reach it, but the claim was wrong and is now stated
accurately.

TimestampMicroseconds takes the 0 slot so `default(ExtendedTimestampOutputKind)`
is the default behaviour rather than the strict one -- again as Int96OutputKind
does. Breaking against the previous commit only; nothing has shipped.

The range message no longer offers TimestampMicroseconds as an escape when it IS
microseconds that overflowed, and otherwise says which mode the caller is in
rather than only what to switch to.

Parquet suite 1099/1099 on net10.0 and 1093/1093 on net472; solution builds clean
including the net10.0 AOT/trim gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 3: min/max in both directions for TIMESTAMP on FIXED_LEN_BYTE_ARRAY(12).

THE COMPARATOR. This is the one FLBA column whose bytes are not ordered
lexicographically. It is little-endian two's complement, so the most significant
byte is LAST and -1 encodes as all-0xFF -- which SequenceCompareTo ranks above
every positive value. DECIMAL sidesteps this by being rewritten to big-endian
before statistics run; this carrier cannot, because little-endian is what the
spec puts on the wire. StatisticsCollector therefore takes a comparator switch
for FLBA, and the writer sets it from (Arrow TimestampType, FLBA) -- a pair an
Arrow timestamp reaches by no other route, so the parquet logical type is not
needed at that point.

TheLexicographicComparatorReallyWouldDisagree pins that the default comparator is
genuinely wrong here, so the tests above it cannot quietly become tautologies.

THE BOUNDS DECODE via BigInteger's byte[] constructor, which reads little-endian
two's complement and takes the sign from the top bit of the last byte -- exactly
this layout, and exactly why DECIMAL next to it has to reverse first.

Verified against the upstream fixture's own footer: apache/parquet-testing#123
carries min = year 0001 and max = year 9999 on all three columns, both far
outside int64 nanoseconds, so a reader that could only narrow to int64 would have
no bounds to offer at all. A second test reads the column back and checks the
footer is not lying about it, which is the property that makes a bound safe to
prune on.

The write half is still latent -- nothing emits this carrier until phase 4 -- so
the collector is exercised directly rather than through a round trip.

Parquet suite 1119/1119 on net10.0 and 1113/1113 on net472; solution builds clean
including the net10.0 AOT/trim gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 4, opted into per column with ParquetWriteOptions.ExtendedTimestampColumns.

THE PROMOTION IS NEVER AUTOMATIC AND CANNOT BE. An Arrow timestamp is int64, so
any value Arrow can hold already fits INT64 with room to spare -- nothing this
library can be handed NEEDS the wider carrier. The option exists to produce files
in that shape, for interop fixtures and for readers being tested against the
proposal. It follows that we cannot write the far-past and far-future NANOSECOND
values that motivate the carrier at all: they cannot be expressed in Arrow to
begin with. The MILLIS and MICROS columns of the upstream fixture are fully
reproducible, and ReproducesTheFixtureEncodingExactly checks all six values of
each against the byte sequences confirmed to appear verbatim in that file.

converted_type is OMITTED for this carrier. TIMESTAMP_MILLIS and TIMESTAMP_MICROS
are defined for INT64 only, so a reader that understands converted types but not
the new logical-type carrier would decode twelve bytes as eight. This is not in
the spec PR's text -- parquet-java found it in review -- and it has its own test.

NESTED PATHS ARE REFUSED RATHER THAN IGNORED. The schema is built by
ArrowToSchemaConverter while the physical type the data is written with is decided
by NestedLevelWriter, which does not see these options. Honouring a nested request
would put FIXED_LEN_BYTE_ARRAY(12) in the footer over pages holding INT64: a
well-formed file that is wrong. A column named but not a timestamp is refused for
the same reason -- the caller asked for something and would otherwise silently get
something else.

BOTH WRITERS, ONE ENCODER. The encoder lives on ExtendedTimestamp because the
buffered writer is an independent implementation that has drifted from the
streaming one before, and a carrier encoded two ways would drift SILENTLY -- both
files would be well-formed. BothWritersProduceTheSameBytes pins that.

The buffered writer encodes at accumulation time, because its encoders dispatch on
the Arrow type and so have to see the carrier rather than a timestamp. That
uncovered a double-encode: its dictionary-fallback path reconstructs the column
and hands it to ColumnChunkWriter, which would encode the already-twelve-byte
values a second time and read them back as int64. Caught by the round trip through
both writers; the encode step now runs only while the values are still timestamps.

Statistics key off the option and the path rather than the Arrow type, because the
type is what the encode step changes.

Parquet suite 1130/1130 on net10.0 and 1124/1124 on net472; solution builds clean
including the net10.0 AOT/trim gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 5. A predicate on a DATE, TIME or TIMESTAMP column could never consult a
bloom filter, so writing one on such a column bought nothing.

The bloom coercion dispatched on the PHYSICAL type alone, and the statistics layer
hands temporal literals over as DateOnly / TimeOnly / DateTimeOffset -- none of
which any physical arm accepted, so every one fell through to null and the filter
went unread. Not a correctness bug (declining to probe only costs a pruning
opportunity) but the opportunity was the entire point of writing the filter.

Temporal literals are now decided by the LOGICAL type, before the physical
dispatch. That covers the extended-precision carrier as well: the filter holds the
hash of the bytes as they sit in the file, so a timestamp literal against a
promoted column becomes the same twelve little-endian bytes rather than an int64.
Ordinary INT64 timestamp columns get it too -- supporting the experimental carrier
and not the everyday case would have been a strange place to stop.

EXACTNESS IS THE RULE. A literal is worth probing with only if it converts to the
column's unit with no remainder. 1500.5 ms is not a MILLIS value, and rounding it
would probe for something the caller never asked about; declining means the row
group is read, which is always safe. A tick is 100 ns, so NANOS never has to
decline and MILLIS/MICROS sometimes do.

THE TESTS WERE WRONG FIRST, AND PASSED. Every "this gets pruned" case used a
literal outside the column's min/max -- which STATISTICS prune, with or without a
bloom filter, so all of them passed with the new coercion disabled. They now probe
a GAP: a value inside min/max and absent from the column, which is the only thing
a bloom filter can rule out that statistics cannot. Each such test carries its own
control that reads the same file with FilterUseBloomFilters off and asserts the
row group survives. Disabling the coercion now fails 3 of the 7.

The write side needed nothing: the filter is built after the carrier encoding, so
it already hashed the twelve bytes that reach the file.

Parquet suite 1137/1137 on net10.0 and 1131/1131 on net472; solution builds clean
including the net10.0 AOT/trim gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ostics

Phase 7.

doc/parquet-extended-precision-timestamps.md, following the parquet-fsst.md shape:
what the carrier is, and then the part a reader of the code cannot recover --
which decisions are ours rather than the spec's.

The headline is that the BYTE ORDER IS NOT SETTLED. The spec text, parquet-java
and the fixture are all little-endian, but a proposal co-author argued for
big-endian on the spec PR and the approving reviewer left the choice explicitly
open. Nothing on the wire distinguishes the two, so a flip makes already-written
files silently wrong-valued rather than unreadable. The doc carries a four-item
list of exactly what would change, since that is the question anyone will have.

Also recorded: that Arrow has no type for this and no plan for one, so the read
mapping is our choice and not a standard; that the default was `Timestamp` for one
commit and the corpus sweep is what changed it; that converted_type suppression is
parquet-java's finding and not in the spec PR's text; that we deliberately did not
invent an Arrow extension name; and that the promotion can never be automatic --
with the consequence that this library cannot write the very values the carrier
exists for, because they cannot be expressed in Arrow.

Validation gets its own section INCLUDING ITS LIMITS: there is no external oracle,
which is weaker than ALP and FSST had, and it compounds the endianness risk. What
we do have is stated precisely enough to be checked.

README gains a diagnostics table, which BACK-FILLS EWPARQUET0002 -- it gated the
one option here that produces files no other implementation can read, and it was
documented nowhere but its own XML comment.

known-issues.md gains the three residual limits (top-level only, no Arrow
extension type, cannot write out-of-int64 values), plus a note on the Column Index
entry: if page indexes are ever added, these bounds must never be truncated,
because truncation assumes lexicographic order and this carrier is little-endian
signed. parquet-java had to special-case BinaryTruncator for exactly that.

Every relative link checked to resolve. Parquet suite 1137/1137 on net10.0 and
1131/1131 on net472; solution builds clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds experimental (EWPARQUET0004) support in EngineeredWood.Parquet for Parquet’s proposed extended-precision TIMESTAMP carrier: TIMESTAMP-annotated FIXED_LEN_BYTE_ARRAY(12) storing a signed 96-bit little-endian count of the declared unit since epoch. This extends the read/write pipeline (schema mapping, encoding, stats, bloom filters) and pins behavior with new tests and design docs.

Changes:

  • Implement read mapping and write opt-in (ParquetWriteOptions.ExtendedTimestampColumns) for FLBA(12) TIMESTAMP, with selectable read output via ParquetReadOptions.ExtendedTimestampOutput.
  • Add correct signed 96-bit ordering for statistics and enable exact temporal bloom-filter probing (including the extended carrier).
  • Add extensive tests and documentation covering fixture expectations, writer parity, statistics behavior, and operational constraints.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/EngineeredWood.Parquet.Tests/Parquet/Data/TimestampCarrierGateTests.cs Updates gate tests to allow only FLBA(12) TIMESTAMP decoding and keep other shapes as physical fallthrough.
test/EngineeredWood.Parquet.Tests/Parquet/Data/TemporalBloomFilterTests.cs New tests ensuring temporal predicates can probe bloom filters (including the extended carrier) without unsafe rounding.
test/EngineeredWood.Parquet.Tests/Parquet/Data/ExtendedTimestampWriteTests.cs New writer tests for opt-in promotion, footer semantics, writer parity, and stats/minmax behavior.
test/EngineeredWood.Parquet.Tests/Parquet/Data/ExtendedTimestampTests.cs New unit tests for the 96-bit encoding/decoding, comparator correctness, scaling, and range properties.
test/EngineeredWood.Parquet.Tests/Parquet/Data/ExtendedTimestampStatisticsTests.cs New tests for correct stats ordering/bounds for the carrier and fixture-based read-side bounds checks.
test/EngineeredWood.Parquet.Tests/Parquet/Data/ExtendedTimestampReadTests.cs New tests for default microsecond-rescale reads, declared-unit reads, raw bytes, and range-refusal behavior.
src/EngineeredWood.Parquet/Parquet/ParquetWriteOptions.cs Adds ExtendedTimestampColumns and internal helpers to recognize promoted columns.
src/EngineeredWood.Parquet/Parquet/ParquetStatisticsAccessor.cs Adds bound decoding for FLBA(12) TIMESTAMP via little-endian two’s-complement (BigInteger).
src/EngineeredWood.Parquet/Parquet/ParquetReadOptions.cs Introduces ExtendedTimestampOutputKind and ParquetReadOptions.ExtendedTimestampOutput (experimental).
src/EngineeredWood.Parquet/Parquet/ParquetFileWriter.cs Threads write options into schema conversion so promotion is reflected in the footer.
src/EngineeredWood.Parquet/Parquet/Data/StatisticsCollector.cs Adds a carrier-aware FLBA comparator path for correct min/max ordering.
src/EngineeredWood.Parquet/Parquet/Data/ExtendedTimestamp.cs New core implementation for encoding/decoding/rescaling/comparison and column promotion encoding.
src/EngineeredWood.Parquet/Parquet/Data/ColumnChunkWriter.cs Performs promotion-to-bytes early in the pipeline and ensures downstream (dict/stats/bloom) sees on-wire bytes.
src/EngineeredWood.Parquet/Parquet/Data/ColumnChunkReader.cs Plumbs declared unit into build state so narrowing/rescaling can happen at array build time.
src/EngineeredWood.Parquet/Parquet/Data/ArrowToSchemaConverter.cs Allows opt-in schema promotion to FLBA(12) TIMESTAMP and refuses nested/non-timestamp requests.
src/EngineeredWood.Parquet/Parquet/Data/ArrowSchemaConverter.cs Adds read-side schema mapping and output-kind selection for the extended carrier.
src/EngineeredWood.Parquet/Parquet/Data/ArrowArrayBuilder.cs Adds narrowing/rescaling support for extended carrier alongside existing INT96 conversion path.
src/EngineeredWood.Parquet/Parquet/BufferedParquetWriter.cs Ensures buffered writer encodes promoted timestamps to the 12-byte storage type consistently.
src/EngineeredWood.Parquet/Parquet/BloomFilterPredicateEvaluator.cs Enables temporal-literal bloom probing and adds extended-carrier byte encoding for bloom keys.
src/EngineeredWood.Parquet/EngineeredWood.Parquet.csproj Adds Clast.DatabaseDecimal dependency for Int128 polyfill on netstandard2.0.
README.md Documents experimental diagnostics and the extended-precision timestamp feature and read/write controls.
doc/parquet-extended-precision-timestamps.md New design record documenting spec status, byte-order risk, mapping choices, and implementation touchpoints.
doc/known-issues.md Records limitations and future constraints for the extended timestamp carrier.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +317 to +321
default:
// A tick is 100 ns, so every DateTimeOffset lands exactly on a nanosecond count.
count = ticks * 100;
return true;
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Correct, and the consequence is exactly as you describe. Fixed in d051d25.

Confirmed the arithmetic first: year 9999 is 2,534,023,007,999,999,999 ticks since the epoch, ×100 =
2.53e20 nanoseconds, and long holds 9.22e18. It wrapped to -4852116231933722724. As a probe
that means asking whether some other timestamp entirely is present, and "absent" is what prunes — so
a file whose column genuinely holds that value could lose it.

Worth noting where it is and is not reachable: not through this library's own writer, because Arrow
timestamps are int64 and such a value cannot be expressed in the first place. But perfectly
reachable in a file parquet-java writes once the proposal lands, which is precisely the file this
carrier exists to read.

The conversion now happens in Int128 and has moved to ExtendedTimestamp, where the rest of the
unit arithmetic already lived. An INT64 column range-checks the result and declines rather than
probing — a value that type cannot hold is not in it — and the carrier does the same against ±2^95.

TIME(NANOS) keeps its 64-bit multiply, and now carries a comment saying why rather than leaving the
asymmetry unexplained: a time of day is at most 8.64e13 nanoseconds.

Comment on lines +441 to +445
/// <summary>
/// Maps a TIMESTAMP-annotated FIXED_LEN_BYTE_ARRAY(12) column per
/// <see cref="ParquetReadOptions.ExtendedTimestampOutput"/>. Unlike INT96, this carrier declares its
/// own unit, so the default keeps it and only the option to rescale or decline is a choice.
/// </summary>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Correct — the doc was stale and I missed it. Fixed in d051d25.

That comment was accurate when written: the default was Timestamp, which keeps the declared unit.
It changed to TimestampMicroseconds one commit later, when adding the conformance fixture to
parquet-testing broke the corpus sweep — a plain ParquetFileReader could not read a valid,
spec-conforming file, because year 9999 in nanoseconds does not fit an Arrow int64 timestamp. The
XML doc did not follow.

It now states which mode rescales and why, rather than describing the behaviour that was replaced.

Comment on lines +183 to +197
var data = array.Data;
var source = MemoryMarshal.Cast<byte, long>(
data.Buffers[1].Span.Slice(data.Offset * sizeof(long), array.Length * sizeof(long)));

var encoded = new byte[array.Length * ByteWidth];
for (int i = 0; i < array.Length; i++)
{
Write((Int128)source[i], encoded.AsSpan(i * ByteWidth, ByteWidth));
}

// Offset 0: the slice above has already applied it, and carrying it forward would double-count.
var newData = new ArrayData(
new FixedSizeBinaryType(ByteWidth), array.Length, data.NullCount, offset: 0,
[data.Buffers[0], new ArrowBuffer(encoded)]);
return ArrowArrayFactory.BuildArray(newData);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Correct, and it turned out worse than described. Fixed in d051d25.

I reproduced it before fixing: a three-row slice starting at row 2 of a five-row column, nulls at
absolute rows 0 and 3.

Expected 30, null, 50
Actual null, 40, 50

So both the values and the nulls moved — and the file was well-formed either way, which is the
part that makes it worth catching now.

Reachable through BufferedParquetWriter, which takes sliced arrays as they come: it tracks
Data.Offset rather than compacting. ParquetFileWriter calls CompactSlicedColumns first, which is
why the streaming path was unaffected and why nothing existing caught this.

The validity bitmap is now rebuilt to match the compacted values. One subtlety worth recording: the
rebuild is not skipped when the null count is zero. A bitmap can be present and fully set across
the slice while holding zeros outside it, and reading that at offset 0 would invent nulls.

Covered at both levels — a unit test on EncodeColumn and an end-to-end round trip through the
buffered writer — since the unit test alone would not have shown that the buffered path is the one
that reaches it.

Copilot's review of #217: three comments, all three correct.

THE SLICE BUG WAS THE WORST. ExtendedTimestamp.EncodeColumn sliced the VALUE
buffer by Data.Offset and then handed back the caller's validity bitmap alongside
offset 0 -- so bit i was read where bit (offset + i) was meant, and every null
moved. Measured on a three-row slice of a five-row column: expected 30, null, 50;
got null, 40, 50. Both the values and the nulls, and a perfectly well-formed file
either way.

Reachable through BufferedParquetWriter, which takes sliced arrays as they come:
it tracks Data.Offset rather than compacting, unlike ParquetFileWriter, whose
CompactSlicedColumns runs first and hid the problem on that path. The bitmap is
now rebuilt to match the compacted values -- and NOT skipped when the null count
is zero, because a bitmap can be fully set across the slice while holding zeros
outside it, which read at offset 0 invents nulls.

NANOSECONDS WRAPPED AT THE END OF THE SQL RANGE. Converting ticks to nanoseconds
as `ticks * 100` in 64 bits overflows: year 9999 is 2.53e20 nanoseconds and a long
holds 9.22e18. It wrapped to -4852116231933722724. As a bloom-filter probe that
means asking whether some OTHER timestamp is present, and being told "absent" is
what prunes a row group -- so a file whose column genuinely holds that value could
lose it. Not reachable through our own writer, since Arrow cannot express such a
value in the first place, but perfectly reachable in a file parquet-java wrote.

The conversion now happens in Int128 and moves to ExtendedTimestamp, where the
rest of the unit arithmetic already lives. An INT64 column range-checks the result
and declines rather than probing: a value that type cannot hold is not in it. The
carrier does the same against +/-2^95. TIME(NANOS) keeps its 64-bit multiply and
now says why -- a time of day is at most 8.64e13 nanoseconds.

THE DOC WAS STALE. MakeExtendedTimestampArrowType still said the default keeps the
file's declared unit. That stopped being true when the default became
TimestampMicroseconds, and I missed the comment.

Each fix verified by reverting it: the two correctness fixes fail 2 of the 57
carrier tests when undone.

Parquet suite 1113 passed / 37 skipped on net10.0, 1107 / 37 on net472; solution
builds clean including the net10.0 AOT/trim gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

2 participants