Add file with an FLBA(12) TIMESTAMP column - #123
Open
divjotarora wants to merge 2 commits into
Open
Conversation
Co-authored-by: Isaac
divjotarora
force-pushed
the
flba12-timestamp
branch
from
August 19, 2026 14:45
699fc5a to
6f41d6d
Compare
emkornfield
reviewed
Aug 20, 2026
emkornfield
requested changes
Aug 20, 2026
divjotarora
force-pushed
the
flba12-timestamp
branch
from
August 20, 2026 16:47
1fbdbd0 to
2988135
Compare
emkornfield
approved these changes
Aug 20, 2026
CurtHagenlocher
added a commit
to clast-project/engineered-wood
that referenced
this pull request
Sep 5, 2026
…12), behind EWPARQUET0004 (#217) * feat(parquet): decode the 96-bit extended-precision timestamp carrier 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> * feat(parquet): read TIMESTAMP on FIXED_LEN_BYTE_ARRAY(12) 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> * feat(parquet)!: default extended timestamps to microseconds, which cannot 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> * feat(parquet): statistics for the extended-precision timestamp carrier 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> * feat(parquet): write TIMESTAMP on FIXED_LEN_BYTE_ARRAY(12) 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> * feat(parquet): let temporal predicates probe bloom filters 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> * docs(parquet): record the extended-timestamp decisions, and the diagnostics 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> * fix(parquet): honour slice offsets and stop nanoseconds wrapping 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> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: CurtHagenlocher <904803+CurtHagenlocher@users.noreply.github.com>
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
2 participants
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.
This PR adds a test file that contains three columns with physical type
FIXED_LEN_BYTE_ARRAY(12)and logical typeTIMESTAMPmillis/micros/nanos. This file is intended to be used to test apache/parquet-format#600.