From 6b686e62f97646f45ad94f80193c72cdeb1a7ebd Mon Sep 17 00:00:00 2001 From: Per Nyfelt Date: Sun, 18 Oct 2026 15:17:26 +0200 Subject: [PATCH 1/4] Fix Parquet 0.8.0 type fidelity and selective reads --- matrix-parquet/build.gradle | 4 + matrix-parquet/readme.md | 90 +- matrix-parquet/release.md | 17 +- .../req/optimized-parquet-review.md | 6 +- matrix-parquet/req/v0.8.0-fixes.md | 970 ++++++++++++++++++ .../matrix/parquet/ColumnMaterializer.groovy | 33 +- .../matrix/parquet/ColumnTypePolicy.groovy | 10 +- .../matrix/parquet/DatasetBatches.groovy | 9 +- .../matrix/parquet/DecimalSchemaPolicy.groovy | 11 +- .../se/alipsa/matrix/parquet/GlobSpec.groovy | 70 ++ .../matrix/parquet/HttpRangeInputFile.groovy | 2 +- .../matrix/parquet/MatrixParquetReader.groovy | 203 ++-- .../matrix/parquet/MatrixParquetWriter.groovy | 176 ++-- .../alipsa/matrix/parquet/NestedPaths.groovy | 47 + .../matrix/parquet/ParquetBatches.groovy | 19 +- .../matrix/parquet/ParquetPredicate.groovy | 40 +- .../matrix/parquet/ParquetValueWriter.groovy | 107 +- .../matrix/parquet/ParquetWriteOptions.groovy | 77 +- .../matrix/parquet/TemporalUnits.groovy | 27 + .../groovy/MatrixParquetBigIntegerTest.groovy | 4 +- .../groovy/ParquetDatasetAndHttpTest.groovy | 35 + .../matrix/parquet/GroupValueReader.groovy | 90 ++ .../matrix/parquet/ParquetFixFixtures.groovy | 26 + .../matrix/parquet/ParquetInt96Test.groovy | 43 + .../parquet/ParquetLayoutBenchmarkTest.groovy | 14 + .../ParquetMapCompatibilityTest.groovy | 37 + .../parquet/ParquetNestedTypesTest.groovy | 79 ++ .../parquet/ParquetNestedWriteTest.groovy | 35 + ...arquetPredicateRepresentabilityTest.groovy | 80 ++ .../parquet/ParquetReadBenchmarkTest.groovy | 2 +- .../matrix/parquet/ParquetReaderIoTest.groovy | 78 ++ .../ParquetReviewRegressionTest.groovy | 12 + .../ParquetTemporalPrecisionTest.groovy | 92 ++ .../parquet/ParquetTypePolicyTest.groovy | 10 +- .../matrix/parquet/ParquetUnsignedTest.groovy | 43 + .../ParquetWriterOptionsValidationTest.groovy | 49 + 36 files changed, 2291 insertions(+), 356 deletions(-) create mode 100644 matrix-parquet/req/v0.8.0-fixes.md create mode 100644 matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/GlobSpec.groovy create mode 100644 matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/NestedPaths.groovy create mode 100644 matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/TemporalUnits.groovy create mode 100644 matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/GroupValueReader.groovy create mode 100644 matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetFixFixtures.groovy create mode 100644 matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetInt96Test.groovy create mode 100644 matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetMapCompatibilityTest.groovy create mode 100644 matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedTypesTest.groovy create mode 100644 matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedWriteTest.groovy create mode 100644 matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetPredicateRepresentabilityTest.groovy create mode 100644 matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetReaderIoTest.groovy create mode 100644 matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy create mode 100644 matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetUnsignedTest.groovy create mode 100644 matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetWriterOptionsValidationTest.groovy diff --git a/matrix-parquet/build.gradle b/matrix-parquet/build.gradle index 094566f83..c3763d237 100644 --- a/matrix-parquet/build.gradle +++ b/matrix-parquet/build.gradle @@ -49,6 +49,10 @@ dependencies { } test { + ['parquet.baseline', 'parquet.layoutBenchmark', 'parquet.datasetBenchmark', 'parquet.benchmarkRows'].each { key -> + def value = providers.gradleProperty(key).orNull + if (value != null) { systemProperty key, value } + } testLogging.showStandardStreams = true useJUnitPlatform() } diff --git a/matrix-parquet/readme.md b/matrix-parquet/readme.md index 9d32c5c9c..bb33120cb 100644 --- a/matrix-parquet/readme.md +++ b/matrix-parquet/readme.md @@ -32,8 +32,11 @@ assert selected.column('id') == [2L, 3L] Projection accepts top-level fields in output order. Filter-only columns are read internally. `Predicates.eq/ne/lt/le/gt/ge` have Number, String, Boolean, LocalDate, LocalDateTime, Instant, LocalTime and byte[] overloads. Use `isNull`, `isNotNull`, `and`, `or` and `not` for composition. -Values must match the file's logical schema. Integer overflow, fractional integer values, -lossy decimal rescaling, and timestamp/time precision loss are rejected before reading pages. +Values must match the file's logical schema. Unrepresentable numeric bounds, fractional integer values, NaN/Infinity, lossy decimal +rescaling, and finer timestamp/time bounds use exact residual evaluation instead of pushdown. +For example, `Predicates.gt('i', 1.5G)` on an integer column correctly selects integers 2 and above. +An `and` can still push its safe child; `or` requires both children to be safe supersets, and `not` +is always residual. Repeated/nested predicates are rejected. Missing indexes fall back to scans; corrupt data fails. String ordering uses unsigned UTF-8 bytes, matching Parquet. Unannotated BINARY predicates take @@ -72,7 +75,8 @@ IDs remain valid when filters prune their groups; such groups produce no batch. ordinal row seeking API. Datasets validate every footer before returning rows and require identical logical fields, -order, decimal precision/scale, timestamp units/UTC flags and Matrix type metadata. Duplicate +order, decimal precision/scale, timestamp units/UTC flags and Matrix type metadata, including +`matrix.nestedTypes`. Duplicate canonical files and empty selections fail. Directory reads include direct `.parquet` children; globs are explicit and can select deeper paths. Compatible Matrix indices are restored when every shard has the same index metadata and projection retains its columns. A source column @@ -81,6 +85,17 @@ use independent streams and converters, reuse each parsed footer, prefetch at mo worker, and preserve file/group order. `rowGroups` is for one file; use `rowGroupsByFile: [(file.canonicalPath): [1, 0]]` for explicit dataset group selections. +Directory results use the directory name; list and glob results use the first selected filename +without `.parquet`. `matrixName` overrides either default, including batches. + +Globs match paths relative to the literal root. `train/*.parquet` visits only direct children; +`train/**/*.parquet` permits arbitrary depth. `{a,b/c}` alternatives also permit variable depth. +Fixed-depth patterns bound the filesystem walk. A missing root or an empty match raises a contextual +`IllegalArgumentException`. POSIX backslashes escape glob characters (`train/x\\*.parquet` in a +Groovy string selects the literal `x*.parquet`); Windows backslashes are converted to `/` separators, +and glob escaping is unavailable there. `C:*.parquet` drive-relative patterns are rejected; use +`C:/data/*.parquet`. Ordinary `read(String)` paths never expand globs. + ## HTTP and Hub URIs ```groovy @@ -102,9 +117,11 @@ propagate. Range caching is bounded by `rangeCacheBytes`; individual requests ar reads prefetch at most 64 KiB. Validated redirect targets are reused, and fully consumed range responses permit HTTP keep-alive reuse. ETag/Last-Modified validators and total length detect changes when available; same-length changes without validators cannot be detected reliably. +Strong ETags use `If-Match`; weak `W/` tags use `If-Unmodified-Since` when Last-Modified is present. +Response ETags are still compared on every fetch to detect changes. Redirects are bounded and HTTPS downgrades rejected. Connection failures before response headers are retried once; malformed responses and body failures propagate. Streams are spooled to -temporary files; byte arrays use a seekable memory input. +owner-only temporary files on POSIX; byte arrays use a seekable memory input. Hub dataset URI parsing is pinned to huggingface_hub v2.2.0. Revisions follow the repository: `hf://datasets/owner/repo@revision/path`. Encode ordinary revision slashes as `%2F`; special @@ -161,6 +178,7 @@ not enable a filter by themselves. Column settings use physical primitive paths | Option | Default | Meaning | |---|---|---| | compressionCodec | SNAPPY | Codec name/CompressionCodecName; tested SNAPPY, ZSTD, GZIP, UNCOMPRESSED | +| timestampUnit | MICROS | MICROS truncates sub-microsecond timestamp digits; NANOS preserves them within int64 range | | inferPrecisionAndScale | true | Infer exact decimal settings when no explicit settings apply | | precision / scale | null / null | Uniform positive precision and nonnegative scale, provided together | | decimalMeta | empty | Logical field path to `[precision, scale]`; overrides uniform settings | @@ -172,8 +190,8 @@ not enable a filter by themselves. Column settings use physical primitive paths | writerVersion | V1 | V1 or V2 writer encodings and page layout | | columnIndexTruncateLength | 64 | Positive binary page-bound truncation bytes | | statisticsTruncateLength | 2147483647 | Positive row-group binary statistics truncation bytes | -| bloomFilterEnabled | empty, disabled | Primitive path to Boolean override | -| bloomFilterNDV | empty | Primitive path to positive expected distinct-value count | +| bloomFilterEnabled | empty, disabled | Primitive path to Boolean override; SPI also accepts trimmed true/false strings | +| bloomFilterNDV | empty | Primitive path to positive integral long count; SPI accepts integral numbers and numeric strings, rejecting fractions/overflow | | sizeStatisticsEnabled | true | Collect size statistics | Files are written to a temporary sibling and atomically replace the destination after successful @@ -187,8 +205,9 @@ these are unpublished temporary output and can be removed once no writer is usin JSON and ENUM annotations decode as UTF-8 String; BSON remains binary. Strings are STRING-annotated recursively; byte[] stays raw BINARY. Byte/Short use signed width annotations, UUID uses FIXED[16], LocalTime uses TIME(NANOS), and Instant uses adjusted -TIMESTAMP(MICROS). Instants with sub-microsecond precision or int64-microsecond overflow fail; -Instant.MIN/MAX are unsupported. Character and enums become String (`name()` for enums). +TIMESTAMP(MICROS) by default. Sub-microsecond digits are truncated consistently for Instant, +LocalDateTime and Timestamp; int64 overflow fails with column/row context. Instant.MIN/MAX +are unsupported. Character and enums become String (`name()` for enums). ZonedDateTime/OffsetDateTime/custom objects require explicit conversion to Instant/String/Map. Date/Timestamp subclasses must share a single encoding policy; mixed units fail before output. @@ -205,6 +224,61 @@ names (for example `nested.element.amount`). With inference disabled every decim All-null BigDecimal infers `(1, 0)`. There is no DOUBLE fallback. Explicit decimal conversion obeys roundingMode. See [migration notes](release.md) for changed 0.7.x behavior. +Nested collections with different concrete implementations share one LIST/MAP policy. Map values +with different logical policies become STRUCT fields and therefore require String keys. Maps in +lists become STRUCTs when every key is a String; Integer or other supported primitive keys retain +MAP encoding. Readers derive key/value roles from the schema, including nonstandard field names, +legacy outer MAP_KEY_VALUE groups and key-only maps (whose values become null). String-declared columns accept any CharSequence, including GString, and write text. + +`matrix.nestedTypes` is a JSON footer object mapping logical leaf paths to canonical Matrix class +names. It preserves nested Instant, Date, Timestamp, sql.Date, Byte, Short and BigInteger values +in lists, map keys/values and struct fields. LocalTime/Time are distinguished by TIME annotations. +Older readers ignore the new key; files without it retain schema inference. Unknown classes and +incompatible entries are ignored, with no external class loading. + +| Parquet / Matrix type | Read or write behavior | +|---|---| +| UINT8 / UINT16 | Integer; residual predicates | +| UINT32 | Long with unsigned widening; residual predicates | +| UINT64 | BigInteger with unsigned widening; residual predicates | +| Instant / LocalDateTime / Timestamp | TIMESTAMP(MICROS, adjusted=true) by default; truncate sub-microsecond digits; NANOS opt-in | +| External INT96 | Read as UTC instant, returned as LocalDateTime in reader zone; compatible metadata restores Instant/Timestamp/Date; residual predicates; writer never emits INT96 | +| FLOAT16 | Contextual unsupported-type error | + +### Timestamp precision + +The default `timestampUnit: 'MICROS'` discards sub-microsecond digits downward in time for +LocalDateTime, Timestamp and Instant, including before 1970. LocalDateTime is converted through +the writer zone first. This chiefly affects Linux JDK 15+ nanosecond clocks and Windows 100 ns +clocks; measured macOS clocks provide microseconds. Ordinary `now()` writes therefore succeed. + +`timestampUnit('NANOS')` preserves full precision, but Spark-family readers may not support +TIMESTAMP(NANOS). The int64 NANOS range is 1677-09-21T00:12:43.145224192Z through +2262-04-11T23:47:16.854775807Z; values outside it fail. MICROS covers about ±292,000 years +and is suitable for wider dates. Both units reject overflow with column/row context. + +Under MICROS, filtering with the original sub-microsecond value finds no equal row because +predicates compare exactly against the truncated stored value. Truncate the filter value too, +or write NANOS: + +```groovy +import java.time.Instant +import java.time.temporal.ChronoUnit +import se.alipsa.matrix.core.Matrix +import se.alipsa.matrix.parquet.* + +Instant original = Instant.EPOCH.plusNanos(123456789) // also occurs with Instant.now() +def data = Matrix.builder().columns(ts: [original]).build() +byte[] micros = MatrixParquetWriter.builder(data).writeBytes() +assert MatrixParquetReader.builder().filter(Predicates.eq('ts', original)).read(micros).rowCount() == 0 +assert MatrixParquetReader.builder().filter(Predicates.eq('ts', original.truncatedTo(ChronoUnit.MICROS))) + .read(micros).rowCount() == 1 +byte[] nanos = MatrixParquetWriter.builder(data).timestampUnit('NANOS').writeBytes() +assert MatrixParquetReader.builder().filter(Predicates.eq('ts', original)).read(nanos).rowCount() == 1 +// SPI equivalent, with the same default MICROS if omitted: +data.write([timestampUnit: 'NANOS'], new File('timestamps.parquet')) +``` + ## CDC output, tests and benchmarks Native CDC writing is deferred: [parquet-java #3818](https://github.com/apache/parquet-java/pull/3818) diff --git a/matrix-parquet/release.md b/matrix-parquet/release.md index 25e394e0c..d2ccbbf8b 100644 --- a/matrix-parquet/release.md +++ b/matrix-parquet/release.md @@ -19,10 +19,16 @@ Complete defaults, supported types and end-to-end examples are in [the module gu | Unadjusted timestamps | Local wall-clock time preserved | No bug-preserving flag; applications needing the former shift can explicitly convert the local value through UTC into the desired zone | | Integer-backed DECIMAL | BigDecimal with declared scale | No bug-preserving flag; explicit `unscaledValue()` obtains prior raw numeric value | | UUID annotation | UUID | No text-decoding compatibility flag; explicit UUID/string/byte conversion at application boundary | -| FLOAT16 / INT96 unsupported cases | Contextual unsupported-type error | No UTF-8 fallback; convert these types explicitly before reading | +| FLOAT16 | Contextual unsupported-type error | No UTF-8 fallback; convert explicitly before reading | +| External INT96 | Read-only UTC timestamp support, returned in reader zone or as compatible metadata type | Residual filters; writer never emits INT96 | +| Unsigned integers | UINT8/16 → Integer, UINT32 → Long, UINT64 → BigInteger | Correct unsigned values; residual-only predicates | +| Timestamp precision | LocalDateTime/Timestamp/Instant default to TIMESTAMP(MICROS), consistently truncating sub-microsecond digits; Instant.now() works on Linux/Windows | timestampUnit('NANOS') preserves precision within int64 range; intentional exception to lossless-decimal default, bounded below 1 µs | +| Dataset names | Directory name for directory reads; first filename without extension for lists/globs | matrixName overrides collected/batched names | +| String-declared GString | CharSequence values accepted and converted to text | Other declared-type checks remain strict | +| Nested type metadata | matrix.nestedTypes preserves canonical nested leaf classes | Older readers ignore key; missing/unknown/incompatible metadata retains inference | | Byte / Short fallback | Signed INT32 with INT(8/16), restored as Byte/Short | Legacy unannotated text stays String; explicitly convert to String before writing to retain text output | | LocalTime fallback | TIME(NANOS), restored as LocalTime using new-format metadata | Legacy text stays String; explicit String conversion retains text output | -| Instant fallback | TIMESTAMP(MICROS, adjusted=true), restored as Instant using new-format metadata | Legacy text stays String; reject sub-microsecond precision loss and int64-micros overflow; explicitly convert to String for text output | +| Instant fallback | TIMESTAMP(MICROS, adjusted=true), restored as Instant using new-format metadata | Legacy text stays String; truncate sub-microsecond digits under MICROS, reject int64 overflow; NANOS preserves precision | | UUID writer fallback | FIXED[16] + UUID, restored as UUID | Legacy text stays String; explicit String conversion retains text output | | Character fallback | STRING, returned as String | Normalize emitted metadata to String; Character identity is not preserved | | Enum fallback | STRING using enum name(), returned as String | Normalize metadata to String; convert explicitly for former overridden toString() output | @@ -42,7 +48,7 @@ Legacy Matrix text compatibility applies recursively; new raw binary metadata re The checked-in original 0.7.x string fixture predates these writer changes. Legacy byte[] recovery claims were narrowed after an end-to-end check found InvalidSchemaException in this checkout. -External INT96/FLOAT16 raise contextual unsupported-type errors. External TIME still returns +External INT96 decodes as adjusted UTC timestamps; FLOAT16 raises contextual unsupported-type errors. External TIME still returns java.sql.Time at millisecond precision; exact predicate filtering preserves original precision. Mixed Date/Timestamp policies are now rejected before writing, preventing the prior 1000× unit corruption. New Object/supertype values are resolved consistently for schema and metadata. @@ -58,6 +64,11 @@ the link. Local file writes require same-directory atomic replacement support; u or FUSE filesystems raise `AtomicMoveNotSupportedException` and leave existing output unchanged. HTTP bulk requests reuse resolved targets and a separately bounded cache. +October review fixes also add exact residual fallback for unrepresentable predicate bounds, +partial AND pruning, canonical container inference, full nested error paths, strict bloom/codec +validation, secure read spooling, bounded cross-platform glob parsing, weak-ETag preconditions, +legacy MAP layouts, and indexed column access in the write loop; dead reader/POJO paths were removed. + Native CDC writing remains deferred to upstream parquet-java #3818; the Python rewrite preserves custom metadata. Hadoop stays as a runtime dependency because codec initialization still loads it. diff --git a/matrix-parquet/req/optimized-parquet-review.md b/matrix-parquet/req/optimized-parquet-review.md index a0f21a03e..6b5138f42 100644 --- a/matrix-parquet/req/optimized-parquet-review.md +++ b/matrix-parquet/req/optimized-parquet-review.md @@ -20,10 +20,10 @@ on `feature/optimized-parquet-review`, based on the original PR commit eee2b16e. - 9.6 [x] Support sourceFileColumn for local single-file reads and batches. Restore common compatible indices for both collected and batched dataset reads, including truncated batches; omit indices when projection removes their columns or shards disagree on index metadata. -- 9.7 [x] Resolve each top-level column's validated type policy once and carry it in WriteSchema +- 9.7 [x] (Partially completed here; finished in [v0.8.0-fixes §6](v0.8.0-fixes.md#6-full-logical-paths-in-nested-errors-completes-review-97).) Resolve each top-level column's validated type policy once and carry it in WriteSchema through decimal/schema construction and metadata emission. Remove obsolete duplicate inference helpers and report full nested logical paths; test nested error context. -- 9.8 [x] Treat null option-map values as absent consistently, preserving Boolean/default/map +- 9.8 [x] (Partially completed here; finished in [v0.8.0-fixes §7.1](v0.8.0-fixes.md#7-writer-option-validation).) Treat null option-map values as absent consistently, preserving Boolean/default/map values and nullable AUTO semantics. Reject null bloom entries with contextual validation. - 9.9 [x] Create dataset worker threads through a daemon ThreadFactory. Assert their daemon status while an iterator remains open; explicit close is still required for prompt cleanup. @@ -33,7 +33,7 @@ on `feature/optimized-parquet-review`, based on the original PR commit eee2b16e. retain the pinned client URL cases. Test nested paths and encoded revision traversal. - 9.12 [x] Reject unrecoverable legacy [B metadata only for required output/filter columns. Test successful projection around the damaged column and rejection when a filter needs it. -- 9.13 [x] Correct the reader's memory/batch/cache comments and writer option documentation, +- 9.13 [x] (Partially completed here; finished in [v0.8.0-fixes §14.1](v0.8.0-fixes.md#14-documentation).) Correct the reader's memory/batch/cache comments and writer option documentation, including literal V1 defaults; update source/index/transport behavior in the module guide. ## Verification diff --git a/matrix-parquet/req/v0.8.0-fixes.md b/matrix-parquet/req/v0.8.0-fixes.md new file mode 100644 index 000000000..802206f54 --- /dev/null +++ b/matrix-parquet/req/v0.8.0-fixes.md @@ -0,0 +1,970 @@ +# Matrix-Parquet v0.8.0 Fix Plan + +## Scope + +This plan fixes every finding from the October 2026 `matrix-parquet` review. That includes +items that `req/v0.7.0-fixes.md` and `req/optimized-parquet-review.md` mark as complete but that +are only partly done. Every finding was confirmed against the code at commit `7afb1231`, either by +reading the code or with a throwaway probe test. Three findings could not be fully confirmed and +are marked **(unverified)**. + +`matrix-parquet` is already `0.8.0-SNAPSHOT` and `v0.8.0` is unreleased. These fixes therefore go +into the existing `v0.8.0 (unreleased)` section of `release.md` and need no version bump. No git +tag may be created. + +A task is done only when its tests pass and the exact commands appear in the verification record +at the end of this file, or in the PR description. + +Finding-to-section map: + +| Finding | Section | +|---|---| +| Unsigned INT32/INT64 read as signed values | 2 | +| Unrepresentable predicate values throw `ArithmeticException` / lossy-predicate errors | 3 | +| Map values of different concrete List/Map/CharSequence classes become STRUCTs or fail | 4.1 | +| List elements that are maps with non-String keys cannot be written | 4.2 | +| `GString` values rejected in String-declared columns | 4.3 | +| Nested values lose their Matrix types (Instant, Date, Byte, BigInteger, ...) | 5 | +| Partial 9.7: nested error messages omit the full logical path | 6 | +| Partial 9.8: bloom option values are not type-validated and errors lack column context | 7.1 | +| `compressionCodec(String)` is locale-sensitive and null-unsafe | 7.2 | +| Sub-microsecond timestamps handled inconsistently (`LocalDateTime`/`Timestamp` truncated, `Instant` rejected); resolved by decision D: truncate all three under default `MICROS`, opt-in `NANOS` | 8.1 | +| `/` creates a BigDecimal per temporal value in the write loop | 8.2 | +| Per-cell `matrix[i, col]` gives O(rows x cols^2) writes | 8.3 | +| `read(InputStream)`/URL download temp file is world-readable | 9.1 | +| Dataset reads are named after the first shard including `.parquet` | 9.2 | +| `readGlob` breaks on Windows and walks the entire subtree **(unverified on Windows)** | 9.3 | +| `If-Match` sent with weak ETags causes 412 mid-read **(unverified against real servers)** | 10 | +| MAP key/value field names are hard-coded when reading; legacy outer `MAP_KEY_VALUE` groups read as structs **(unverified with real legacy files)** | 11 | +| Plan review: `and` loses all pruning when one child is not pushable | 3.6 | +| Plan review: benchmark classes are not enabled by `-PrunSlowTests=true` | 1.4, 8.4, 13.4, 15.2 | +| INT96 timestamps unsupported | 12 | +| Dead production code (Group reader helpers, POJO struct branches, unused ThreadLocal) | 13 | +| Partial 3.8/9.13: stale writer GroovyDoc ("double storage") | 14 | + +## Compatibility Decisions + +### A. Unrepresentable predicate values (section 3) + +1. Keep throwing, but throw `IllegalArgumentException` instead of `ArithmeticException`. + Rejected: `x > 1.5` on an INT32 column is a valid, well-defined predicate. +2. Treat a value that cannot be converted exactly as "not pushable". Keep the predicate as an + exact residual filter on the decoded value. **Recommended:** results are exact, and the only + cost is less pruning for these rare predicates. +3. Rewrite to the correct bound (`ceil` for `gt`/`ge`, `floor` for `lt`/`le`; `eq` matches + nothing) to keep pruning. Deferred: more code and more edge cases. Option 2 can be extended to + this later without changing behavior. + +Choose option 2. + +### B. Unsigned integer columns (section 2) + +UINT32 reads as `Long` and UINT64 reads as `BigInteger`. UINT8/UINT16 stay `Integer`, since every +value fits. This changes the reported column types for external unsigned files. 0.8.0 is +unreleased, and the old behavior returned wrong values, so no compatibility flag is offered. +Unsigned columns are filtered with residual predicates only, and are not pushed down until a test +proves that parquet-java uses unsigned comparators for record, statistics, dictionary and +column-index filtering. + +### C. Nested value types (section 5) + +1. Document the loss only. Rejected: a nested `Instant` silently becomes a zone-shifted + `LocalDateTime`. +2. **Recommended:** write a new footer key, `matrix.nestedTypes`. It is a JSON object that maps a + nested logical path (the same `col.element`, `col.key`, `col.value`, `col.field` paths used by + `decimalMeta`) to the canonical Matrix class name. Only write a path when its class differs + from the reader's default inference for that leaf. The reader passes these expected classes + to the nested converters. Files without the key keep today's inference. + +Choose option 2. Older readers ignore unknown keys, so the format stays backward compatible. + +### D. Sub-microsecond timestamps (section 8.1) — **decided by the maintainer, 2026-10-18** + +**Decision:** +- The default unit is microseconds: `timestampUnit: 'MICROS'`, written as + `TIMESTAMP(MICROS, adjusted=true)`. +- Sub-microsecond digits are truncated, without an error, for `LocalDateTime`, `Timestamp` and + `Instant` alike. +- The current `Instant` rejection is a bug and is removed. +- Full precision is available with the opt-in `timestampUnit('NANOS')`. + +**Context:** +- Before this change, `LocalDateTime` and `Timestamp` were silently truncated while `Instant` was + rejected (`ColumnTypePolicy.groovy:43`, `ParquetValueWriter.groovy:141-143`). +- Clock precision decides who is affected: + - macOS JDK 21 measured microseconds here (`2026-10-18T10:27:08.024084Z`), so `now()` values + there lose nothing. + - Since JDK 15 (JDK-8242504) the Linux system clock has nanosecond precision. This was not run + here. + - Windows clocks have 100 ns resolution. +- Under the old rule, `Instant.now()` columns therefore already failed on Linux. + +**Why truncate rather than reject:** +- A strict default would make ordinary `now()`-based writes fail on Linux and Windows servers. +- Microseconds are the widest-supported Parquet timestamp unit: Spark reads TIMESTAMP(NANOS) + poorly. +- Truncation drops less than one microsecond, and only where the platform clock is finer than + microseconds. + +**Exception to v0.7.0:** this is a deliberate exception to the "lossless by default" rule adopted +for decimals in `v0.7.0-fixes.md` (Compatibility Decision, option 2). Decimal rounding can change +values by whole units of the declared scale. Timestamp truncation is bounded below one +microsecond. + +**Truncation semantics:** +- Digits are always discarded downward in time, because `Instant`/`LocalDateTime` nanos are + non-negative, including before 1970. +- `LocalDateTime` is converted to an instant with the writer zone first, then truncated. +- int64 overflow is still rejected with column/row context under both units. Under `NANOS` the + representable range is only 1677-09-21T00:12:43.145224192Z to 2262-04-11T23:47:16.854775807Z. + +**Known consequence (documented, tested in §8.4):** under `MICROS`, filtering with the exact +sub-microsecond value that was written finds nothing. For example, after writing `Instant.now()`, +`eq('ts', sameInstant)` returns 0 rows, because the stored value was truncated and predicates are +evaluated exactly (decision A). This is logically correct but surprising, so the readme explains +it and shows `truncatedTo(ChronoUnit.MICROS)` and `timestampUnit('NANOS')` as the fixes. + +**Rejected alternatives (kept for future reviewers):** +1. Reject sub-microsecond values for all three classes (strict, lossless): breaks `now()` on Linux + and Windows. +2. Keep the inconsistent status quo (truncate two classes, reject `Instant`). +3. A strict opt-in flag (for example `rejectTimestampTruncation(true)`): deferred until a user asks + for it. It can be added later without changing the default. + +**Required documentation** so users and future reviewers are not surprised: +- GroovyDoc on `ParquetWriteOptions.timestampUnit`, `WriterBuilder.timestampUnit` and the + `MatrixParquetWriter` class Javadoc (§14.1); +- the readme type table and a dedicated "Timestamp precision" paragraph (§14.2); +- a `release.md` row (§14.3); +- a code comment at the truncation site that points to this decision. + +### E. INT96 timestamps (section 12) + +`release.md` currently documents INT96 as a contextual unsupported-type error. Change this to +read-only support: decode INT96 as an adjusted (UTC) instant. Return it as `LocalDateTime` in the +reader zone, the same as other adjusted timestamps. Return `Instant`/`Timestamp`/`Date` when +compatible Matrix metadata asks for them. The writer never emits INT96. FLOAT16 stays unsupported. + +## 1. Preparation + +1.1 [x] Confirm `matrix-parquet/build.gradle` is still `version = '0.8.0-SNAPSHOT'` and that +`release.md` still has `## v0.8.0 (unreleased)`. Do not create a git tag. + +1.2 [x] Create a branch (for example `parquet-v080-fixes`). Do not commit to `main` without +confirmation. + +1.3 [x] Add one regression test class per area, all in +`src/test/groovy/se/alipsa/matrix/parquet/` and package `se.alipsa.matrix.parquet` (so package-scope +helpers are reachable): `ParquetUnsignedTest`, `ParquetPredicateRepresentabilityTest`, +`ParquetNestedWriteTest`, `ParquetNestedTypesTest`, `ParquetWriterOptionsValidationTest`, +`ParquetTemporalPrecisionTest`, `ParquetReaderIoTest`, `ParquetMapCompatibilityTest`, +`ParquetInt96Test`. Add HTTP cases to the existing `ParquetDatasetAndHttpTest` server helpers. + +1.4 [x] Benchmark gating: `-PrunSlowTests=true` does **not** enable the benchmark classes. +- `ParquetLayoutBenchmarkTest` is gated by the system property `parquet.layoutBenchmark`. +- `ParquetReadBenchmarkTest` and `ParquetBaselineTest` are gated by `parquet.baseline`. +- `ParquetDatasetBenchmarkTest` is gated by `parquet.datasetBenchmark`. +- In `build.gradle`, `runSlowTests` only gates the CDC fixture task, and nothing forwards these + properties to the test JVM. + +Add a forwarding block to the `test` task in `matrix-parquet/build.gradle`: + +```groovy +['parquet.baseline', 'parquet.layoutBenchmark', 'parquet.datasetBenchmark', 'parquet.benchmarkRows'].each { key -> + def value = providers.gradleProperty(key).orNull + if (value != null) { systemProperty key, value } +} +``` + +Benchmark runs then use, for example, +`./gradlew :matrix-parquet:test --tests '*ParquetLayoutBenchmarkTest' -Pparquet.layoutBenchmark=true --rerun`. +The init scripts in `optimized-parquet-plan.md:776-782` remain valid. Record which mechanism was +used. A benchmark task only counts as verified when the test report shows the class **executed**, +not skipped. + +Always add `--rerun` (supported by the Gradle 9.4.0 wrapper) to benchmark commands. Without it, a +second run with unchanged inputs is `UP-TO-DATE`, and the previous report still says "executed" +even though nothing ran. Check the console for `> Task :matrix-parquet:test` without +`UP-TO-DATE`/`FROM-CACHE`, as well as the report. + +## 2. Unsigned Integer Columns + +2.1 [x] In `MatrixParquetReader.extractFieldTypes` (`MatrixParquetReader.groovy:1032-1051`), handle +`LogicalTypeAnnotation.IntLogicalTypeAnnotation` with `!isSigned()`: bit width 32 maps to `Long`, +64 maps to `BigInteger`, and 8/16 map to `Integer`. + +2.2 [x] In `decodePrimitive` (`:1092-1137`), before `narrowInteger`, widen unsigned values: +`Integer.toUnsignedLong(int)` for UINT32 and `new BigInteger(Long.toUnsignedString(long))` for +UINT64. Apply the same widening when `expected == null` (nested values), so lists and maps of +unsigned values are also correct. + +2.3 [x] In `ParquetBatches.compatibleMetadata` (`ParquetBatches.groovy:114-129`), add an explicit +`IntLogicalTypeAnnotation` branch before the physical fallback. Today `compatiblePhysicalMetadata` +only receives the physical type name, so it accepts `Byte`/`Short`/`Integer` for any INT32 +column, including unsigned ones. Rules: +- unsigned 8/16 → `Integer` only; +- unsigned 32 → `Long` only; +- unsigned 64 → `BigInteger` only; +- signed 8 → `Byte`/`Short`/`Integer`; +- signed 16 → `Short`/`Integer`; +- signed 32 → `Integer`; +- signed 64 → `Long`. + +Incompatible metadata falls back to inference as it does today. Without this, a re-annotated file +whose `matrix.columnTypes` says `Integer` for a UINT32 column gives an `Integer`-typed column that +holds `Long` values, or wrong signed values if the widening is skipped. + +2.4 [x] In `ParquetPredicate.pushdown` (`ParquetPredicate.groovy:95-130`), return `null` for unsigned +INT32/INT64 columns. In `physicalValue`, do not attempt `intValueExact`/`longValueExact` for +unsigned columns. `TypeCheck` keeps accepting any `Number`. + +2.5 [x] Tests in `ParquetUnsignedTest`. Write external files with `ExampleParquetWriter`: +`INT(32,false)` holding `3_000_000_000` (stored as `(int) 3_000_000_000L`), `INT(64,false)` holding +`-1L` (= 2^64-1), `INT(8,false)` holding 255, a LIST of `INT(32,false)`, and a signed control +column `s16 INT(16,true)` holding `-123`. Assert: +- the values and Matrix column types; +- `Predicates.gt('u32', 2_000_000_000L)` and `Predicates.eq('u64', 18446744073709551615G)` return + the correct rows; +- batches and `limit` behave the same; +- wrong-metadata guard. Write the same five columns with `withExtraMetaData`: + - `matrix.columnTypes` = `java.lang.Integer,java.lang.Long,java.lang.Byte,java.util.List,java.lang.Short`, with + **exactly one entry per column**. `ParquetBatches.groovy:79` ignores the metadata entirely + when `declared.size() != full.fieldCount`, so a shorter list would make the test pass without + §2.3; + - `matrix.parquet.version` = `2`. Without it, `ParquetBatches.groovy:62` switches to legacy + mode, which changes `binaryAsString` and runs `validateLegacyBinary`. + + Assert: + - `u32` is `Long` = 3000000000, `u64` is `BigInteger` = 2^64-1, `u8` is `Integer` = 255; + - each wrong declaration was rejected and fell back to inference, not silently applied; + - control, in the **same file**: `result.type('s16') == Short` and the value is `(short) -123`. + Inference never produces `Short` (an INT32 column infers `Integer`), so this proves the + metadata was parsed and applied per column while the unsigned entries were rejected. An + `Integer` control on a signed INT32 column would not prove this, because inference gives + the same answer. + +## 3. Predicates With Unrepresentable Values + +3.1 [x] Make `ParquetPredicate.physicalValue` (`:159-183`) return a dedicated `NOT_PUSHABLE` +sentinel instead of throwing in these cases: +- decimal `setScale(..., UNNECESSARY)` fails or precision overflows (`:163-164`); +- `intValueExact`/`longValueExact` fails (`:180-181`, `:117`, `:121`); +- `new BigDecimal(value.toString())` fails for NaN/Infinity; +- TIME/TIMESTAMP values are lossy for the column unit (`:171`, `:177`). + +Keep the `Math.toIntExact` date overflow and the timestamp `multiplyExact` overflow as +`NOT_PUSHABLE` as well. + +3.2 [x] In `bind` (`:145-157`), keep calling `TypeCheck.validate`, but stop treating +`physicalValue` failures as errors. Bind must not throw for any value that `TypeCheck` accepts. + +3.3 [x] Make `physicalValue` return exactly the Java type that the physical column filter needs: +- `Integer` for INT32, including DATE, TIME(MILLIS), INT(8/16/32) and INT32-backed DECIMAL; +- `Long` for INT64, including TIME(MICROS/NANOS), TIMESTAMP and INT64-backed DECIMAL; +- the existing String/byte[] values for BINARY. + +Today it returns a `BigInteger` for decimals (`:165`) and a `Long` for TIME(MILLIS) (`:172`). The +`intValueExact`/`longValueExact` at `:117`/`:121` are what currently fix this up, so do not delete +them until this step is done. + +Do the narrowing inside `physicalValue` with `intValueExact`/`longValueExact`, and return +`NOT_PUSHABLE` when it fails, for example a decimal unscaled value that does not fit the physical +width. After that, `pushdown` uses the value directly and returns `null` for `NOT_PUSHABLE`. + +3.4 [x] Confirm that `ParquetPredicate.compare` (`:62-91`) gives exact results for the residual +cases: BigDecimal vs scaled decimal, fractional vs INT32, large Long vs INT32, NaN, and +`Instant`/`LocalTime` with finer precision than the column unit. + +3.5 [x] Tests in `ParquetPredicateRepresentabilityTest`, each comparing the result with an +in-memory filter over the full read: +- DECIMAL(3,2) `[1.23, 2.50]` with `gt('d', 1.005G)`, `eq('d', 1.005G)` and `le('d', 2.505G)`; +- INT32 `[1, 2]` with `gt('i', 1.5G)`, `eq('i', 10_000_000_000L)`, `lt('i', 10_000_000_000L)` and + `ne('i', Double.NaN)`; +- TIMESTAMP(MILLIS) with an `Instant` that has microseconds; +- TIME(MILLIS) with a `LocalTime` that has nanoseconds; +- all of the above inside `and`/`or`/`not`, through `batches()` with `maxBatchRows`, and through a + dataset read; +- pushdown regressions for exact values that must still be pushed, with the built + `FilterPredicate` asserted through a package-scope `pushdown(schema)` call and the results + checked: + - INT32-backed DECIMAL(5,2) (external file built with `Types`) with `gt('d', 1.25G)`; + - INT64-backed DECIMAL(12,2); + - TIME(MILLIS) INT32 with `ge('t', LocalTime.of(12, 0))`; + - DATE with `eq`. + +3.5.1 [x] Update the existing tests that assert the old throwing behavior (decision A), in +`ParquetTypePolicyTest.groovy:244-245`: +- `assertThrows(ArithmeticException) { ...Predicates.eq('price', 1.251G)... }` becomes an + exact-result assertion: 0 rows, since no stored price equals 1.251. +- `assertThrows(IllegalArgumentException) { ...Predicates.eq('instant', Instant.EPOCH.plusNanos(1))... }` + becomes an exact-result assertion: rows equal to `EPOCH.plusNanos(1)` cannot exist in a MICROS + column, so the result is 0 rows. + +Grep the whole test tree for `assertThrows(ArithmeticException` and `Lossy` to catch any others. + +3.6 [x] Partial `and` pushdown. In `pushdown` (`:96-104`), for `and` push only the children that +are not `null`: +- both pushable → `FilterApi.and`; +- one pushable → that child alone; +- none pushable → `null`. + +`or` and `not` stay all-or-nothing. A partial `and` is a superset of the exact predicate, so an +`or` of supersets is still a superset; `not` is never pushed, so negation of a relaxed predicate +cannot occur. + +This is exact because the residual is always applied (`ParquetBatches.groovy:69` binds it, +`:180` evaluates it per record). Add a comment at the `and` branch saying that correctness +depends on the residual always being applied and on `not` never being pushed. + +Tests: +- `and(gt('i', 1.5G), eq('j', 3))` prunes row groups/pages, measured with `CountingInput` byte + counts as in `ParquetPruningTest`, and returns exact rows; +- `or(and(gt('i', 1.5G), eq('j', 3)), eq('j', 7))` and `not(and(...))` return exact rows. + +## 4. Writer Shape Inference for Nested Values + +4.1 [x] In `MatrixParquetWriter.buildMapType` (`MatrixParquetWriter.groovy:920-924`), compute the +value classes with `ColumnTypePolicy.policyType(it.class)` instead of `it.class`. Fall back to a +STRUCT only when the policy types differ, or when a policy type is `null` (so the existing +contextual error still fires). Probe evidence: `[a: List.of(1), b: List.of(1,2,3)]` became a +STRUCT, and `[(1): List.of(1), (2): List.of(1,2,3)]` and `[(1): "x${i}", (2): 'y']` failed with +`Struct field 'm' requires String keys`. + +4.2 [x] In `buildParquetType` (`:801-827`), honor `preferStructForMaps` only when every non-null +element map has only `String` keys. Otherwise build a MAP with `buildMapType`. Confirm that +`ParquetValueWriter.writeValue` sends MAP-annotated element groups to `writeMap`; it dispatches on +the annotation, so this should need no change. + +4.3 [x] In `ColumnTypePolicy.resolve` (`ColumnTypePolicy.groovy:39`), accept any `CharSequence` +when the declared type is `String`. The policy type is already `String`, and +`ParquetValueWriter` writes `value.toString()`. Do not loosen any other declared types. + +4.4 [x] Tests in `ParquetNestedWriteTest`: +- Map columns with `List.of`/`ArrayList`/`Collections.emptyList()` values; +- `LinkedHashMap`/`HashMap`/`TreeMap` values; +- `GString`/`String` values with String keys and with Integer keys. + +Assert that the footer schema is MAP (not a group without an annotation) and that the data +round-trips. Also cover: +- a `List` column whose element maps have Integer keys, round-tripping as a list of maps; +- a list column whose element maps have String keys, still written as a STRUCT (no regression); +- a String-declared column containing `GString` values. + +## 5. Nested Type Metadata + +5.1 [x] Add `static final String METADATA_NESTED_TYPES = 'matrix.nestedTypes'` to both +`MatrixParquetWriter` and `MatrixParquetReader`, next to the existing metadata keys. + +5.2 [x] While building the schema, record each nested leaf's canonical `ColumnTypePolicy.metadata` +class by logical path. Use the path that `DecimalSchemaPolicy.child` already produces. Thread the +map through `WriteSchema`. Write the key only for paths whose class differs from the reader's +default for that physical/logical type. Those classes are: `Instant`, `Date`, `Timestamp`, +`java.sql.Date`, `Byte`, `Short` and `BigInteger`. `LocalTime` and `Time` need no entry: +- `LocalTime` is written as TIME(NANOS, adjusted=false); +- `Time` is written as TIME(MILLIS, adjusted=true); +- `decodePrimitive` (`MatrixParquetReader.groovy:1113-1120`) already tells them apart when + `expected == null`. + +Serialize with the existing `toJsonStringArray`-style escaping as a JSON object. + +5.3 [x] Reader: +- Parse the key with the JSON helpers already in `MatrixParquetReader`; extend + `parseJsonStringArray` into a small object parser, or add a sibling. +- Ignore unknown class names (look them up through `ColumnTypePolicy.METADATA_TYPES`, as + top-level types do) and incompatible entries (reuse `ParquetBatches.compatibleMetadata`). +- Pass the per-path expected class into `ColumnMaterializer.converter` (`ColumnMaterializer.groovy:71-114`) + instead of `null`. + +5.3.1 [x] Path mapping. The writer records *logical* paths (`col.element`, `col.key`, +`col.value`, `col.`). `ColumnMaterializer.converter` recurses over the *physical* schema +(`col.list.element`, `col.key_value.key`, `col.key_value.value`). + +Build the lookup path from the logical rules: +- for a LIST-annotated group, skip the repeated wrapper level and append `element`; +- apply the same 2-level/`array`/`_tuple` rules already used at + `ColumnMaterializer.groovy:99-102`: a repeated primitive, or a wrapper that is itself the + element, maps to `element`; +- for a MAP (or legacy MAP_KEY_VALUE, see §11) group, skip the repeated wrapper and append the + key/value *role* (`key`/`value`), not the physical field name; +- for a struct, append the field name. + +Put this mapping in one package-scope helper used by both the writer path recording and the +reader lookup, so the two cannot drift apart. + +5.4 [x] Projection and datasets: when a column is projected, apply only that column's paths. +`DatasetBatches.validateSchemas` must also require equal `matrix.nestedTypes` across shards, the +same rule it uses for `matrix.columnTypes`. + +5.5 [x] Tests in `ParquetNestedTypesTest`: +- Round-trip lists containing `Instant`, `Date`, `Timestamp`, `java.sql.Date`, `Byte`, `Short`, + `BigInteger` and `LocalTime`; +- maps with those as keys and values; +- structs with those as fields. + +Also assert: +- a file written without the key (legacy fixture) reads as before; +- a corrupted or unknown entry falls back to inference; +- path mapping: + - a `List>` (`l.element.element`) and a `Map>` + (`m.value.element`) round-trip with exact types. Both fail if the `list`/`key_value` wrapper + levels leak into the lookup path; + - an external file using a 2-level `repeated int32 array` list with a hand-written + `matrix.nestedTypes` entry `{"l.element":"java.lang.Short"}` returns `Short` elements; +- `LocalTime` and `Time` nested values still round-trip without any `matrix.nestedTypes` entry, + as a regression guard. + +## 6. Full Logical Paths in Nested Errors (completes review 9.7) + +6.1 [x] Schema builder: pass `decimals.path` (falling back to the column name) into +`buildMapType`, `buildStructType` and `buildPrimitiveType`, and use it in every message: +`MatrixParquetWriter.groovy:833, 853, 899, 912, 950`. + +6.2 [x] Value writer: add a `String path` parameter to `ParquetValueWriter.writeValue`, +`writePrimitiveValue`, `writeBigDecimalValue`, `writeFixedLenBigDecimal`, `writeList`, `writeMap` +and `writeStruct`. Children receive `path.element`, `path.key`, `path.value` and `path.`. +Use the path in the messages at `ParquetValueWriter.groovy:142, 153, 174, 187, 192, 200, 214, 230`. +Include the row number when it is available: pass it in from `writeRows`. + +6.3 [x] Tests (extend `ParquetReviewRegressionTest`). Assert that the messages contain: +- `l.element` for nested decimal overflow with `decimalMeta(['l.element': [3, 2]])` and value `10.00`; +- `l.element` for a list of maps with a null key; +- `mm.value` for a map whose value map has a null key; +- `l.element` for an Integer-keyed element struct (if anything still produces a struct error + after 4.2); +- `l.element` for unsupported nested `Object` values (already correct; keep it as a guard). + +## 7. Writer Option Validation + +7.1 [x] Bloom options (completes review 9.8): +- In `ParquetWriteOptions.fromMap` (`ParquetWriteOptions.groovy:291-292`), copy each map entry. + Convert enabled flags with `OptionMaps.booleanValueOrNull(v, "bloomFilterEnabled['$k']")`. +- Convert NDVs with a strict integral parser so string-map/SPI callers get the same handling for + booleans and numbers: + - accept integral `Number`s: `Integer`/`Long`/`Short`/`Byte`/`BigInteger` within long range, + and `BigDecimal` with `stripTrailingZeros().scale() <= 0`; + - accept numeric strings parsed with `new BigDecimal(s.trim())` under the same integral rule; + - reject fractional values (`1.5`, `'1.5'`) and non-numeric values. Never truncate with + `longValue()`. +- `OptionMaps` currently has no numeric helper; it only has `normalizeKeys`, `stringValueOrNull` + and `booleanValueOrNull`. Add `OptionMaps.longValueOrNull(Object, String)` in matrix-core if + another module needs the same conversion (check `grep -rn "as Number).longValue\|(Number)" matrix-*/src/main` + for SPI option parsing), otherwise keep it as a private helper in `ParquetWriteOptions`. If it + goes into matrix-core, add matrix-core tests and run `./gradlew :matrix-core:test`. +- Reject other types with `IllegalArgumentException` naming the column. +- In `validate()` (`:178-180`), check `instanceof Boolean` / `instanceof Number` and include the + column path in every message. +- Tests: + - `fromMap([bloomFilterEnabled: [id: 'true']])` writes a bloom filter. Today it throws + `MissingMethodException`. + - `[id: 'maybe']`, `[id: null]`, `bloomFilterNDV: [id: 'x']`, `[id: 0]`, `[id: 1.5]` and + `[id: '1.5']` fail with messages containing `id`; + - `bloomFilterNDV: [id: '1000']` and `[id: 1000.0G]` are accepted as `1000L`. + - Integer NDVs from the builder still work. + +7.2 [x] `compressionCodec(String)` (`:129-132`): +- Add a null check: throw `IllegalArgumentException('compressionCodec cannot be null')`. +- Use `value.trim().toUpperCase(Locale.ROOT)`. +- Wrap `valueOf` in a contextual `IllegalArgumentException` listing the valid names. + +Test under `Locale.setDefault(Locale.forLanguageTag('tr-TR'))` (restore it in `finally`): +`'gzip'`, `' zstd '`, `null`, `'nope'`. + +## 8. Temporal Precision and Write-Loop Performance + +8.1 [x] Sub-microsecond timestamps (decision D: default `MICROS` with truncation; `Instant` +rejection removed): +- Add `timestampUnit` (`'MICROS'` default, `'NANOS'`) to `ParquetWriteOptions`: + - the field, the fluent setter (trimmed, case-insensitive with `Locale.ROOT`, null and unknown + values rejected with `IllegalArgumentException`), `validate`, `toMap`, `fromMap` and + `descriptors()`; + - `WriterBuilder.timestampUnit(String)`. +- `buildPrimitiveType` (`MatrixParquetWriter.groovy:887-890`) uses the configured unit for + `LocalDateTime`/`Timestamp`/`Instant`. +- Remove the `Instant` sub-microsecond rejection at `ColumnTypePolicy.groovy:43` and + `ParquetValueWriter.groovy:141-143`. +- Under `MICROS`, truncate sub-microsecond digits for all three classes. The existing + `instantToMicros` already discards them, so after removing the check the behavior is the same + for all three (it is replaced by the shared helper below, which also truncates). Add a comment + at the helper: "Sub-microsecond digits are intentionally truncated + under MICROS; see req/v0.8.0-fixes.md decision D." +- Keep the int64 overflow check for both units (`ColumnTypePolicy.groovy:44-45` becomes + unit-aware and calls the shared helper below), and extend it to `LocalDateTime` (after zone conversion) and `Timestamp`, with + column/row context. For `LocalDateTime`, the zone-dependent overflow check can happen in + `ParquetValueWriter`, where the zone is known. +- All epoch-unit conversions go through one shared helper, for example + `TemporalUnits.epochUnits(Instant, TimeUnit)`. There are four call sites, and none may keep the + naive formula: + 1. `ColumnTypePolicy.groovy:44-45`, the overflow check at schema-building time. It runs before + any value is written, so a naive check here would reject the NANOS lower bound before the + writer is reached; + 2. the `MICROS` writer path, `ParquetValueWriter.instantToMicros` (`:163`). Delete it and call + the helper directly; + 3. the new `NANOS` writer path; + 4. `ParquetPredicate.physicalValue` (`:178`), see below. + + The naive + `addExact(multiplyExact(epochSecond, 1_000_000_000L), nano)` overflows at the lower NANOS bound: + `1677-09-21T00:12:43.145224192Z` has `epochSecond = -9223372037`, and the intermediate product + is below `Long.MIN_VALUE` even though the final value (`Long.MIN_VALUE`) fits. For negative + `epochSecond`, compute `addExact(multiplyExact(epochSecond + 1, unitsPerSecond), + nano.intdiv(nanosPerUnit) - unitsPerSecond)`; otherwise + `addExact(multiplyExact(epochSecond, unitsPerSecond), nano.intdiv(nanosPerUnit))`. Verified + with a JDK 21 probe: the naive form throws `long overflow` at the minimum; the split form gives + exactly `Long.MIN_VALUE`/`Long.MAX_VALUE` at the bounds, throws one nanosecond outside either + bound, and gives `-500000000` for `1969-12-31T23:59:59.5Z`. The `MICROS` path has the same + overflow at its lower bound, `-290308-12-21T19:59:05.224192Z` (`epochSecond = -9223372036855`): + a second probe showed the naive form throws and the split form gives `Long.MIN_VALUE`. The + upper bound is `+294247-01-10T04:00:54.775807Z`. +- The helper **truncates** sub-unit digits (`nano.intdiv(nanosPerUnit)`, which floors because + `nano` is never negative). It does not check for loss; that is the caller's job. +- Use the same helper in `ParquetPredicate.physicalValue` (`:178`, which today has the same naive + formula). Otherwise an exact predicate on the NANOS lower bound silently becomes + `NOT_PUSHABLE` under decision A: results stay correct but pruning is lost. +- `physicalValue` must keep its lossy check, `instant.nano % nanosPerUnit != 0` → + `NOT_PUSHABLE`, **before** calling the helper. The residual predicate re-checks every row + that the pushdown keeps (`ParquetBatches.groovy:69,180`), but it cannot bring back rows that + the pushdown dropped. So a truncated push is wrong whenever it drops a matching row. Example: + a MICROS column stores S = `.123456`, the predicate value is V = `.123456789`, and its + truncated value is T = `.123456`. Stored values have whole-microsecond precision, so for a + stored R, `R <= V` is the same as `R <= T`: + - `lt` is the only operator that breaks. Exact `S < V` is true, but the pushed `lt(T)` drops S; + - `le(T)` keeps exactly the matching rows, and `gt(T)` is exact for the same reason; + - `eq(T)` and `ge(T)` keep a superset, which the residual narrows; + - `ne` and `not` are never pushed (`ParquetPredicate.groovy:108` returns `null` for `ne` + before computing a physical value, and `not` falls through to `default -> null` at `:102`). + + The check stays mandatory for every operator. Only `lt` fails today, but that depends on how + each operator happens to be built, and a later change (pushing `not`, for example) would break + silently. +- Pass the unit through `ColumnTypePolicy` and `ParquetValueWriter` as a parameter, not a + ThreadLocal. +- Reader: TIMESTAMP(NANOS) is already decoded by `timestampInstant`. Confirm that + `ParquetPredicate.physicalValue` handles NANOS columns, and add a predicate test. +- Unit-test the helper directly for MILLIS/MICROS/NANOS: at the NANOS bounds, one nanosecond + outside them, at the MICROS bounds (`-290308-12-21T19:59:05.224192Z`, + `+294247-01-10T04:00:54.775807Z`), one microsecond outside them, `Instant.EPOCH`, + `1969-12-31T23:59:59.5Z` and `Instant.ofEpochSecond(-1)`. + +8.2 [x] Replace Groovy `/` with integer division in `ParquetValueWriter.timeToMillis` (`:159`). +`instantToMicros` (`:163`) is removed by §8.1, so it is not fixed here. Use `intdiv` or `Math.floorDiv`, matching +`MatrixParquetReader.groovy:1178`. Probe evidence: `long / long` and `int / long` both produce +`BigDecimal` under `@CompileStatic`. Grep `matrix-parquet/src/main` for other integer `/` uses in +per-value paths and fix them the same way. + +8.3 [x] `MatrixParquetWriter.writeRows` (`:629-645`): +- Before the loop, look up `List columns = colNames.collect { matrix.column(it) }` and + `List fieldTypes = colNames.collect { schema.getType(it) }`. +- Loop with indexes and no per-row closure: plain `for` loops are acceptable in this hot path; + add a comment saying why. +- This removes the per-cell linear `Matrix.columnIndex` scan and the `asType` call made by + `Matrix.getAt(Integer, String)`. +- Before relying on dropping `asType`, confirm that every declared type is satisfied by the raw + value. `ColumnTypePolicy.resolve` already requires `declared.isInstance(value)`; for CharSequence + values (section 4.3) the writer calls `toString()`. + +8.4 [x] Tests in `ParquetTemporalPrecisionTest`: +- `LocalDateTime`, `Timestamp` and `Instant` with 123456789 ns write without error and read back + as `.123456` under the default. This is the regression test for the removed `Instant` + rejection. Update the existing test that asserts the old error: + `ParquetTypePolicyTest.groovy:78` lists `Instant.EPOCH.plusNanos(1)` among values that must + throw. Remove it from that list (keep `Instant.MIN`/`Instant.MAX` as overflow cases) and assert + that it now round-trips as `Instant.EPOCH`; +- a pre-1970 `Instant` with sub-microsecond nanos truncates downward in time (nanos are + non-negative); +- `Instant.now()` and `LocalDateTime.now()` columns write successfully under the default on any + platform; +- an int64-overflowing `Instant` (for example `Instant.MAX`) fails under both units with the + column and row in the message; +- `timestampUnit('NANOS')` writes a `TIMESTAMP(NANOS)` footer (assert the schema), round-trips + the nanosecond values exactly, and filters with `Predicates.eq` on a nanosecond value; +- filtering on the value that was written, the documented surprise of decision D: + - write `Instant.EPOCH.plusNanos(123_456_789)` under the default `MICROS`; + - `eq('ts', thatSameInstant)` returns **0 rows**: the stored value is `.123456`, the predicate + is not pushable (lossy), and the exact residual compares `.123456` with `.123456789`; + - `eq('ts', thatInstant.truncatedTo(ChronoUnit.MICROS))` returns the row; + - `ge('ts', thatSameInstant)` returns 0 rows and `le` returns the row; + - under `NANOS`, the original `eq` returns the row; +- NANOS range: `Instant.parse('1677-09-21T00:12:43.145224192Z')` and + `Instant.parse('2262-04-11T23:47:16.854775807Z')` write under `NANOS`, and one nanosecond + outside either bound fails with column/row context. The same values succeed under `MICROS`. + `eq('ts', lowerBound)` on the NANOS file returns the row **and is pushed down** (assert the + bound predicate is not `NOT_PUSHABLE`, for example via the pushdown result or a byte-count + check like `ParquetPruningTest`); +- MICROS lower bound: `Instant.parse('-290308-12-21T19:59:05.224192Z')` writes under the + default and reads back exactly. One microsecond earlier fails with column/row context. This + goes through both the `ColumnTypePolicy` check and the writer; +- lossy predicates are not pushed: on a MICROS column storing `EPOCH.plusNanos(123_456_000)`, + `lt`, `le` and `ne` with `EPOCH.plusNanos(123_456_789)` each return the row. This is a + regression guard. Only `lt` would fail without the check today (see §8.1); +- invalid `timestampUnit` values (`null`, `'millis'`, `'SECONDS'`) are rejected; `' nanos '` is + accepted; +- microsecond values are unchanged under the default; +- `Time` millisecond round-trip still passes; +- SPI `timestampUnit` and descriptor assertions. + +Add a write-throughput measurement to `ParquetLayoutBenchmarkTest`: 200 columns x 50,000 rows. +It is gated by `parquet.layoutBenchmark`; run it with +`./gradlew :matrix-parquet:test --tests '*ParquetLayoutBenchmarkTest' -Pparquet.layoutBenchmark=true --rerun` +after §1.4, or with the init script `/tmp/parquet-layout.init.gradle` from +`optimized-parquet-plan.md`. Record the before/after times from 8.3 in this plan, and confirm in +the test report that the class executed rather than being skipped. + +## 9. Reader I/O Fixes + +9.1 [x] In `ReaderBuilder.read(InputStream)` (`MatrixParquetReader.groovy:316`), replace +`File.createTempFile` with `Files.createTempFile('matrix-parquet-', EXTENSION)`, which is +owner-only on POSIX. Probe evidence: the current file is created `rw-r--r--`. Test on POSIX +(`assumeTrue` POSIX file store) by capturing the temp path through a stream whose `read` inspects +`java.io.tmpdir` for the newest `matrix-parquet-*.parquet`. Alternatively, extract a package-scope +`createReadTempFile()` helper and test that directly (preferred). + +9.2 [x] Dataset matrix names (`DatasetBatches.groovy:49, 116`): +- Add a `fallbackName` constructor argument. +- `ReaderBuilder.read(File dir)` / `batches(File dir)` pass the directory name. +- `read(List)` / `readGlob` pass `defaultMatrixName(files[0])` (extension stripped). +- `options.matrixName` still wins. + +Tests: a directory read is named after the directory, a list read is named `part-0`, and an +explicit `matrixName` is honored on both the collected and the batched path. + +9.3 [x] **(unverified on Windows)** Rewrite `globFiles` (`MatrixParquetReader.groovy:827-838`). +Put the parsing in a package-scope `GlobSpec parseGlob(String pattern, String separator)` so it can +be tested on any OS. + +`GlobSpec` holds the literal root string, the matcher pattern string, the walk depth (or +unlimited), and a flag saying whether any wildcard or escape was present. + +- **Escapes and separators per OS.** + - On every platform, the JDK glob syntax (`FileSystem.getPathMatcher` Javadoc) treats `\` as an + escape character, not a separator. Its Windows example is `C:\\*`. + - On Windows (`separator == '\\'`), `\` in a user pattern means a path separator, so **glob + escaping is not supported on Windows**; document this. `parseGlob` converts every `\` to `/` + in the pattern before any further processing. The Windows glob treats `/` as a separator, + and `Path.of` accepts `/` on Windows. + - On POSIX (`separator == '/'`), `\` stays the glob escape: `\*` and `\{` are literals. +- **Root split.** After the Windows conversion, scan for the first *unescaped* metacharacter + (`*`, `?`, `[`, `{`). The root is the prefix up to the last `/` before it. + - Keep that `/` in the root when the prefix would otherwise be empty or end in `:`, so the + root is the filesystem root and not the current directory: `/*.parquet` → root `/` (today's + code does this with `Math.max(1, slash)`, `MatrixParquetReader.groovy:833`), and + `C:\*.parquet` → `C:/` rather than the drive-relative `C:`; + - If there is no `/` at all before the first metacharacter (for example `*.parquet`), the root is + the current directory, `Path.of('.')`, and the whole pattern is the relative pattern. + Probed on macOS/JDK 21: walking `.` yields `./a.parquet`, `root.relativize(p)` gives + `a.parquet`, and `glob:*.parquet` matches it (`Path.of('')` behaves the same). + - On Windows, a drive-relative pattern such as `C:*.parquet` (no separator after the drive) + is rejected with `IllegalArgumentException("Drive-relative glob not supported ...")`. + Treating it as `.` would match nothing, because relativized paths never start with `C:`. + Not verified on Windows; covered only by the `parseGlob` unit test below. + - The root's literal text is unescaped (`\x` becomes `x`) before it goes through `Path.of`. + - If the pattern has no unescaped metacharacter but contains a POSIX `\` escape (for example + `root/x\*.parquet`), the whole pattern is still a glob: the root is the prefix up to the last + `/` and the matcher gets the rest. It is not treated as a literal path, because + `Path.of('root/x\*.parquet')` would look for a file name containing a backslash. + - Only a pattern with neither metacharacters nor escapes is a plain file path, which is + today's behavior. +- **Matching.** Build the matcher from the *relative* pattern part, after the Windows + conversion, with `FileSystems.default.getPathMatcher("glob:" + relativePattern)`. Match it + against `root.relativize(p)` as a `Path`. No `/` joining and no second `Path.of` on the + pattern are needed. +- **Depth.** Let `separators` be the number of unescaped `/` in the relative pattern outside + `[...]`; the walk depth is `separators + 1` (the number of segments in the relative pattern). + Use `Files.walk(root, separators + 1)`. Fall back to an unlimited walk when the pattern contains `**`, + or when any `{...}` alternative contains a `/`, for example `{a,b/c}`. In both of those cases + the depth is not fixed. +- **Errors.** A missing literal root gives `IllegalArgumentException("No such glob root ...")`. + An empty match gives `IllegalArgumentException("No files match glob '...'")` instead of the + generic dataset message. + +Tests (POSIX run): +- a deep tree where `root/*.parquet` must not descend (assert via results, and that the walk is + depth-limited through a spy or a large decoy tree); +- `root/**/*.parquet`; +- `root/{a,b/c}/*.parquet` matches both depths; +- `root/x\*.parquet` (no unescaped metacharacter) matches only the literal file `x*.parquet` and + not `xy.parquet`. This needs a POSIX file name containing `*`; use `assumeTrue` where the file + store rejects it; +- `root/a\{b\}/*.parquet` resolves the literal directory `a{b}` as the root; +- a missing root; no matches. + +`parseGlob` unit tests with `separator = '/'`: +- `*.parquet` → root `.`, pattern `*.parquet`, depth 1; +- `{a,b}/*.parquet` → root `.`, pattern `{a,b}/*.parquet`, depth 2; +- `/*.parquet` → root `/`, pattern `*.parquet`, depth 1. + +`parseGlob` unit tests with `separator = '\\'`, which run on any OS: +- `C:\data\*.parquet` → root `C:/data`, matcher pattern `*.parquet` (no backslashes left), depth 1; +- `C:\data\sub\*.parquet` → root `C:/data/sub`, pattern `*.parquet`. Before the conversion was + added, this case produced `sub\*.parquet` with an escaped `*`, which matches nothing; +- `C:\data\{a,b}\*.parquet` → root `C:/data`, pattern `{a,b}/*.parquet`, depth 2; +- `C:\data\**\*.parquet` → unlimited depth; +- `C:\*.parquet` → root `C:/`, pattern `*.parquet`, depth 1; +- `C:*.parquet` → `IllegalArgumentException` (drive-relative). + +These cover the conversion, the root split and the depth calculation. The actual `PathMatcher` +behavior on Windows still needs a real Windows run; record in the PR that none was done unless CI +provides one. + +## 10. HTTP Range Preconditions With Weak ETags **(unverified against real servers)** + +10.1 [x] In `HttpRangeInputFile.request` (`HttpRangeInputFile.groovy:51-67`), send `If-Match` only +when `etag` is strong, meaning it does not start with `W/`. For a weak ETag, send +`If-Unmodified-Since` when `Last-Modified` is present, and keep the response-header equality +checks in `fetch` (`:74-77`) as the change detector. RFC 9110 section 13.1.1 requires strong +comparison for `If-Match`, so weak tags would get `412` on every data request. + +10.2 [x] Tests (extend `ParquetDatasetAndHttpTest`) using its local `HttpServer` pattern. The server +returns `ETag: W/"v1"` and responds `412` whenever `If-Match` is present. Assert that RANGE and +AUTO reads succeed, that no request carried `If-Match`, and that changing the weak ETag +mid-read still raises `Remote object/range changed during read`. Keep the existing strong-ETag +test. + +## 11. MAP Key/Value Field Names **(unverified with real legacy files)** + +11.1 [x] In `ColumnMaterializer.converter` (`ColumnMaterializer.groovy:103-106`), read the key and +value field names from the schema (`GroupType kv = group.getType(0).asGroupType()`; +`kv.getType(0).name`, and `kv.getType(1).name` when `kv.fieldCount > 1`) instead of +`pair.key` / `pair.value`. + +11.1.1 [x] Legacy MAP_KEY_VALUE. Some old writers annotated the *outer* group +`MAP_KEY_VALUE` (`LogicalTypeAnnotation.MapKeyValueTypeAnnotation`) instead of `MAP`. Today such +a group goes to the struct branch and becomes `[key_value: [[key: .., value: ..], ...]]`. Per the +Parquet backward-compatibility rules, treat a group annotated `MAP_KEY_VALUE` that is **not** +directly inside a MAP-annotated group as a MAP, when its single repeated child group has one or +two fields. The repeated child inside a proper MAP group may also carry `MAP_KEY_VALUE`; it must +keep being handled as the key/value wrapper. + +Pass a `parentIsMap` flag through the `converter` recursion to tell the two cases apart. Code +changes are needed only in: +- `ColumnMaterializer.converter` (the MAP branch at `:103-106`); +- the §5.3.1 path mapping helper. + +Verify only, with no code change expected: +- `MatrixParquetReader.extractFieldTypes` already returns `Map` for any non-primitive group that + is not LIST/MAP (`:1029-1030`), so an outer `MAP_KEY_VALUE` column is already typed `Map`; +- `ParquetBatches.compatibleMetadata` (`ParquetBatches.groovy:116-119`) already accepts `Map` + metadata for any non-LIST group. + +Cover both with the §11.2 assertions on `result.type('m')` rather than adding code. + +11.2 [x] Tests in `ParquetMapCompatibilityTest`, using external files built with +`Types.buildMessage()`: +- a MAP group whose repeated group is `map` with fields `k` and `v`; +- an outer group annotated `MAP_KEY_VALUE` with a `key_value` repeated child; +- a proper MAP whose repeated child is annotated `MAP_KEY_VALUE`; +- a key-only map (no value field). + +Assert the **exact entries**, not just `instanceof Map`, because a struct result is also a `Map`. +For example, `assertEquals([a: 1, b: 2], result[0, 'm'])` and `assertEquals(Map, result.type('m'))`. +Also check that a struct with a field literally named `key_value` and no annotation still reads as +a struct. + +## 12. INT96 Timestamps (decision E) + +12.1 [x] In `MatrixParquetReader`: +- `extractFieldTypes` maps INT96 to `LocalDateTime`. +- `decodePrimitive` (`:1093-1095`) decodes the 12-byte little-endian value: 8 bytes of + nanos-of-day, then a 4-byte Julian day. Use epoch day = julianDay - 2_440_588. +- The result is an `Instant`, converted like adjusted timestamps: `LocalDateTime` in the reader + zone by default, `Instant`/`Timestamp`/`Date` when the expected type asks for them. +- Validate the length and range with contextual errors. + +12.2 [x] `ParquetBatches.compatiblePhysicalMetadata`: accept `Date`, `Timestamp`, `LocalDateTime` +and `Instant` for INT96. + +12.3 [x] Predicates: +- `TypeCheck.validate` (`ParquetPredicate.groovy:197-200`) and `bind` (`:150-153`) must treat + INT96 as adjusted, without casting a null logical annotation. +- `ColumnMaterializer.ValueConverter.logicalValue` returns `Instant` for INT96. +- `pushdown` returns `null` for INT96, which is already the case because INT96 is not in the + pushable list. + +12.4 [x] Tests in `ParquetInt96Test`. Write a file with `Types.optional(INT96)` and hand-encoded +values: epoch, a pre-1970 value, and a value with nanoseconds. Assert: +- `LocalDateTime` in an explicit `zoneId`; +- `Instant` through compatible metadata; +- residual `gt`/`eq` filters; +- a malformed-length error. + +Optionally add a PyArrow fixture (`use_deprecated_int96_timestamps=True`) to +`src/test/python/generate_optimized.py`, kept under the existing fixture size limits. + +12.5 [x] Update the `FLOAT16 / INT96 unsupported cases` row and the "External INT96/FLOAT16 +raise ..." sentence in `release.md`, and the readme type table. + +## 13. Remove Dead Production Code + +13.1 [x] Move the Group-based decoding helpers into a test-only helper, +`src/test/groovy/se/alipsa/matrix/parquet/GroupValueReader.groovy`, that calls the package-scope +`MatrixParquetReader.decodePrimitive`. The helpers are `readValue`, `readPrimitive`, `readList`, +`readMap` and `readStruct` (`MatrixParquetReader.groovy:1057-1089, 1203-1245`), together with the +`FIELD_LIST`/`FIELD_KEY_VALUE` constants and the never-set `BINARY_AS_STRING` ThreadLocal +(`:110`). Point `ParquetReadBenchmarkTest.legacy` (`:67`) at the helper. Remove the imports that +become unused. + +13.2 [x] Remove the unreachable POJO paths: +- `MatrixParquetWriter.buildStructType` `else` branch (`:967-985`), the `declaredType` parameter + and the `fieldTypes` map; +- `ParquetValueWriter.toStructMap` POJO branch (`:265-273`); +- `PROPERTY_DESCRIPTOR_CACHE`; +- `getPropertyDescriptors`; +- the `Introspector`/`PropertyDescriptor`/`ConcurrentHashMap` imports. + +`ColumnTypePolicy` rejects POJOs before schema construction, and `buildStructType` only receives +Maps. + +13.3 [x] Remove the impossible `if (elementType == null)` block (`MatrixParquetWriter.groovy:806-808`) +and the redundant directory branch in static `read(File)` (`MatrixParquetReader.groovy:812`). + +13.4 [x] Run `./gradlew :matrix-parquet:codenarcMain` to confirm there are no new unused-import or +unused-method violations. `./gradlew :matrix-parquet:compileTestGroovy` proves the benchmark +compiles. Proving it *runs* needs the gating property: +`./gradlew :matrix-parquet:test --tests '*ParquetReadBenchmarkTest' -Pparquet.baseline=true -Pparquet.benchmarkRows=10000 --rerun` +after §1.4, or the init script `/tmp/parquet-baseline.init.gradle` from `optimized-parquet-plan.md`. +Confirm in `build/test-results` that the test executed and was not skipped. + +## 14. Documentation + +14.1 [x] Fix the stale writer GroovyDoc (completes v0.7.0 3.8 / review 9.13): +- `MatrixParquetWriter.groovy:156`: `inferPrecisionAndScale(false)` requires explicit + `precision`/`scale` or `decimalMeta` and otherwise throws. +- `:99`: change the class example to `.inferPrecisionAndScale(false).precision(38).scale(18)`. +- `:424-425`: unmatched BigDecimal columns are inferred; metadata for non-decimal paths is + rejected. +- `:334-335` and `:465-466`: check the wording against the actual behavior. +- Timestamp precision (decision D): + - `ParquetWriteOptions.timestampUnit(String)` and `WriterBuilder.timestampUnit(String)` document + the default `MICROS`, that sub-microsecond digits of `LocalDateTime`/`Timestamp`/`Instant` + are truncated (downward in time) under `MICROS`, that `NANOS` preserves them, and that + Spark-family readers may not support `TIMESTAMP(NANOS)`; + - they also state the `NANOS` range: int64 nanoseconds cover + 1677-09-21T00:12:43.145224192Z to 2262-04-11T23:47:16.854775807Z, and values outside it fail. + `MICROS` covers about ±292,000 years; + - the `MatrixParquetWriter` class Javadoc gets one sentence pointing to `timestampUnit`. + +14.2 [x] `readme.md`: document every behavior change and new option, with defaults and examples: +- unsigned types; +- predicate residual fallback; +- nested MAP/STRUCT rules; +- `matrix.nestedTypes`; +- `timestampUnit`, in a dedicated "Timestamp precision" paragraph next to the type table: + - default `MICROS`; sub-microsecond digits are truncated for `LocalDateTime`, `Timestamp` and + `Instant`; + - this mainly affects Linux (JDK 15+ nanosecond clock) and Windows (100 ns clock) `now()` + values; macOS clocks are microsecond; + - `timestampUnit('NANOS')` preserves full precision, at the cost of weaker interoperability + with Spark-family readers, and only for 1677-09-21T00:12:43.145224192Z to + 2262-04-11T23:47:16.854775807Z (int64 nanoseconds). Values outside that range fail; use + `MICROS` for them; + - **filtering on a written value:** under `MICROS`, a filter with the original + sub-microsecond value does not match the stored, truncated value. For example, after writing + `Instant.now()`, `eq('ts', sameInstant)` returns no rows. Filter with + `value.truncatedTo(ChronoUnit.MICROS)`, or write with `timestampUnit('NANOS')`. Include a short + code example; + - include a builder example and an SPI example (`[timestampUnit: 'NANOS']`), and update the + Instant row of the type table, which currently says sub-microsecond Instants fail; +- INT96 read support; +- dataset naming; +- glob semantics (depth, separators, errors); +- weak-ETag behavior. + +14.3 [x] `release.md` `v0.8.0 (unreleased)`: add rows for the behavior changes: +- unsigned reads; +- timestamp precision (decision D), as a table row: + - behavior: `LocalDateTime`/`Timestamp`/`Instant` are written as TIMESTAMP(MICROS), and + sub-microsecond digits are truncated consistently. Sub-microsecond `Instant` values are no + longer rejected, so `Instant.now()` works on Linux and Windows; + - compatibility/opt-out: `timestampUnit('NANOS')` preserves full precision. This is an + intentional exception to the lossless-decimal default, bounded below 1 µs; + - also update the existing `Instant fallback` row ("reject sub-microsecond precision loss ...") + so it no longer contradicts the new behavior; +- INT96; +- dataset names; +- GString acceptance; +- nested type metadata. + +Add one review-fixes sentence for the rest. + +14.4 [x] Add a pointer to this file from `req/optimized-parquet-review.md` next to items 9.7, 9.8 +and 9.13. Example: "partially completed; finished in v0.8.0-fixes §6/§7.1/§14.1". + +## 15. Final Verification + +15.1 [x] Run the module gates in the required order and record the results below: + +```bash +./gradlew :matrix-parquet:codenarcMain :matrix-parquet:codenarcTest --console=plain +./gradlew :matrix-parquet:spotlessCheck --console=plain +./gradlew :matrix-parquet:test --console=plain +``` + +15.2 [x] Run the slow, interop and consumer checks: + +```bash +PARQUET_PYTHON=/tmp/matrix-parquet-fixtures/bin/python \ + ./gradlew :matrix-parquet:test :matrix-parquet:compileConsumerApi -PrunSlowTests=true --console=plain +``` + +`-PrunSlowTests=true` does not enable the benchmark classes (see §1.4). Run them separately with +their gating properties and confirm in the test report that they executed: + +```bash +./gradlew :matrix-parquet:test --tests '*ParquetReadBenchmarkTest' -Pparquet.baseline=true -Pparquet.benchmarkRows=10000 --rerun --console=plain +./gradlew :matrix-parquet:test --tests '*ParquetLayoutBenchmarkTest' -Pparquet.layoutBenchmark=true --rerun --console=plain +``` + +15.3 [x] Run the full repository suite: `./gradlew test -Pheadless=true --console=plain`. + +15.4 [x] Check that `release.md` has exactly one `v0.8.0 (unreleased)` section and that no git tag +was created. + +## Recommended Implementation Order + +1. Section 1 (preparation) +2. Section 13 (dead-code removal; shrinks later diffs) +3. Sections 8.2 and 8.3 (mechanical performance fixes, with no behavior change) +4. Sections 2, 3, 11, 12 (reader correctness) +5. Sections 4, 6, 7, 8.1 (writer correctness and validation) +6. Section 5 (nested type metadata; depends on 4 and 6 path plumbing) +7. Sections 9, 10 (I/O and HTTP) +8. Section 14, then 15 + +## Verification Record + +Verified on macOS with OpenJDK 21.0.12.1. The module remains `0.8.0-SNAPSHOT`. + +- `./gradlew :matrix-parquet:codenarcMain :matrix-parquet:codenarcTest --console=plain` — passed. + A later reused worker logged an internal CodeNarc/Groovy error on unchanged `HttpTransportTest`; + `./gradlew :matrix-parquet:codenarcMain :matrix-parquet:codenarcTest --no-daemon --rerun-tasks --console=plain` + passed cleanly with all tasks executed and no rule-engine errors. Final source checks with + `./gradlew :matrix-parquet:codenarcMain :matrix-parquet:codenarcTest --no-daemon --console=plain` + also passed cleanly. +- `./gradlew :matrix-parquet:spotlessApply :matrix-parquet:spotlessCheck --console=plain` — passed. +- `./gradlew :matrix-parquet:test --console=plain` — passed (155 passed, 6 optional cases skipped before the final metadata guard; the final slow run below passed 156). +- `PARQUET_PYTHON=/tmp/matrix-parquet-fixtures/bin/python ./gradlew :matrix-parquet:test :matrix-parquet:compileConsumerApi -PrunSlowTests=true --console=plain` + — passed again on the final implementation (156 passed, 6 skipped), including the generated + default CDC fixture and consumer compilation. Six opt-in + benchmark/legacy-capture cases skipped; benchmark execution was verified separately below. +- `/tmp/matrix-parquet-fixtures/bin/python matrix-parquet/src/test/python/verify_matrix_output.py` + — passed with PyArrow 21.0.0 for SNAPPY/ZSTD, annotated strings, raw binary, exact decimals and + CDC rewrite metadata preservation. The following slow/module runs read the CDC rewrites back. +- `/tmp/matrix-parquet-fixtures/bin/python matrix-parquet/src/test/python/generate_benchmarks.py` + — generated the three million-row layout fixtures under ignored `build/layout-benchmarks`. +- `./gradlew :matrix-parquet:test --tests '*ParquetReadBenchmarkTest' -Pparquet.baseline=true -Pparquet.benchmarkRows=10000 --rerun --console=plain` + — passed; XML: 1 test, 0 skipped. Console showed `:matrix-parquet:test` executed, without cache/up-to-date markers. +- `./gradlew :matrix-parquet:test --tests '*ParquetLayoutBenchmarkTest.wideMatrixWriteThroughput' -Pparquet.layoutBenchmark=true --rerun --console=plain` + — passed with the old per-cell lookup loop temporarily restored for comparison: **9,590 ms** + for 200 columns × 50,000 rows, 40,071,174 bytes. The optimized implementation was restored + immediately after this measurement. +- `./gradlew :matrix-parquet:test --tests '*ParquetLayoutBenchmarkTest' -Pparquet.layoutBenchmark=true --rerun --console=plain` + — passed; XML: 2 tests, 0 skipped. Console showed the test task executed. Optimized wide write: + **3,427 ms**, identical 40,071,174 bytes (one local run per implementation, about 2.8× faster). + The layout benchmark also exercised full/projection/selective reads of ordinary, indexed and + CDC-indexed million-row fixtures. Properties were forwarded by the module build, without init scripts. +- `./gradlew test -Pheadless=true --console=plain` — passed (119 tasks; 31 executed, 88 up-to-date). + Final rerun after the last metadata guard — passed (119 tasks; 5 executed, 114 up-to-date). +- Release section uniqueness and unchanged git tags — passed: exactly one unreleased section; tag-list SHA-1 unchanged (`c53be6da3edb71e66d2eece6ea94cf0ebc53559b`). + +Limits: no Windows host run was performed; Windows root/separator/depth/drive-relative rules +were tested through `parseGlob` on macOS. Weak ETags and legacy MAP layouts were exercised with +local HTTP/synthetic external Parquet fixtures, not production remote servers or real legacy files. diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnMaterializer.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnMaterializer.groovy index 6e92eb0ca..099ed5220 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnMaterializer.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnMaterializer.groovy @@ -24,7 +24,7 @@ final class ColumnMaterializer extends RecordMaterializer { private final int[] sizes private final ParquetPredicate residual - ColumnMaterializer(MessageType schema, List types, boolean binaryAsString, ParquetPredicate residual) { + ColumnMaterializer(MessageType schema, List types, boolean binaryAsString, ParquetPredicate residual, Map nestedTypes = [:]) { columns = schema.fields.collect { [] } sizes = new int[columns.size()] this.residual = residual @@ -32,7 +32,7 @@ final class ColumnMaterializer extends RecordMaterializer { schema.fields.eachWithIndex { Type field, int index -> Closure logicalSink = residual != null && residual.columns().contains(field.name) ? { Object value -> current[field.name] = value } : null children[index] = converter(field, types[index], binaryAsString, - { Object value -> columns[index].add(value) }, logicalSink) + { Object value -> columns[index].add(value) }, logicalSink, field.name, nestedTypes) } root = new GroupConverter() { @Override @@ -68,21 +68,28 @@ final class ColumnMaterializer extends RecordMaterializer { } } - private static Converter converter(Type type, Class expected, boolean binaryAsString, Closure sink, Closure logicalSink = null) { + @SuppressWarnings('ParameterCount') // Recursion carries physical wrapper roles and logical paths explicitly. + private static Converter converter(Type type, Class expected, boolean binaryAsString, Closure sink, Closure logicalSink = null, String path = '', Map nestedTypes = [:], + boolean parentIsMap = false, boolean wrapper = false, boolean mapWrapper = false) { if (type.primitive) { - return new ValueConverter(type.asPrimitiveType(), expected, binaryAsString, sink, logicalSink) + return new ValueConverter(type.asPrimitiveType(), nestedTypes[path] ?: expected, binaryAsString, sink, logicalSink) } GroupType group = type.asGroupType() Map values = [:] Converter[] children = new Converter[group.fieldCount] + boolean map = NestedPaths.isMap(group, parentIsMap) + boolean list = group.logicalTypeAnnotation instanceof LogicalTypeAnnotation.ListLogicalTypeAnnotation group.fields.eachWithIndex { Type child, int index -> - children[index] = converter(child, null, binaryAsString) { Object value -> + boolean childWrapper = list && NestedPaths.wrappedList(group) || map + String childPath = childWrapper ? path : + NestedPaths.child(path, list || wrapper && !mapWrapper ? 'element' : mapWrapper ? (index == 0 ? 'key' : 'value') : child.name) + children[index] = converter(child, null, binaryAsString, { Object value -> if (child.repetition == Type.Repetition.REPEATED) { ((List) values.computeIfAbsent(child.name) { [] }).add(value) } else { values[child.name] = value } - } + }, null, childPath, nestedTypes, map, childWrapper, map) } new GroupConverter() { @Override @@ -97,12 +104,15 @@ final class ColumnMaterializer extends RecordMaterializer { if (logical instanceof LogicalTypeAnnotation.ListLogicalTypeAnnotation) { Type repeated = group.getType(0) List entries = (List) (values[repeated.name] ?: []) - boolean wrapped = !repeated.primitive && repeated.asGroupType().fieldCount == 1 && - !(repeated.name in ['array', "${group.name}_tuple".toString()]) + boolean wrapped = NestedPaths.wrappedList(group) sink.call(wrapped ? entries.collect { entry -> ((Map) entry)[repeated.asGroupType().getType(0).name] } : entries) - } else if (logical instanceof LogicalTypeAnnotation.MapLogicalTypeAnnotation) { + } else if (map) { Map result = [:] - ((List) (values[group.getType(0).name] ?: [])).each { entry -> Map pair = (Map) entry; result[pair.key] = pair.value } + GroupType kv = group.getType(0).asGroupType() + ((List) (values[group.getType(0).name] ?: [])).each { entry -> + Map pair = (Map) entry + result[pair[kv.getType(0).name]] = kv.fieldCount > 1 ? pair[kv.getType(1).name] : null + } sink.call(result) } else { Map result = [:] @@ -137,8 +147,9 @@ final class ColumnMaterializer extends RecordMaterializer { private Object logicalValue(Object value) { Class logicalExpected = field.logicalTypeAnnotation instanceof LogicalTypeAnnotation.TimeLogicalTypeAnnotation ? java.time.LocalTime : + (field.primitiveTypeName == PrimitiveType.PrimitiveTypeName.INT96 || field.logicalTypeAnnotation instanceof LogicalTypeAnnotation.TimestampLogicalTypeAnnotation && - ((LogicalTypeAnnotation.TimestampLogicalTypeAnnotation) field.logicalTypeAnnotation).isAdjustedToUTC() ? java.time.Instant : + ((LogicalTypeAnnotation.TimestampLogicalTypeAnnotation) field.logicalTypeAnnotation).isAdjustedToUTC()) ? java.time.Instant : field.logicalTypeAnnotation == null && field.primitiveTypeName == PrimitiveType.PrimitiveTypeName.BINARY ? byte[] : null MatrixParquetReader.decodePrimitive(value, field, logicalExpected, false) } diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnTypePolicy.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnTypePolicy.groovy index a3549f13c..03bc98e13 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnTypePolicy.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnTypePolicy.groovy @@ -14,7 +14,6 @@ import java.time.LocalTime @PackageScope final class ColumnTypePolicy { - private static final int NANOS_PER_MICRO = 1000 static final List SUPPORTED = [String, Integer, Long, Float, Double, Boolean, Byte, Short, BigInteger, BigDecimal, Character, byte[], LocalDate, LocalDateTime, LocalTime, Instant, UUID, Date, java.sql.Date, Time, Timestamp, List, Map] @@ -28,7 +27,7 @@ final class ColumnTypePolicy { this.metadata = metadata } - static ColumnTypePolicy resolve(String name, List values, Class declaration) { + static ColumnTypePolicy resolve(String name, List values, Class declaration, org.apache.parquet.schema.LogicalTypeAnnotation.TimeUnit unit = org.apache.parquet.schema.LogicalTypeAnnotation.TimeUnit.MICROS) { Class declared = box(declaration ?: Object) boolean infer = declared == Object || declared.interface || Modifier.isAbstract(declared.modifiers) Class policy = null @@ -36,13 +35,12 @@ final class ColumnTypePolicy { if (value == null) { return } - if (!declared.isInstance(value)) { + if (!declared.isInstance(value) && !(declared == String && value instanceof CharSequence)) { throw new IllegalArgumentException("Column '$name', row $row: declared ${declared.name}, got ${value.getClass().name}") } if (value instanceof Instant) { - if (value.nano % NANOS_PER_MICRO != 0) { throw new IllegalArgumentException("Column '$name', row $row: Instant loses sub-microsecond precision") } - try { Math.addExact(Math.multiplyExact(value.epochSecond, 1_000_000L), value.nano.intdiv(NANOS_PER_MICRO)) } - catch (ArithmeticException failure) { throw new IllegalArgumentException("Column '$name', row $row: Instant overflows epoch micros", failure) } + try { TemporalUnits.epochUnits(value, unit) } + catch (ArithmeticException failure) { throw new IllegalArgumentException("Column '$name', row $row: Instant overflows epoch $unit", failure) } } Class current = policyType(value.getClass()) if (current == null || (policy != null && policy != current)) { diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/DatasetBatches.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/DatasetBatches.groovy index 0ce753e36..bad18ee76 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/DatasetBatches.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/DatasetBatches.groovy @@ -14,6 +14,7 @@ import java.util.concurrent.Future /** Ordered shard batches with strict schema checks and at most one prefetched batch per worker. */ final class DatasetBatches implements MatrixBatchIterator { + private final String fallbackName private final List files private final List footers = [] private final List> work = [] @@ -31,7 +32,8 @@ final class DatasetBatches implements MatrixBatchIterator { private Matrix ready /** Checks every footer before reading any rows; supplied shard order is retained. */ - DatasetBatches(List selection, ParquetReadOptions options) { + DatasetBatches(List selection, ParquetReadOptions options, String fallbackName) { + this.fallbackName = fallbackName options.validate() this.options = options if (selection == null) { throw new IllegalArgumentException('Dataset files cannot be null') } @@ -46,7 +48,7 @@ final class DatasetBatches implements MatrixBatchIterator { ParquetReadOptions emptyOptions = ParquetReadOptions.fromMap(options.toMap()) emptyOptions.rowGroups = [] emptyOptions.sourceFileColumn = null - new ParquetBatches(new LocalInputFile(files[0].toPath()), emptyOptions, files[0].name, footers[0]).withCloseable { cursor -> template = cursor.emptyMatrix() } + new ParquetBatches(new LocalInputFile(files[0].toPath()), emptyOptions, fallbackName, footers[0]).withCloseable { cursor -> template = cursor.emptyMatrix() } if (!footers.every { it.fileMetaData.keyValueMetaData[MatrixParquetReader.METADATA_INDEX_COLUMNS] == footers[0].fileMetaData.keyValueMetaData[MatrixParquetReader.METADATA_INDEX_COLUMNS] }) { template = Matrix.builder(template.matrixName).columnNames(template.columnNames()).types(template.types()).build() } @@ -77,6 +79,7 @@ final class DatasetBatches implements MatrixBatchIterator { def schema = footers[0].fileMetaData.schema footers.eachWithIndex { ParquetMetadata footer, int index -> if (footer.fileMetaData.keyValueMetaData[MatrixParquetReader.METADATA_COLUMN_TYPES] != footers[0].fileMetaData.keyValueMetaData[MatrixParquetReader.METADATA_COLUMN_TYPES]) { throw new IllegalArgumentException("Incompatible shard Matrix types: ${files[index]}") } + if (footer.fileMetaData.keyValueMetaData[MatrixParquetReader.METADATA_NESTED_TYPES] != footers[0].fileMetaData.keyValueMetaData[MatrixParquetReader.METADATA_NESTED_TYPES]) { throw new IllegalArgumentException("Incompatible shard nested types: ${files[index]}") } if (footer.fileMetaData.schema.fields != schema.fields) { throw new IllegalArgumentException("Incompatible shard schema: ${files[index]}") } if (options.sourceFileColumn != null && schema.containsField(options.sourceFileColumn)) { throw new IllegalArgumentException('sourceFileColumn collides with dataset schema') } } @@ -113,7 +116,7 @@ final class DatasetBatches implements MatrixBatchIterator { perFile.limit = null perFile.sourceFileColumn = null perFile.rowGroups = [work[index].group] - cursors[index] = new ParquetBatches(new LocalInputFile(files[source].toPath()), perFile, files[source].name, footers[source]) + cursors[index] = new ParquetBatches(new LocalInputFile(files[source].toPath()), perFile, fallbackName, footers[source]) scheduleBatch(index) } diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/DecimalSchemaPolicy.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/DecimalSchemaPolicy.groovy index 1708ddd83..86e95258a 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/DecimalSchemaPolicy.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/DecimalSchemaPolicy.groovy @@ -10,10 +10,17 @@ final class DecimalSchemaPolicy { final Integer precision final Integer scale final String path + final org.apache.parquet.schema.LogicalTypeAnnotation.TimeUnit timestampUnit + final Map nestedTypes final Set used + @SuppressWarnings('ParameterCount') // Child policies share write-scoped settings and metadata accumulators. DecimalSchemaPolicy(Map settings, boolean infer = true, Integer precision = null, - Integer scale = null, String path = '', Set used = new HashSet<>()) { + Integer scale = null, String path = '', Set used = new HashSet<>(), + org.apache.parquet.schema.LogicalTypeAnnotation.TimeUnit timestampUnit = org.apache.parquet.schema.LogicalTypeAnnotation.TimeUnit.MICROS, + Map nestedTypes = [:]) { + this.timestampUnit = timestampUnit + this.nestedTypes = nestedTypes this.settings = settings this.infer = infer this.precision = precision @@ -23,7 +30,7 @@ final class DecimalSchemaPolicy { } DecimalSchemaPolicy child(String name) { - new DecimalSchemaPolicy(settings, infer, precision, scale, path.empty ? name : "$path.$name".toString(), used) + new DecimalSchemaPolicy(settings, infer, precision, scale, NestedPaths.child(path, name), used, timestampUnit, nestedTypes) } int[] resolve(List values, int[] supplied) { diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/GlobSpec.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/GlobSpec.groovy new file mode 100644 index 000000000..7a6af81fc --- /dev/null +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/GlobSpec.groovy @@ -0,0 +1,70 @@ +package se.alipsa.matrix.parquet + +import groovy.transform.PackageScope + +/** Parsed platform-independent glob root, relative matcher and maximum walk depth. */ +@PackageScope +final class GlobSpec { + private static final String SLASH = '/' + private static final String ESCAPE = '\\' + private static final int NOT_FOUND = -1 + + final String root + final String pattern + final int depth + final boolean glob + + private GlobSpec(String root, String pattern, int depth, boolean glob) { + this.root = root + this.pattern = pattern + this.depth = depth + this.glob = glob + } + + static GlobSpec parse(String source, String separator) { + if (source == null || source.empty) { throw new IllegalArgumentException('Glob pattern cannot be empty') } + boolean windows = separator == ESCAPE + String text = windows ? source.replace(ESCAPE, SLASH) : source + if (windows && text ==~ /^[A-Za-z]:[^\/].*/) { throw new IllegalArgumentException("Drive-relative glob not supported: $source") } + boolean escaped = false + boolean escapePresent = false + int first = NOT_FOUND + int lastSlash = NOT_FOUND + for (int index = 0; index < text.length(); index++) { + char ch = text.charAt(index) + if (escaped) { escaped = false; continue } + if (ch == ESCAPE as char) { escaped = true; escapePresent = true; continue } + if (ch in ['*' as char, '?' as char, '[' as char, '{' as char]) { first = index; break } + if (ch == SLASH as char) { lastSlash = index } + } + if (first < 0 && !escapePresent) { return new GlobSpec(text, '', 0, false) } + if (first < 0) { lastSlash = text.lastIndexOf(SLASH) } + String prefix = lastSlash < 0 ? '.' : text.substring(0, lastSlash == 0 || lastSlash == 2 && text.charAt(1) == ':' as char ? lastSlash + 1 : lastSlash) + String relative = text.substring(lastSlash + 1) + StringBuilder literal = new StringBuilder() + escaped = false + prefix.each { String ch -> + if (escaped) { literal.append(ch); escaped = false } + else if (ch == ESCAPE) { escaped = true } + else { literal.append(ch) } + } + int segments = 1 + int braces = 0 + boolean bracket = false + boolean unlimited = false + escaped = false + for (int index = 0; index < relative.length(); index++) { + char ch = relative.charAt(index) + if (escaped) { escaped = false; continue } + if (ch == ESCAPE as char) { escaped = true; continue } + if (ch == '[' as char) { bracket = true } + if (ch == ']' as char) { bracket = false } + if (bracket) { continue } + if (ch == '{' as char) { braces++ } + if (ch == '}' as char) { braces-- } + if (ch == SLASH as char) { segments++; if (braces > 0) { unlimited = true } } + if (ch == '*' as char && index + 1 < relative.length() && relative.charAt(index + 1) == '*' as char) { unlimited = true } + } + new GlobSpec(literal.toString(), relative, unlimited ? Integer.MAX_VALUE : segments, true) + } +} diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/HttpRangeInputFile.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/HttpRangeInputFile.groovy index a425f2466..2fd5ac609 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/HttpRangeInputFile.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/HttpRangeInputFile.groovy @@ -50,7 +50,7 @@ final class HttpRangeInputFile implements InputFile { private Map request(long start, long end) { Map headers = [Range: "bytes=$start-$end".toString()] - if (etag != null) { headers['If-Match'] = etag } + if (etag != null && !etag.startsWith('W/')) { headers['If-Match'] = etag } else if (modified != null) { headers['If-Unmodified-Since'] = modified } URL target = resolvedTarget ?: source HttpURLConnection connection = HttpTransport.open(target, options, headers) diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetReader.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetReader.groovy index e8b0e8f9e..eaa96c935 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetReader.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetReader.groovy @@ -2,7 +2,6 @@ package se.alipsa.matrix.parquet import groovy.transform.PackageScope -import org.apache.parquet.example.data.Group import org.apache.parquet.schema.GroupType import org.apache.parquet.schema.LogicalTypeAnnotation import org.apache.parquet.schema.PrimitiveType @@ -14,7 +13,6 @@ import se.alipsa.matrix.core.util.Logger import java.nio.ByteBuffer import java.nio.file.Files -import java.nio.file.StandardCopyOption import java.sql.Time import java.sql.Timestamp import java.time.DateTimeException @@ -85,30 +83,24 @@ import java.time.ZoneOffset * @see Matrix */ class MatrixParquetReader { + private static final int JSON_PAIR_LENGTH = 2 private static final String OPEN_BRACKET = '[' private static final String EXTENSION = '.parquet' - private static final Logger log = Logger.getLogger(MatrixParquetReader) - /** Metadata key for storing Matrix column types in Parquet file */ + /** Metadata key for canonical nested leaf types by logical path. */ + static final String METADATA_NESTED_TYPES = 'matrix.nestedTypes' + + /** Metadata key for storing Matrix column types in Parquet file. */ static final String METADATA_COLUMN_TYPES = 'matrix.columnTypes' /** Metadata key for storing Matrix index column names in Parquet file */ static final String METADATA_INDEX_COLUMNS = 'matrix.indexColumns' - /** Parquet schema field name for repeated list entries */ - private static final String FIELD_LIST = 'list' - - /** Parquet schema field name for map key-value pairs */ - private static final String FIELD_KEY_VALUE = 'key_value' - /** Thread-local storage for timezone used during read operations */ private static final ThreadLocal ZONE_ID_HOLDER = new ThreadLocal<>() - /** Binary text override used by the retained Group benchmark helper */ - private static final ThreadLocal BINARY_AS_STRING = new ThreadLocal<>() - private static final String ERR_PATH_NULL = 'Path cannot be null' private static final String ERR_FILE_PATH_NULL = 'File path cannot be null' private static final String ERR_CONTENT_NULL = 'Content cannot be null' @@ -221,7 +213,7 @@ class MatrixParquetReader { } /** Opens ordered shard batches; prefetch is bounded by parallelism. */ - MatrixBatchIterator batches(List files) { new DatasetBatches(files, options) } + MatrixBatchIterator batches(List files) { new DatasetBatches(files, options, files ? defaultMatrixName(files[0]) : null) } /** Reads explicit file globs in normalized path order. */ Matrix readGlob(String pattern) { read(globFiles(pattern)) } @@ -231,12 +223,11 @@ class MatrixParquetReader { /** Opens closeable batches; close explicitly when stopping before exhaustion. */ MatrixBatchIterator batches(File file) { - if (file?.isDirectory()) { return batches(directoryFiles(file)) } + if (file?.isDirectory()) { return new DatasetBatches(directoryFiles(file), options, file.name) } validateFile(file) new ParquetBatches(new org.apache.parquet.io.LocalInputFile(file.toPath()), options, defaultMatrixName(file), null, file.canonicalPath) } - /** * Sets the name for the resulting Matrix. * @@ -277,7 +268,7 @@ class MatrixParquetReader { * @return a Matrix populated with the file contents */ Matrix read(File file) { - if (file?.isDirectory()) { return read(directoryFiles(file)) } + if (file?.isDirectory()) { return batches(file).withCloseable { iterator -> collectBatches(iterator) } } validateFile(file) readInput(new org.apache.parquet.io.LocalInputFile(file.toPath()), options, defaultMatrixName(file), file.canonicalPath) } @@ -313,9 +304,9 @@ class MatrixParquetReader { */ Matrix read(InputStream is) { if (is == null) { throw new IllegalArgumentException('InputStream cannot be null') } - File temp = File.createTempFile('matrix-parquet-', EXTENSION) + File temp = createReadTempFile().toFile() try { - Files.copy(is, temp.toPath(), StandardCopyOption.REPLACE_EXISTING) + Files.newOutputStream(temp.toPath()).withCloseable { output -> is.transferTo(output) } readInput(new org.apache.parquet.io.LocalInputFile(temp.toPath()), options, null) } finally { Files.deleteIfExists(temp.toPath()) } } @@ -809,7 +800,6 @@ class MatrixParquetReader { * @throws IllegalArgumentException if file is null or does not exist */ static Matrix read(File file) { - if (file?.isDirectory()) { return builder().read(file) } builder().read(file) } @@ -824,17 +814,23 @@ class MatrixParquetReader { .sort { it.canonicalPath }.toList() } + @PackageScope + static java.nio.file.Path createReadTempFile() { Files.createTempFile('matrix-parquet-', EXTENSION) } + + @PackageScope + static GlobSpec parseGlob(String pattern, String separator) { GlobSpec.parse(pattern, separator) } + private static List globFiles(String pattern) { - java.nio.file.Path path = java.nio.file.Path.of(pattern).toAbsolutePath().normalize() - String text = path as String - int wildcard = text.findIndexOf { it in ['*', '?', OPEN_BRACKET, '{'] } - if (wildcard < 0) { return [path.toFile()] } - int slash = text.lastIndexOf(File.separator, wildcard) - java.nio.file.Path root = java.nio.file.Path.of(text.substring(0, Math.max(1, slash))) - def matcher = java.nio.file.FileSystems.default.getPathMatcher("glob:$text") - Files.walk(root).withCloseable { paths -> - paths.filter { Files.isRegularFile(it) && matcher.matches(it) }.sorted().map { it.toFile() }.toList() - } + GlobSpec spec = parseGlob(pattern, File.separator) + if (!spec.glob) { return [new File(spec.root)] } + java.nio.file.Path root = java.nio.file.Path.of(spec.root) + if (!Files.isDirectory(root)) { throw new IllegalArgumentException("No such glob root '${spec.root}'") } + def matcher = java.nio.file.FileSystems.default.getPathMatcher('glob:' + spec.pattern) + List matches = Files.walk(root, spec.depth).withCloseable { paths -> + paths.filter { Files.isRegularFile(it) && matcher.matches(root.relativize(it)) }.sorted().map { it.toFile() }.toList() + } + if (matches.empty) { throw new IllegalArgumentException("No files match glob '$pattern'") } + matches } private static Matrix collectBatches(MatrixBatchIterator iterator) { @@ -905,6 +901,38 @@ class MatrixParquetReader { value.split(COMMA)*.trim() as String[] } + @PackageScope + static Map parseNestedTypes(String json) { + Map result = [:] + if (json == null) { return result } + String text = json.trim() + if (!text.startsWith('{') || !text.endsWith('}')) { return result } + // Reuse the strict escaped-string parser; separators inside quoted paths stay intact. + StringBuilder array = new StringBuilder(OPEN_BRACKET) + int separators = 0 + boolean quoted = false + boolean escaped = false + for (int index = 1; index < text.length() - 1; index++) { + char ch = text.charAt(index) + if (!quoted && (ch == ':' as char || ch == COMMA as char)) { + char required = separators % JSON_PAIR_LENGTH == 0 ? ':' as char : COMMA as char + if (ch != required) { return [:] } + separators++ + array.append(COMMA) + } + else { array.append(ch) } + if (!escaped && ch == '"' as char) { quoted = !quoted } + if (!escaped && ch == '\\' as char) { escaped = true } else { escaped = false } + } + array.append(']') + try { + List entries = parseJsonStringArray(array.toString()) + if (entries.size() % JSON_PAIR_LENGTH != 0 || !entries.isEmpty() && separators != entries.size() - 1) { return result } + for (int index = 0; index < entries.size(); index += JSON_PAIR_LENGTH) { result[entries[index]] = entries[index + 1] } + } catch (IllegalArgumentException ignored) { return [:] } + result + } + private static List parseJsonStringArray(String json) { List result = [] int pos = skipWhitespace(json, 0) @@ -1001,6 +1029,7 @@ class MatrixParquetReader { private static Class getJavaType(PrimitiveType.PrimitiveTypeName typeName) { switch (typeName) { + case PrimitiveTypeName.INT96 -> LocalDateTime case PrimitiveTypeName.INT32 -> Integer case PrimitiveTypeName.INT64 -> Long case PrimitiveTypeName.FLOAT -> Float @@ -1032,7 +1061,9 @@ class MatrixParquetReader { def primitive = field.asPrimitiveType().primitiveTypeName if (logical != null) { - if (logical == LogicalTypeAnnotation.dateType()) { + if (logical instanceof LogicalTypeAnnotation.IntLogicalTypeAnnotation && !logical.isSigned()) { + unsignedType(logical) + } else if (logical == LogicalTypeAnnotation.dateType()) { LocalDate } else if (logical instanceof LogicalTypeAnnotation.TimestampLogicalTypeAnnotation) { LocalDateTime @@ -1055,43 +1086,10 @@ class MatrixParquetReader { } @PackageScope - static Object readValue(Group group, String fieldName, Type fieldType, Class expectedType) { - if (group.getFieldRepetitionCount(fieldName) == 0) { - return null - } - - if (fieldType.isPrimitive()) { - return readPrimitive(group, fieldName, fieldType.asPrimitiveType(), expectedType) - } - - GroupType groupType = fieldType.asGroupType() - def logical = groupType.logicalTypeAnnotation - if (logical instanceof LogicalTypeAnnotation.ListLogicalTypeAnnotation) { - readList(group, fieldName, groupType) - } else if (logical instanceof LogicalTypeAnnotation.MapLogicalTypeAnnotation) { - readMap(group, fieldName, groupType) - } else { - readStruct(group, fieldName, groupType) - } - } - - private static Object readPrimitive(Group group, String fieldName, PrimitiveType fieldType, Class expectedType) { - Object raw = switch (fieldType.primitiveTypeName) { - case PrimitiveTypeName.INT32 -> group.getInteger(fieldName, 0) - case PrimitiveTypeName.INT64 -> group.getLong(fieldName, 0) - case PrimitiveTypeName.FLOAT -> group.getFloat(fieldName, 0) - case PrimitiveTypeName.DOUBLE -> group.getDouble(fieldName, 0) - case PrimitiveTypeName.BOOLEAN -> group.getBoolean(fieldName, 0) - case PrimitiveTypeName.BINARY, PrimitiveTypeName.FIXED_LEN_BYTE_ARRAY -> group.getBinary(fieldName, 0) - default -> throw new IllegalArgumentException("Unsupported ${fieldType.primitiveTypeName} field $fieldName") - } - decodePrimitive(raw, fieldType, expectedType, BINARY_AS_STRING.get() == Boolean.TRUE) - } - - @PackageScope + @SuppressWarnings('AbcMetric') // Exhaustive physical/logical type decoding. static Object decodePrimitive(Object raw, PrimitiveType field, Class expected, boolean binaryAsString) { if (field.primitiveTypeName == PrimitiveTypeName.INT96) { - throw new IllegalArgumentException("Unsupported INT96 field '${field.name}'; convert legacy timestamps explicitly") + return decodeInt96((org.apache.parquet.io.api.Binary) raw, field.name, expected) } def logical = field.logicalTypeAnnotation if (logical instanceof LogicalTypeAnnotation.DecimalLogicalTypeAnnotation) { @@ -1104,9 +1102,7 @@ class MatrixParquetReader { } if (logical instanceof LogicalTypeAnnotation.TimestampLogicalTypeAnnotation) { Instant instant = timestampInstant(((Number) raw).longValue(), logical.unit) - if (expected == Instant) { return instant } - if (expected == Timestamp) { return Timestamp.from(instant) } - if (expected == Date) { return Date.from(instant) } + if (expected in [Instant, Timestamp, Date]) { return adjustedTimestamp(instant, expected) } return LocalDateTime.ofInstant(instant, logical.isAdjustedToUTC() ? getZoneId() : ZoneOffset.UTC) } if (logical instanceof LogicalTypeAnnotation.TimeLogicalTypeAnnotation) { @@ -1133,9 +1129,35 @@ class MatrixParquetReader { } return raw.bytes } + if (logical instanceof LogicalTypeAnnotation.IntLogicalTypeAnnotation && !logical.isSigned()) { + if (logical.bitWidth == Integer.SIZE) { return Integer.toUnsignedLong(((Number) raw).intValue()) } + if (logical.bitWidth == Long.SIZE) { return new BigInteger(Long.toUnsignedString(((Number) raw).longValue())) } + } narrowInteger(raw, field.name, expected) } + private static Object decodeInt96(org.apache.parquet.io.api.Binary raw, String fieldName, Class expected) { + byte[] bytes = raw.bytes + if (bytes.length != 12) { throw new IllegalArgumentException("Invalid INT96 length for field '$fieldName': ${bytes.length}") } + ByteBuffer buffer = ByteBuffer.wrap(bytes).order(java.nio.ByteOrder.LITTLE_ENDIAN) + long nanos = buffer.getLong() + long day = Integer.toUnsignedLong(buffer.getInt()) - 2_440_588L + if (nanos < 0 || nanos >= NANOS_PER_DAY) { throw new IllegalArgumentException("Invalid INT96 nanos-of-day for field '$fieldName': $nanos") } + adjustedTimestamp(Instant.ofEpochSecond(Math.multiplyExact(day, 86400L), nanos), expected) + } + + private static Object adjustedTimestamp(Instant instant, Class expected) { + if (expected == Instant) { return instant } + if (expected == Timestamp) { return Timestamp.from(instant) } + if (expected == Date) { return Date.from(instant) } + LocalDateTime.ofInstant(instant, getZoneId()) + } + + @PackageScope + static Class unsignedType(LogicalTypeAnnotation.IntLogicalTypeAnnotation logical) { + logical.bitWidth == Integer.SIZE ? Long : logical.bitWidth == Long.SIZE ? BigInteger : Integer + } + private static Object narrowInteger(Object raw, String fieldName, Class expected) { if (expected == Byte) { long value = ((Number) raw).longValue() @@ -1200,47 +1222,4 @@ class MatrixParquetReader { Math.floorMod(value, MICROS_PER_SECOND) * NANOS_PER_MICRO) } - private static List readList(Group group, String fieldName, GroupType groupType) { - List result = [] - Group listGroup = group.getGroup(fieldName, 0) - GroupType repeatedType = groupType.getType(0).asGroupType() - Type elementType = repeatedType.getType(0) - int elementCount = listGroup.getFieldRepetitionCount(FIELD_LIST) - for (int j = 0; j < elementCount; j++) { - Group elementGroup = listGroup.getGroup(FIELD_LIST, j) - def element = readValue(elementGroup, elementType.name, elementType, null) - result << element - } - result - } - - private static Map readMap(Group group, String fieldName, GroupType groupType) { - Map result = [:] - Group mapGroup = group.getGroup(fieldName, 0) - GroupType keyValueType = groupType.getType(0).asGroupType() - Type keyType = keyValueType.getType(0) - Type valueType = keyValueType.getFieldCount() > 1 ? keyValueType.getType(1) : null - int keyValueCount = mapGroup.getFieldRepetitionCount(FIELD_KEY_VALUE) - for (int j = 0; j < keyValueCount; j++) { - Group kvGroup = mapGroup.getGroup(FIELD_KEY_VALUE, j) - def key = readValue(kvGroup, keyType.name, keyType, null) - def value = valueType == null ? null : readValue(kvGroup, valueType.name, valueType, null) - result[key] = value - } - result - } - - private static Map readStruct(Group group, String fieldName, GroupType groupType) { - if (groupType.fields.isEmpty()) { - [:] - } else { - Group structGroup = group.getGroup(fieldName, 0) - Map result = [:] - groupType.fields.each { Type field -> - def value = readValue(structGroup, field.name, field, null) - result[field.name] = value - } - result - } - } } diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetWriter.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetWriter.groovy index 351b683ff..9978f9ea3 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetWriter.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetWriter.groovy @@ -17,7 +17,6 @@ import org.apache.parquet.schema.Types import se.alipsa.matrix.core.Matrix import se.alipsa.matrix.core.util.Logger -import java.beans.PropertyDescriptor import java.math.RoundingMode import java.sql.Time import java.sql.Timestamp @@ -46,6 +45,9 @@ import java.time.ZoneId * {@link #METADATA_COLUMN_TYPES}, enabling {@link MatrixParquetReader} to reconstruct * the original Matrix types when reading.

* + * Timestamp precision defaults to MICROS with sub-microsecond truncation; use + * {@link WriterBuilder#timestampUnit(String)} for full NANOS precision. + * * @see MatrixParquetReader * @see Matrix */ @@ -53,7 +55,10 @@ class MatrixParquetWriter { private static final Logger log = Logger.getLogger(MatrixParquetWriter) - /** Metadata key for storing Matrix column types in Parquet file */ + /** Metadata key for canonical nested leaf types by logical path. */ + static final String METADATA_NESTED_TYPES = 'matrix.nestedTypes' + + /** Metadata key for storing Matrix column types in Parquet file. */ static final String METADATA_COLUMN_TYPES = 'matrix.columnTypes' /** Metadata key for storing Matrix index column names in Parquet file */ @@ -96,7 +101,7 @@ class MatrixParquetWriter { * * // Write to byte array * byte[] bytes = MatrixParquetWriter.builder(matrix) - * .inferPrecisionAndScale(false) + * .inferPrecisionAndScale(false).precision(38).scale(18) * .writeBytes() * } * @@ -117,6 +122,16 @@ class MatrixParquetWriter { static class WriterBuilder { private final Matrix matrix + /** + * Timestamp unit, default MICROS: truncates sub-microsecond digits downward in time for + * LocalDateTime, Timestamp and Instant. NANOS preserves digits but Spark-family readers + * may not support it. NANOS range: 1677-09-21T00:12:43.145224192Z to + * 2262-04-11T23:47:16.854775807Z; out-of-range values fail. MICROS covers about ±292,000 years. + * @param value MICROS or NANOS + * @return this builder + */ + WriterBuilder timestampUnit(String value) { options.timestampUnit(value); this } + /** Sets rowGroupSize; see ParquetWriteOptions for defaults. */ WriterBuilder rowGroupSize(long value) { options.rowGroupSize(value); this } @@ -153,7 +168,7 @@ class MatrixParquetWriter { /** * Sets whether to infer precision and scale for BigDecimal columns (default: {@code true}). * - * @param value {@code true} to infer, {@code false} to fall back to double storage + * @param value {@code true} to infer, {@code false} to require explicit precision/scale or decimalMeta * @return this builder */ WriterBuilder inferPrecisionAndScale(boolean value) { @@ -372,10 +387,9 @@ class MatrixParquetWriter { /** * Writes a Matrix to a Parquet file using uniform decimal precision - * and scale for all BigDecimal columns. This is useful if you don't - * want to infer precision/scale but also don't want to store them as double - * so you can set the precision and scale to a sufficiently large value (e.g., [38, 18] for typical financial calculations) - * so there will be no loss of precision. + * and scale for all BigDecimal columns instead of inferring settings. + * Choose settings large enough for the data (e.g., [38, 18] for typical financial calculations). + * Values that require rounding fail unless a rounding mode is explicitly selected. * * @param matrix the matrix to write * @param fileOrDir the target file or directory @@ -422,7 +436,7 @@ class MatrixParquetWriter { * @param matrix the matrix to write * @param fileOrDir the target file or directory. If directory the file name will be based on the matrix name. * @param decimalMeta a Map where keys are column names (String) and values are int arrays [precision, scale]. - * if set to null or there is no matching entry for a BigDecimal column, that column will default to double storage + * Unmatched BigDecimal paths infer exact settings; metadata for unknown/non-decimal paths is rejected. * @return the target file * @throws IllegalArgumentException if matrix is null, has no columns, or fileOrDir is null */ @@ -551,7 +565,8 @@ class MatrixParquetWriter { private static WriteSchema createSchema(Matrix matrix, ParquetWriteOptions options) { buildSchema(matrix, options.decimalMeta, - new DecimalSchemaPolicy(options.decimalMeta, options.inferPrecisionAndScale, options.precision, options.scale)) + new DecimalSchemaPolicy(options.decimalMeta, options.inferPrecisionAndScale, options.precision, options.scale, '', [] as Set, + LogicalTypeAnnotation.TimeUnit.valueOf(options.timestampUnit))) } /** @@ -620,6 +635,11 @@ class MatrixParquetWriter { private static Map buildExtraMeta(Matrix matrix, WriteSchema resolved) { Map extraMeta = ['matrix.parquet.version': FORMAT_VERSION] extraMeta.put(METADATA_COLUMN_TYPES, matrix.columnNames().collect { String col -> resolved.types[col].metadata.name }.join(COMMA)) + if (!resolved.nestedTypes.isEmpty()) { + extraMeta[METADATA_NESTED_TYPES] = '{' + resolved.nestedTypes.collect { String path, String type -> + "\"${escapeJson(path)}\":\"${escapeJson(type)}\"" + }.join(COMMA) + '}' + } if (matrix.hasIndex()) { extraMeta.put(METADATA_INDEX_COLUMNS, toJsonStringArray(matrix.indexedColumns())) } @@ -631,14 +651,13 @@ class MatrixParquetWriter { def rowCount = matrix.rowCount() def colNames = matrix.columnNames() - (0.. - def group = factory.newGroup() - colNames.each { col -> - def value = matrix[i, col] - if (value != null) { - def fieldType = schema.getType(col) - ParquetValueWriter.writeValue(group, col, fieldType, value, roundingMode) - } + List columns = colNames.collect { (List) matrix.column(it) } + List fieldTypes = colNames.collect { schema.getType(it) } + // Indexed loops avoid per-cell column-name scans, coercion and closure allocation in this hot path. + for (int row = 0; row < rowCount; row++) { + Group group = factory.newGroup() + for (int column = 0; column < colNames.size(); column++) { + ParquetValueWriter.writeValue(group, colNames[column], fieldTypes[column], columns[column][row], roundingMode, colNames[column], row) } parquetWriter.write(group) } @@ -718,11 +737,11 @@ class MatrixParquetWriter { * @return validated physical schema and metadata policies. */ private static WriteSchema buildSchema(Matrix matrix, Map explicitDecimalMeta, DecimalSchemaPolicy decimals = null) { + DecimalSchemaPolicy policy = decimals ?: new DecimalSchemaPolicy(explicitDecimalMeta ?: [:]) Map types = matrix.columnNames().collectEntries { String name -> - [(name): ColumnTypePolicy.resolve(name, matrix.column(name), matrix.type(name))] + [(name): ColumnTypePolicy.resolve(name, matrix.column(name), matrix.type(name), policy.timestampUnit)] } Map decimalMeta = withBigIntegerPrecision(matrix, explicitDecimalMeta, types) - DecimalSchemaPolicy policy = decimals ?: new DecimalSchemaPolicy(explicitDecimalMeta ?: [:]) def builder = Types.buildMessage() matrix.columnNames().each { col -> def type = matrix.type(col) @@ -733,7 +752,7 @@ class MatrixParquetWriter { Set unknown = new HashSet<>(policy.settings.keySet()) unknown.removeAll(policy.used) if (!unknown.empty) { throw new IllegalArgumentException("Unknown/non-decimal metadata paths: $unknown") } - new WriteSchema(builder.named(matrix.matrixName ?: 'MatrixSchema'), types) + new WriteSchema(builder.named(matrix.matrixName ?: 'MatrixSchema'), types, policy.nestedTypes) } /** @@ -799,19 +818,17 @@ class MatrixParquetWriter { * @return a Parquet {@link Type} describing the column */ private static Type buildParquetType(String name, List values, Class clazz, int[] decimalMeta = null, boolean preferStructForMaps = false, DecimalSchemaPolicy decimals = new DecimalSchemaPolicy([:]), ColumnTypePolicy resolved = null) { - resolved = resolved ?: ColumnTypePolicy.resolve(decimals.path ?: name, values, clazz) + boolean nested = resolved == null + resolved = resolved ?: ColumnTypePolicy.resolve(decimals.path ?: name, values, clazz, decimals.timestampUnit) if (resolved.encoding == List) { List nestedValues = extractListElements(values) Type elementType = buildParquetType(FIELD_ELEMENT, nestedValues, Object, null, true, decimals.child(FIELD_ELEMENT)) - if (elementType == null) { - elementType = buildPrimitiveType(FIELD_ELEMENT, String, null, false) - } return Types.optionalList().element(elementType).named(name) } if (resolved.encoding == Map) { - if (preferStructForMaps) { - return buildStructType(name, values, clazz, decimals) + if (preferStructForMaps && values.findAll { it != null }.every { ((Map) it).keySet().every { key -> key instanceof String } }) { + return buildStructType(name, values, decimals) } return buildMapType(name, values, decimals) } @@ -823,14 +840,15 @@ class MatrixParquetWriter { if (effectiveClass == BigInteger && effectiveDecimalMeta == null) { effectiveDecimalMeta = [inferBigIntegerPrecision(values), 0] as int[] } - return buildPrimitiveType(name, effectiveClass, effectiveDecimalMeta, false) + if (nested) { recordNestedType(decimals, resolved) } + return buildPrimitiveType(name, effectiveClass, effectiveDecimalMeta, false, decimals.path ?: name, decimals.timestampUnit) } @SuppressWarnings(['AbcMetric', 'CyclomaticComplexity']) // Exhaustive physical/logical type dispatch. - private static PrimitiveType buildPrimitiveType(String name, Class clazz, int[] decimalMeta = null, boolean required = false) { + private static PrimitiveType buildPrimitiveType(String name, Class clazz, int[] decimalMeta = null, boolean required = false, String path = name, LogicalTypeAnnotation.TimeUnit timestampUnit = LogicalTypeAnnotation.TimeUnit.MICROS) { if (clazz == BigInteger) { if (decimalMeta == null || decimalMeta.length != DECIMAL_META_LENGTH || decimalMeta[0] <= 0 || decimalMeta[1] != 0) { - throw new IllegalArgumentException("BigInteger field '$name' requires decimal metadata [precision, 0]") + throw new IllegalArgumentException("BigInteger field '$path' requires decimal metadata [precision, 0]") } int precision = decimalMeta[0] def builder = required ? Types.required(PrimitiveTypeName.BINARY) : Types.optional(PrimitiveTypeName.BINARY) @@ -850,7 +868,7 @@ class MatrixParquetWriter { .as(LogicalTypeAnnotation.decimalType(scale, precision)) return builder.named(name) } - throw new IllegalArgumentException("BigDecimal field '$name' requires precision/scale") + throw new IllegalArgumentException("BigDecimal field '$path' requires precision/scale") } if (clazz == UUID) { @@ -886,7 +904,7 @@ class MatrixParquetWriter { } case LocalDateTime, Timestamp, Instant -> { primitive = PrimitiveTypeName.INT64 - logical = LogicalTypeAnnotation.timestampType(true, LogicalTypeAnnotation.TimeUnit.MICROS) + logical = LogicalTypeAnnotation.timestampType(true, timestampUnit) } case Time -> { primitive = PrimitiveTypeName.INT32 @@ -896,7 +914,7 @@ class MatrixParquetWriter { primitive = PrimitiveTypeName.INT64 logical = LogicalTypeAnnotation.timestampType(true, LogicalTypeAnnotation.TimeUnit.MILLIS) } - default -> throw new IllegalArgumentException("Unsupported primitive $clazz for field $name") + default -> throw new IllegalArgumentException("Unsupported primitive $clazz for field $path") } def builder = required ? Types.required(primitive) : Types.optional(primitive) @@ -909,18 +927,20 @@ class MatrixParquetWriter { private static Type buildMapType(String name, List values, DecimalSchemaPolicy decimals) { def mapBuilder = Types.optionalMap() List maps = values.findAll { it instanceof Map } as List - if (maps.any { it.containsKey(null) }) { throw new IllegalArgumentException("Map field '$name' cannot contain null keys") } + if (maps.any { it.keySet().any { key -> key == null } }) { throw new IllegalArgumentException("Map field '${decimals.path ?: name}' cannot contain null keys") } List keySamples = maps.collectMany { it?.keySet() ?: [] } as List - Class keyClass = ColumnTypePolicy.resolve(decimals.child(FIELD_KEY).path, keySamples, Object).encoding + ColumnTypePolicy keyPolicy = ColumnTypePolicy.resolve(decimals.child(FIELD_KEY).path, keySamples, Object, decimals.timestampUnit) + Class keyClass = keyPolicy.encoding + recordNestedType(decimals.child(FIELD_KEY), keyPolicy) int[] keyDecimalMeta = keyClass == BigInteger ? [inferBigIntegerPrecision(keySamples), 0] as int[] : keyClass == BigDecimal ? decimals.child(FIELD_KEY).resolve(keySamples, null) : null - PrimitiveType keyType = buildPrimitiveType(FIELD_KEY, keyClass ?: String, keyDecimalMeta, true) + PrimitiveType keyType = buildPrimitiveType(FIELD_KEY, keyClass ?: String, keyDecimalMeta, true, decimals.child(FIELD_KEY).path, decimals.timestampUnit) mapBuilder.key(keyType) List valueSamples = maps.collectMany { it?.values() ?: [] } as List - Set> valueClasses = valueSamples.findAll { it != null }*.class.toSet() - if (valueClasses.size() > 1) { - return buildStructType(name, values, Map, decimals) + Set valueClasses = valueSamples.findAll { it != null }.collect { ColumnTypePolicy.policyType(it.getClass()) }.toSet() + if (valueClasses.size() > 1 || valueClasses.contains(null)) { + return buildStructType(name, values, decimals) } Type valueType if (valueSamples.isEmpty()) { @@ -932,63 +952,28 @@ class MatrixParquetWriter { return mapBuilder.named(name) } - private static Type buildStructType(String name, List values, Class declaredType, DecimalSchemaPolicy decimals) { - Object sample = findFirstNonNull(values) - if (sample == null) { - return Types.optionalGroup().named(name) + private static void recordNestedType(DecimalSchemaPolicy decimals, ColumnTypePolicy policy) { + if (policy.metadata in [Instant, Date, Timestamp, java.sql.Date, Byte, Short, BigInteger]) { + decimals.nestedTypes[decimals.path] = policy.metadata.name } + } + private static Type buildStructType(String name, List values, DecimalSchemaPolicy decimals) { def groupBuilder = Types.optionalGroup() - Map> fieldValues = [:] - Map> fieldTypes = [:] - - if (sample instanceof Map || Map.isAssignableFrom(declaredType)) { - Set keys = new LinkedHashSet<>() - values.each { val -> - if (val instanceof Map mapVal) { - mapVal.keySet().each { key -> - if (!(key instanceof String)) { throw new IllegalArgumentException("Struct field '$name' requires String keys; got ${key?.getClass()?.name}") } - keys.add(key) - } - } - } - keys.each { key -> - List collected = [] - values.each { val -> - if (val instanceof Map mapVal) { - collected.add(mapVal.get(key)) - } else { - collected.add(null) - } - } - fieldValues[key] = collected - fieldTypes[key] = Object - } - } else { - List descriptors = ParquetValueWriter.getPropertyDescriptors(sample.class) - descriptors.each { PropertyDescriptor pd -> - List collected = [] - values.each { val -> - if (val == null) { - collected.add(null) - } else { - def method = pd.readMethod - if (!method.accessible) { - method.accessible = true - } - collected.add(method.invoke(val)) - } + Set keys = new LinkedHashSet<>() + values.each { value -> + if (value instanceof Map) { + value.keySet().each { key -> + if (!(key instanceof String)) { throw new IllegalArgumentException("Struct field '${decimals.path ?: name}' requires String keys; got ${key?.getClass()?.name}") } + keys.add(key) } - fieldValues[pd.name] = collected - fieldTypes[pd.name] = pd.propertyType } } - - fieldValues.each { fieldName, collected -> - Type fieldType = buildParquetType(fieldName, collected, fieldTypes[fieldName] ?: Object, null, false, decimals.child(fieldName)) - groupBuilder.addField(fieldType) + keys.each { String key -> + List collected = values.collect { value -> value == null ? null : ((Map) value)[key] } + groupBuilder.addField(buildParquetType(key, collected, Object, null, false, decimals.child(key))) } - return groupBuilder.named(name) + groupBuilder.named(name) } private static List extractListElements(List values) { @@ -1001,15 +986,6 @@ class MatrixParquetWriter { return elements } - private static Object findFirstNonNull(List values) { - for (def val : values) { - if (val != null) { - return val - } - } - return null - } - private static int minBytesForPrecision(int precision) { int bitsNeeded = BigInteger.TEN.pow(precision).subtract(BigInteger.ONE).bitLength() + 1 Math.ceil(bitsNeeded / 8d) as int @@ -1018,8 +994,10 @@ class MatrixParquetWriter { private static final class WriteSchema { final MessageType schema final Map types + final Map nestedTypes - WriteSchema(MessageType schema, Map types) { + WriteSchema(MessageType schema, Map types, Map nestedTypes) { + this.nestedTypes = nestedTypes this.schema = schema this.types = types } diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/NestedPaths.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/NestedPaths.groovy new file mode 100644 index 000000000..023145ad3 --- /dev/null +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/NestedPaths.groovy @@ -0,0 +1,47 @@ +package se.alipsa.matrix.parquet + +import groovy.transform.PackageScope + +import org.apache.parquet.schema.GroupType +import org.apache.parquet.schema.LogicalTypeAnnotation +import org.apache.parquet.schema.Type + +/** Logical paths omit LIST and MAP physical wrappers. */ +@PackageScope +final class NestedPaths { + private NestedPaths() {} + + static String child(String path, String name) { path.empty ? name : "$path.$name".toString() } + + static boolean isMap(GroupType group, boolean parentIsMap = false) { + group.logicalTypeAnnotation instanceof LogicalTypeAnnotation.MapLogicalTypeAnnotation || + (!parentIsMap && group.logicalTypeAnnotation instanceof LogicalTypeAnnotation.MapKeyValueTypeAnnotation && + group.fieldCount == 1 && !group.getType(0).primitive && group.getType(0).repetition == Type.Repetition.REPEATED && + group.getType(0).asGroupType().fieldCount in [1, 2]) + } + + static boolean wrappedList(GroupType group) { + Type repeated = group.getType(0) + !repeated.primitive && repeated.asGroupType().fieldCount == 1 && + !(repeated.name in ['array', "${group.name}_tuple".toString()]) + } + + static Map leaves(Type type, String path, boolean parentIsMap = false) { + if (type.primitive) { return [(path): type] } + GroupType group = type.asGroupType() + if (group.logicalTypeAnnotation instanceof LogicalTypeAnnotation.ListLogicalTypeAnnotation) { + Type repeated = group.getType(0) + return leaves(wrappedList(group) ? repeated.asGroupType().getType(0) : repeated, child(path, 'element')) + } + if (isMap(group, parentIsMap)) { + Map result = [:] + group.getType(0).asGroupType().fields.eachWithIndex { Type field, int index -> + result.putAll(leaves(field, child(path, index == 0 ? 'key' : 'value'))) + } + return result + } + Map result = [:] + group.fields.each { Type field -> result.putAll(leaves(field, child(path, field.name))) } + result + } +} diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetBatches.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetBatches.groovy index 99505b50b..7afbbab77 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetBatches.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetBatches.groovy @@ -28,6 +28,7 @@ final class ParquetBatches implements MatrixBatchIterator { private final String name private final String sourcePath private final String indexMetadata + private final Map nestedTypes = [:] private final boolean binaryAsString private ColumnMaterializer materializer private RecordReader records @@ -81,6 +82,13 @@ final class ParquetBatches implements MatrixBatchIterator { explicit ?: (binaryAsString && field.primitive && field.asPrimitiveType().logicalTypeAnnotation == null && field.asPrimitiveType().primitiveTypeName == org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.BINARY ? String : inferred[index]) } + Map nested = MatrixParquetReader.parseNestedTypes(metadata[MatrixParquetReader.METADATA_NESTED_TYPES]) + requested.fields.each { Type field -> + NestedPaths.leaves(field, field.name).each { String path, Type leaf -> + Class expected = ColumnTypePolicy.METADATA_TYPES[nested[path]] + if (path != field.name && compatibleMetadata(expected, leaf)) { nestedTypes[path] = expected } + } + } selected = options.rowGroups == null ? (0..(options.rowGroups) selected.each { Integer id -> if (id == null || id < 0 || id >= footer.blocks.size()) { throw new IllegalArgumentException("Invalid row-group ID $id") } @@ -111,7 +119,8 @@ final class ParquetBatches implements MatrixBatchIterator { } } - private static boolean compatibleMetadata(Class expected, Type field) { + @groovy.transform.PackageScope + static boolean compatibleMetadata(Class expected, Type field) { if (expected == null) { return false } if (!field.primitive) { boolean list = field.logicalTypeAnnotation instanceof org.apache.parquet.schema.LogicalTypeAnnotation.ListLogicalTypeAnnotation @@ -125,12 +134,18 @@ final class ParquetBatches implements MatrixBatchIterator { if (logical instanceof org.apache.parquet.schema.LogicalTypeAnnotation.DateLogicalTypeAnnotation) { return expected in [java.sql.Date, java.time.LocalDate, Date] } if (logical instanceof org.apache.parquet.schema.LogicalTypeAnnotation.UUIDLogicalTypeAnnotation) { return expected == UUID } if (MatrixParquetReader.isTextLogicalType(logical)) { return expected == String } + if (logical instanceof org.apache.parquet.schema.LogicalTypeAnnotation.IntLogicalTypeAnnotation) { + if (!logical.isSigned()) { return expected == MatrixParquetReader.unsignedType(logical) } + return logical.bitWidth == 8 ? expected in [Byte, Short, Integer] : logical.bitWidth == 16 ? expected in [Short, Integer] : + expected == (logical.bitWidth == Long.SIZE ? Long : Integer) + } compatiblePhysicalMetadata(expected, primitive.primitiveTypeName) } private static boolean compatiblePhysicalMetadata(Class expected, org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName physical) { switch (physical) { + case org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.INT96 -> expected in [Date, java.sql.Timestamp, java.time.LocalDateTime, java.time.Instant] case org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.INT32 -> expected in [Byte, Short, Integer] case org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.INT64 -> expected == Long case org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.FLOAT -> expected == Float @@ -177,7 +192,7 @@ final class ParquetBatches implements MatrixBatchIterator { pages = reader.readFilteredRowGroup(index) if (pages == null) { continue } rowsRemaining = pages.rowCount - materializer = new ColumnMaterializer(requested, types, binaryAsString, residual) + materializer = new ColumnMaterializer(requested, types, binaryAsString, residual, nestedTypes) def columnIO = new ColumnIOFactory(reader.fileMetaData.createdBy).getColumnIO(requested, reader.fileMetaData.schema, true) records = columnIO.getRecordReader(pages, materializer, filter) } diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetPredicate.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetPredicate.groovy index bd4f46ea5..f1d6e1cbd 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetPredicate.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetPredicate.groovy @@ -17,6 +17,7 @@ import java.time.ZoneId /** Immutable logical predicate; values are converted against the file schema before reading. */ final class ParquetPredicate { + private static final Object NOT_PUSHABLE = new Object() private static final String IS_NOT_NULL = 'isNotNull' private static final String KEY_EQ = 'eq' @@ -95,12 +96,12 @@ final class ParquetPredicate { FilterPredicate pushdown(MessageType schema) { if (column == null) { List predicates = children*.pushdown(schema) - if (predicates.any { it == null }) { return null } - return switch (operation) { - case 'and' -> FilterApi.and(predicates[0], predicates[1]) - case 'or' -> FilterApi.or(predicates[0], predicates[1]) - default -> null + if (operation == 'and') { + // Partial AND relies on always applying the residual and never pushing NOT. + return predicates[0] == null ? predicates[1] : predicates[1] == null ? predicates[0] : FilterApi.and(predicates[0], predicates[1]) } + if (predicates.any { it == null }) { return null } + return operation == 'or' ? FilterApi.or(predicates[0], predicates[1]) : null } TypeCheck.validate(this, schema) PrimitiveType field = schema.getType(column).asPrimitiveType() @@ -109,16 +110,16 @@ final class ParquetPredicate { if (logical instanceof LogicalTypeAnnotation.DecimalLogicalTypeAnnotation && field.primitiveTypeName in [PrimitiveType.PrimitiveTypeName.BINARY, PrimitiveType.PrimitiveTypeName.FIXED_LEN_BYTE_ARRAY]) { return null } if (!(field.primitiveTypeName in [PrimitiveType.PrimitiveTypeName.INT32, PrimitiveType.PrimitiveTypeName.INT64, PrimitiveType.PrimitiveTypeName.BINARY])) { return null } + if (logical instanceof LogicalTypeAnnotation.IntLogicalTypeAnnotation && !logical.isSigned()) { return null } Object physical = physicalValue(field) + if (physical.is(NOT_PUSHABLE)) { return null } def col switch (field.primitiveTypeName) { case PrimitiveType.PrimitiveTypeName.INT32 -> { col = FilterApi.intColumn(column) - if (physical != null) { physical = new BigDecimal(physical.toString()).intValueExact() } } case PrimitiveType.PrimitiveTypeName.INT64 -> { col = FilterApi.longColumn(column) - if (physical != null) { physical = new BigDecimal(physical.toString()).longValueExact() } } case PrimitiveType.PrimitiveTypeName.BINARY -> { col = FilterApi.binaryColumn(column) @@ -147,35 +148,45 @@ final class ParquetPredicate { TypeCheck.validate(this, schema) PrimitiveType field = schema.getType(column).asPrimitiveType() Object normalized = value - if (field.logicalTypeAnnotation instanceof LogicalTypeAnnotation.TimestampLogicalTypeAnnotation && - ((LogicalTypeAnnotation.TimestampLogicalTypeAnnotation) field.logicalTypeAnnotation).isAdjustedToUTC() && value instanceof LocalDateTime) { + if ((field.primitiveTypeName == PrimitiveType.PrimitiveTypeName.INT96 || + (field.logicalTypeAnnotation instanceof LogicalTypeAnnotation.TimestampLogicalTypeAnnotation && + ((LogicalTypeAnnotation.TimestampLogicalTypeAnnotation) field.logicalTypeAnnotation).isAdjustedToUTC())) && value instanceof LocalDateTime) { normalized = value.atZone(zone ?: ZoneId.systemDefault()).toInstant() } ParquetPredicate bound = new ParquetPredicate(operation, column, normalized) - bound.physicalValue(field) bound } private Object physicalValue(PrimitiveType field) { + try { physicalValueExact(field) } + catch (ArithmeticException | IllegalArgumentException ignored) { NOT_PUSHABLE } + } + + @SuppressWarnings('AbcMetric') // Exhaustive exact physical conversion with residual fallback. + private Object physicalValueExact(PrimitiveType field) { if (value == null) { return null } def logical = field.logicalTypeAnnotation + if (logical instanceof LogicalTypeAnnotation.IntLogicalTypeAnnotation && !logical.isSigned()) { return NOT_PUSHABLE } if (logical instanceof LogicalTypeAnnotation.DecimalLogicalTypeAnnotation) { BigDecimal decimal = new BigDecimal(value.toString()).setScale(logical.scale, java.math.RoundingMode.UNNECESSARY) if (decimal.precision() > logical.precision) { throw new IllegalArgumentException("Decimal predicate overflows '${field.name}'") } - return decimal.unscaledValue() + BigInteger unscaled = decimal.unscaledValue() + return field.primitiveTypeName == PrimitiveType.PrimitiveTypeName.INT32 ? (Object) unscaled.intValueExact() : + field.primitiveTypeName == PrimitiveType.PrimitiveTypeName.INT64 ? (Object) unscaled.longValueExact() : unscaled } if (value instanceof LocalDate) { return Math.toIntExact(value.toEpochDay()) } if (logical instanceof LogicalTypeAnnotation.TimeLogicalTypeAnnotation) { long nanos = ((LocalTime) value).toNanoOfDay() long divisor = logical.unit == LogicalTypeAnnotation.TimeUnit.MILLIS ? 1_000_000L : logical.unit == LogicalTypeAnnotation.TimeUnit.MICROS ? 1000L : 1L if (nanos % divisor != 0) { throw new IllegalArgumentException("Lossy TIME predicate '${field.name}'") } - return nanos.intdiv(divisor) + long units = nanos.intdiv(divisor) + return field.primitiveTypeName == PrimitiveType.PrimitiveTypeName.INT32 ? (Object) Math.toIntExact(units) : (Object) units } if (logical instanceof LogicalTypeAnnotation.TimestampLogicalTypeAnnotation) { Instant instant = value instanceof Instant ? (Instant) value : ((LocalDateTime) value).toInstant(java.time.ZoneOffset.UTC) long divisor = logical.unit == LogicalTypeAnnotation.TimeUnit.MILLIS ? 1_000_000L : logical.unit == LogicalTypeAnnotation.TimeUnit.MICROS ? 1000L : 1L if (instant.nano % divisor != 0) { throw new IllegalArgumentException("Lossy TIMESTAMP predicate '${field.name}'") } - return Math.addExact(Math.multiplyExact(instant.epochSecond, 1_000_000_000L.intdiv(divisor)), instant.nano.intdiv(divisor)) + return TemporalUnits.epochUnits(instant, logical.unit) } if (field.primitiveTypeName == PrimitiveType.PrimitiveTypeName.INT32) { return new BigDecimal(value.toString()).intValueExact() } if (field.primitiveTypeName == PrimitiveType.PrimitiveTypeName.INT64) { return new BigDecimal(value.toString()).longValueExact() } @@ -197,7 +208,8 @@ final class ParquetPredicate { boolean compatible = Number.isAssignableFrom(inferred) ? value instanceof Number : inferred == java.sql.Time ? value instanceof LocalTime : inferred == LocalDateTime ? (value instanceof LocalDateTime || - (value instanceof Instant && ((LogicalTypeAnnotation.TimestampLogicalTypeAnnotation) field.logicalTypeAnnotation).isAdjustedToUTC())) : inferred.isInstance(value) + (value instanceof Instant && (field.asPrimitiveType().primitiveTypeName == PrimitiveType.PrimitiveTypeName.INT96 || + ((LogicalTypeAnnotation.TimestampLogicalTypeAnnotation) field.logicalTypeAnnotation).isAdjustedToUTC()))) : inferred.isInstance(value) if (!compatible) { throw new IllegalArgumentException("Incompatible predicate value for '${predicate.column}': expected $inferred") } } } diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetValueWriter.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetValueWriter.groovy index 6e093b1d1..8523f6036 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetValueWriter.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetValueWriter.groovy @@ -10,8 +10,6 @@ import org.apache.parquet.schema.PrimitiveType import org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName import org.apache.parquet.schema.Type -import java.beans.Introspector -import java.beans.PropertyDescriptor import java.math.RoundingMode import java.nio.ByteBuffer import java.sql.Time @@ -21,7 +19,6 @@ import java.time.LocalDate import java.time.LocalDateTime import java.time.LocalTime import java.time.ZoneId -import java.util.concurrent.ConcurrentHashMap /** * Writes individual Matrix cell values into Parquet {@link Group} records. @@ -45,14 +42,8 @@ final class ParquetValueWriter { /** Thread-local storage for timezone used during write operations */ private static final ThreadLocal ZONE_ID_HOLDER = new ThreadLocal<>() - /** Cache for PropertyDescriptor lists by class to avoid repeated reflection calls */ - private static final Map, List> PROPERTY_DESCRIPTOR_CACHE = - new ConcurrentHashMap<>() - private static final int ERROR_VALUE_MAX_LENGTH = 100 - private static final long MICROS_PER_SECOND = 1_000_000L private static final long NANOS_PER_MILLI = 1_000_000L - private static final long NANOS_PER_MICRO = 1_000L private ParquetValueWriter() {} @@ -88,61 +79,45 @@ final class ParquetValueWriter { return zoneId != null ? zoneId : ZoneId.systemDefault() } - /** - * Gets cached PropertyDescriptors for the given class. - * Uses Introspector to get bean info and caches the result for subsequent calls. - */ - static List getPropertyDescriptors(Class clazz) { - return PROPERTY_DESCRIPTOR_CACHE.computeIfAbsent(clazz) { c -> - def beanInfo = Introspector.getBeanInfo(c, Object) - return beanInfo.propertyDescriptors.findAll { it.readMethod != null } - } - } - - static void writeValue(Group group, String fieldName, Type fieldType, Object value, RoundingMode roundingMode) { + static void writeValue(Group group, String fieldName, Type fieldType, Object value, RoundingMode roundingMode, String path, int row) { if (value == null) { return } if (fieldType.isPrimitive()) { - writePrimitiveValue(group, fieldName, fieldType.asPrimitiveType(), value, roundingMode) + try { writePrimitiveValue(group, fieldName, fieldType.asPrimitiveType(), value, roundingMode, path, row) } + catch (ArithmeticException failure) { throw new IllegalArgumentException("Column '$path', row $row: timestamp overflows epoch units", failure) } return } GroupType groupType = fieldType.asGroupType() def logical = groupType.logicalTypeAnnotation if (logical instanceof LogicalTypeAnnotation.ListLogicalTypeAnnotation) { - writeList(group, fieldName, groupType, value, roundingMode) + writeList(group, fieldName, groupType, value, roundingMode, path, row) } else if (logical instanceof LogicalTypeAnnotation.MapLogicalTypeAnnotation) { - writeMap(group, fieldName, groupType, value, roundingMode) + writeMap(group, fieldName, groupType, value, roundingMode, path, row) } else { - writeStruct(group, fieldName, groupType, value, roundingMode) + writeStruct(group, fieldName, groupType, value, roundingMode, path, row) } } static void writePrimitiveValue(Group group, String fieldName, PrimitiveType field, Object value, - RoundingMode roundingMode) { + RoundingMode roundingMode, String path, int row) { switch (value.class) { case Byte, Short, Integer, int -> group.append(fieldName, (int) value) case Long, long -> group.append(fieldName, ((Number) value).longValue()) case BigInteger -> group.add(fieldName, Binary.fromConstantByteArray(((BigInteger) value).toByteArray())) case Float, float -> group.append(fieldName, ((Number) value).floatValue()) case Double, double -> group.append(fieldName, ((Number) value).doubleValue()) - case BigDecimal -> writeBigDecimalValue(group, fieldName, field, (BigDecimal) value, roundingMode) + case BigDecimal -> writeBigDecimalValue(group, fieldName, field, (BigDecimal) value, roundingMode, path, row) case Boolean, boolean -> group.append(fieldName, (boolean) value) case LocalDate -> group.append(fieldName, ((LocalDate) value).toEpochDay().intValue()) case java.sql.Date -> group.append(fieldName, ((java.sql.Date) value).toLocalDate().toEpochDay().intValue()) case Time -> group.append(fieldName, timeToMillis((Time) value)) - case LocalDateTime -> group.append(fieldName, instantToMicros(((LocalDateTime) value).atZone(getZoneId()).toInstant())) - case Timestamp -> group.append(fieldName, instantToMicros(((Timestamp) value).toInstant())) + case LocalDateTime -> group.append(fieldName, TemporalUnits.epochUnits(((LocalDateTime) value).atZone(getZoneId()).toInstant(), ((LogicalTypeAnnotation.TimestampLogicalTypeAnnotation) field.logicalTypeAnnotation).unit)) + case Timestamp -> group.append(fieldName, TemporalUnits.epochUnits(((Timestamp) value).toInstant(), ((LogicalTypeAnnotation.TimestampLogicalTypeAnnotation) field.logicalTypeAnnotation).unit)) case byte[] -> group.add(fieldName, Binary.fromConstantByteArray((byte[]) value)) case LocalTime -> group.append(fieldName, ((LocalTime) value).toNanoOfDay()) - case Instant -> { - Instant instant = (Instant) value - if (instant.nano % NANOS_PER_MICRO != 0) { - throw new IllegalArgumentException("Instant field '$fieldName' loses sub-microsecond precision") - } - group.append(fieldName, instantToMicros(instant)) - } + case Instant -> group.append(fieldName, TemporalUnits.epochUnits((Instant) value, ((LogicalTypeAnnotation.TimestampLogicalTypeAnnotation) field.logicalTypeAnnotation).unit)) case UUID -> { UUID uuid = (UUID) value group.add(fieldName, Binary.fromConstantByteArray(ByteBuffer.allocate(16).putLong(uuid.mostSignificantBits).putLong(uuid.leastSignificantBits).array())) @@ -150,33 +125,30 @@ final class ParquetValueWriter { case String, Character, CharSequence -> group.append(fieldName, value.toString()) case Enum -> group.append(fieldName, ((Enum) value).name()) case Date -> group.append(fieldName, ((Date) value).time) - default -> throw new IllegalArgumentException("Unsupported ${value.getClass().name} for field $fieldName") + default -> throw new IllegalArgumentException("Unsupported ${value.getClass().name} for field $path, row $row") } } private static int timeToMillis(Time time) { def localTime = Instant.ofEpochMilli(time.time).atZone(ZoneId.systemDefault()).toLocalTime() - (int) (localTime.toNanoOfDay() / NANOS_PER_MILLI) - } - - private static long instantToMicros(Instant instant) { - Math.addExact(Math.multiplyExact(instant.epochSecond, MICROS_PER_SECOND), (long) (instant.nano / NANOS_PER_MICRO)) + (int) (localTime.toNanoOfDay().intdiv(NANOS_PER_MILLI)) } private static void writeBigDecimalValue(Group group, String fieldName, PrimitiveType field, BigDecimal value, - RoundingMode roundingMode) { + RoundingMode roundingMode, String path, int row) { def logical = field.getLogicalTypeAnnotation() if (field.primitiveTypeName == PrimitiveTypeName.FIXED_LEN_BYTE_ARRAY && logical instanceof LogicalTypeAnnotation.DecimalLogicalTypeAnnotation) { writeFixedLenBigDecimal(group, fieldName, field, value, - (LogicalTypeAnnotation.DecimalLogicalTypeAnnotation) logical, roundingMode) + (LogicalTypeAnnotation.DecimalLogicalTypeAnnotation) logical, roundingMode, path, row) } else { - throw new IllegalArgumentException("Missing DECIMAL annotation for $fieldName") + throw new IllegalArgumentException("Missing DECIMAL annotation for $path, row $row") } } + @SuppressWarnings('ParameterCount') // Encoding context includes logical path and row for nested diagnostics. private static void writeFixedLenBigDecimal(Group group, String fieldName, PrimitiveType field, BigDecimal value, - LogicalTypeAnnotation.DecimalLogicalTypeAnnotation logical, RoundingMode roundingMode) { + LogicalTypeAnnotation.DecimalLogicalTypeAnnotation logical, RoundingMode roundingMode, String path, int row) { int scale = logical.scale int precision = logical.precision BigDecimal rescaled @@ -184,12 +156,12 @@ final class ParquetValueWriter { rescaled = value.setScale(scale, roundingMode) } catch (ArithmeticException exception) { throw new IllegalArgumentException( - "BigDecimal value '$value' for field '$fieldName' cannot be represented at scale $scale with roundingMode $roundingMode. " + + "BigDecimal value '$value' for field '$path', row $row cannot be represented at scale $scale with roundingMode $roundingMode. " + 'Use an exact value or explicitly select a rounding mode such as HALF_UP.', exception) } if (rescaled.precision() > precision) { throw new IllegalArgumentException( - "BigDecimal value '$value' for field '$fieldName' exceeds DECIMAL($precision, $scale) after applying roundingMode $roundingMode. " + + "BigDecimal value '$value' for field '$path', row $row exceeds DECIMAL($precision, $scale) after applying roundingMode $roundingMode. " + "Rescaled value '$rescaled' has precision ${rescaled.precision()}. Increase the precision or choose an appropriate rounding mode.") } def unscaled = rescaled.unscaledValue() @@ -197,7 +169,7 @@ final class ParquetValueWriter { int size = field.typeLength if (bytes.length > size) { throw new IllegalArgumentException( - "BigDecimal value '$value' for field '$fieldName' cannot be encoded in DECIMAL($precision, $scale). " + + "BigDecimal value '$value' for field '$path', row $row cannot be encoded in DECIMAL($precision, $scale). " + "The value requires ${bytes.length} bytes but schema allows only $size bytes " + "after applying roundingMode $roundingMode. Increase the precision or use inferPrecisionAndScale=true.") } @@ -209,10 +181,10 @@ final class ParquetValueWriter { group.add(fieldName, Binary.fromConstantByteArray(padded)) } - private static void writeList(Group group, String fieldName, GroupType groupType, Object value, RoundingMode roundingMode) { + private static void writeList(Group group, String fieldName, GroupType groupType, Object value, RoundingMode roundingMode, String path, int row) { if (!(value instanceof Collection)) { throw new IllegalArgumentException( - "Cannot write field '$fieldName' as Parquet LIST: expected a Collection (List, Set, etc.) " + + "Cannot write field '$path', row $row as Parquet LIST: expected a Collection (List, Set, etc.) " + "but got ${value.class.simpleName}. Value: ${truncateForError(value)}") } Collection collection = (Collection) value @@ -221,14 +193,14 @@ final class ParquetValueWriter { Type elementType = repeatedType.getType(0) collection.each { Object element -> Group entry = listGroup.addGroup(FIELD_LIST) - writeValue(entry, elementType.name, elementType, element, roundingMode) + writeValue(entry, elementType.name, elementType, element, roundingMode, NestedPaths.child(path, 'element'), row) } } - private static void writeMap(Group group, String fieldName, GroupType groupType, Object value, RoundingMode roundingMode) { + private static void writeMap(Group group, String fieldName, GroupType groupType, Object value, RoundingMode roundingMode, String path, int row) { if (!(value instanceof Map)) { throw new IllegalArgumentException( - "Cannot write field '$fieldName' as Parquet MAP: expected a Map " + + "Cannot write field '$path', row $row as Parquet MAP: expected a Map " + "but got ${value.class.simpleName}. Value: ${truncateForError(value)}") } Map mapValue = (Map) value @@ -238,39 +210,22 @@ final class ParquetValueWriter { Type valueType = keyValueType.getFieldCount() > 1 ? keyValueType.getType(1) : null mapValue.each { Object k, Object v -> Group kvGroup = mapGroup.addGroup(FIELD_KEY_VALUE) - writePrimitiveValue(kvGroup, keyPrimitive.name, keyPrimitive, k, roundingMode) + writeValue(kvGroup, keyPrimitive.name, keyPrimitive, k, roundingMode, NestedPaths.child(path, 'key'), row) if (valueType != null && v != null) { - writeValue(kvGroup, valueType.name, valueType, v, roundingMode) + writeValue(kvGroup, valueType.name, valueType, v, roundingMode, NestedPaths.child(path, 'value'), row) } } } - private static void writeStruct(Group group, String fieldName, GroupType groupType, Object value, RoundingMode roundingMode) { - Map structValues = toStructMap(value) + private static void writeStruct(Group group, String fieldName, GroupType groupType, Object value, RoundingMode roundingMode, String path, int row) { + Map structValues = (Map) value Group structGroup = group.addGroup(fieldName) groupType.fields.each { Type field -> def childValue = structValues.get(field.name) if (childValue != null) { - writeValue(structGroup, field.name, field, childValue, roundingMode) - } - } - } - - private static Map toStructMap(Object value) { - if (value instanceof Map mapValue) { - Map result = [:] - mapValue.each { k, v -> result[String.valueOf(k)] = v } - return result - } - Map map = [:] - getPropertyDescriptors(value.class).each { PropertyDescriptor pd -> - def method = pd.readMethod - if (!method.accessible) { - method.accessible = true + writeValue(structGroup, field.name, field, childValue, roundingMode, NestedPaths.child(path, field.name), row) } - map[pd.name] = method.invoke(value) } - return map } /** diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetWriteOptions.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetWriteOptions.groovy index 8aec792f2..5d54a92a9 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetWriteOptions.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetWriteOptions.groovy @@ -12,6 +12,10 @@ import java.time.ZoneId * Typed options for Parquet write operations via the SPI. */ class ParquetWriteOptions { + private static final String MICROS = 'MICROS' + private static final String ERR_CODEC_NULL = 'compressionCodec cannot be null' + private static final String KEY_BLOOM_ENABLED = 'bloomFilterEnabled' + private static final String KEY_BLOOM_NDV = 'bloomFilterNDV' private static final String KEY_DICTIONARY_ENCODING = 'dictionaryEncoding' private static final String KEY_WRITER_VERSION = 'writerVersion' private static final String KEY_SIZE_STATISTICS_ENABLED = 'sizeStatisticsEnabled' @@ -25,6 +29,25 @@ class ParquetWriteOptions { private static final String KEY_ROUNDING_MODE = 'roundingMode' private static final String ERR_ROUNDING_MODE_NULL = 'roundingMode cannot be null' + String timestampUnit = MICROS + + /** + * Sets timestamp precision (default MICROS). MICROS truncates sub-microsecond digits + * downward in time for LocalDateTime, Timestamp and Instant; NANOS preserves them. + * Spark-family readers may not support TIMESTAMP(NANOS). NANOS covers + * 1677-09-21T00:12:43.145224192Z to 2262-04-11T23:47:16.854775807Z; + * values outside that range fail. MICROS covers about ±292,000 years. + * @param value MICROS or NANOS, trimmed and case-insensitive + * @return this options instance + */ + ParquetWriteOptions timestampUnit(String value) { + if (value == null) { throw new IllegalArgumentException('timestampUnit cannot be null') } + String unit = value.trim().toUpperCase(Locale.ROOT) + if (!(unit in [MICROS, 'NANOS'])) { throw new IllegalArgumentException("Unknown timestampUnit '$value'; use MICROS or NANOS") } + timestampUnit = unit + this + } + long rowGroupSize = 134217728L /** Sets rowGroupSize (default 134217728L); target sizes are not hard memory limits. */ @@ -127,7 +150,11 @@ class ParquetWriteOptions { * @throws IllegalArgumentException if value is not a recognized codec name */ ParquetWriteOptions compressionCodec(String value) { - this.compressionCodec = CompressionCodecName.valueOf(value.toUpperCase()) + if (value == null) { throw new IllegalArgumentException(ERR_CODEC_NULL) } + try { this.compressionCodec = CompressionCodecName.valueOf(value.trim().toUpperCase(Locale.ROOT)) } + catch (IllegalArgumentException failure) { + throw new IllegalArgumentException("Unknown compressionCodec '$value'; valid names: ${CompressionCodecName.values()*.name()}", failure) + } this } @@ -171,13 +198,20 @@ class ParquetWriteOptions { } void validate() { + timestampUnit(timestampUnit) if (rowGroupSize <= 0 || pageSize <= 0 || columnIndexTruncateLength <= 0 || statisticsTruncateLength <= 0) { throw new IllegalArgumentException('Writer sizes/truncate lengths must be positive') } if (!(writerVersion in [KEY_V1, 'V2'])) { throw new IllegalArgumentException('writerVersion must be V1 or V2') } if (bloomFilterEnabled == null || bloomFilterNDV == null) { throw new IllegalArgumentException('Bloom option maps cannot be null') } - if (bloomFilterEnabled.values().any { it == null }) { throw new IllegalArgumentException('Bloom overrides must be non-null Booleans') } - if (bloomFilterNDV.values().any { it == null || it <= 0 }) { throw new IllegalArgumentException('Bloom NDVs must be positive') } + bloomFilterEnabled.each { String path, Object value -> + if (!(value instanceof Boolean)) { throw new IllegalArgumentException("bloomFilterEnabled['$path'] must be a non-null Boolean") } + } + bloomFilterNDV.each { String path, Object value -> + if (!(value instanceof Number) || integralLong(value, "bloomFilterNDV['$path']") <= 0) { + throw new IllegalArgumentException("bloomFilterNDV['$path'] must be a positive integer") + } + } if ((precision == null) != (scale == null)) { throw new IllegalArgumentException('precision and scale must be provided together') @@ -195,7 +229,7 @@ class ParquetWriteOptions { } decimalMeta = normalizeDecimalMeta(decimalMeta) if (compressionCodec == null) { - throw new IllegalArgumentException('compressionCodec cannot be null') + throw new IllegalArgumentException(ERR_CODEC_NULL) } if (roundingMode == null) { throw new IllegalArgumentException(ERR_ROUNDING_MODE_NULL) @@ -216,6 +250,7 @@ class ParquetWriteOptions { if (zoneId != null) { result.zoneId = zoneId } + result.timestampUnit = timestampUnit result.putAll([rowGroupSize: rowGroupSize, pageSize: pageSize, dictionaryEncoding: dictionaryEncoding, writerVersion: writerVersion, columnIndexTruncateLength: columnIndexTruncateLength, statisticsTruncateLength: statisticsTruncateLength, bloomFilterEnabled: bloomFilterEnabled, bloomFilterNDV: bloomFilterNDV, sizeStatisticsEnabled: sizeStatisticsEnabled]) result } @@ -228,6 +263,7 @@ class ParquetWriteOptions { Set unknown = new HashSet<>(normalized.keySet()) unknown.removeAll(known) if (!unknown.empty) { throw new IllegalArgumentException("Unknown Parquet options: $unknown") } + if (normalized.containsKey('timestampunit')) { result.timestampUnit((String) normalized.timestampunit) } normalized = normalized.findAll { String key, Object value -> value != null } if (normalized.containsKey('inferprecisionandscale')) { Object inferPrecisionAndScale = normalized.inferprecisionandscale @@ -288,13 +324,37 @@ class ParquetWriteOptions { if (normalized.containsKey('writerversion')) { result.writerVersion = String.valueOf(normalized.writerversion) } if (normalized.containsKey('columnindextruncatelength')) { result.columnIndexTruncateLength = ((Number) normalized.columnindextruncatelength).intValue() } if (normalized.containsKey('statisticstruncatelength')) { result.statisticsTruncateLength = ((Number) normalized.statisticstruncatelength).intValue() } - if (normalized.containsKey('bloomfilterenabled')) { result.bloomFilterEnabled = (Map) normalized.bloomfilterenabled } - if (normalized.containsKey('bloomfilterndv')) { result.bloomFilterNDV = (Map) normalized.bloomfilterndv } + if (normalized.containsKey('bloomfilterenabled')) { + result.bloomFilterEnabled = requireMap(normalized.bloomfilterenabled, KEY_BLOOM_ENABLED).collectEntries { key, value -> + [(String.valueOf(key)): OptionMaps.booleanValueOrNull(value, "bloomFilterEnabled['$key']")] + } + } + if (normalized.containsKey('bloomfilterndv')) { + result.bloomFilterNDV = requireMap(normalized.bloomfilterndv, KEY_BLOOM_NDV).collectEntries { key, value -> + [(String.valueOf(key)): integralLong(value, "bloomFilterNDV['$key']")] + } + } if (normalized.containsKey('sizestatisticsenabled')) { result.sizeStatisticsEnabled = OptionMaps.booleanValueOrNull(normalized.sizestatisticsenabled, KEY_SIZE_STATISTICS_ENABLED) } result.validate() result } + private static Map requireMap(Object value, String name) { + if (!(value instanceof Map)) { throw new IllegalArgumentException("$name must be a Map") } + value + } + + private static long integralLong(Object value, String name) { + if (!(value instanceof Byte || value instanceof Short || value instanceof Integer || value instanceof Long || + value instanceof BigInteger || value instanceof BigDecimal || value instanceof CharSequence)) { + throw new IllegalArgumentException("$name must be an integral number within long range") + } + try { new BigDecimal(value.toString().trim()).longValueExact() } + catch (ArithmeticException | NumberFormatException failure) { + throw new IllegalArgumentException("$name must be an integral number within long range", failure) + } + } + /** * Normalizes and validates a raw {@code decimalMeta} map into {@code Map}. * @@ -358,6 +418,7 @@ class ParquetWriteOptions { static List descriptors() { ParquetWriteOptions defaults = new ParquetWriteOptions() [ + new OptionDescriptor('timestampUnit', String, defaults.timestampUnit, 'MICROS truncates sub-microsecond digits; NANOS preserves precision within int64 range'), new OptionDescriptor(KEY_INFER_PRECISION_AND_SCALE, Boolean, Boolean.TRUE.toString(), 'Infer precision and scale for BigDecimal columns'), new OptionDescriptor(KEY_PRECISION, Integer, null, 'Uniform precision for all BigDecimal columns'), new OptionDescriptor(KEY_SCALE, Integer, null, 'Uniform scale for all BigDecimal columns'), @@ -371,8 +432,8 @@ class ParquetWriteOptions { new OptionDescriptor(KEY_WRITER_VERSION, String, defaults.writerVersion, 'V1 or V2 encodings and page layout'), new OptionDescriptor('columnIndexTruncateLength', Integer, defaults.columnIndexTruncateLength, 'Truncated binary page bounds bytes'), new OptionDescriptor('statisticsTruncateLength', Integer, defaults.statisticsTruncateLength, 'Truncated row-group binary statistics bytes'), - new OptionDescriptor('bloomFilterEnabled', Map, null, 'Per-column bloom-filter enabled overrides'), - new OptionDescriptor('bloomFilterNDV', Map, null, 'Per-column positive expected distinct values'), + new OptionDescriptor(KEY_BLOOM_ENABLED, Map, null, 'Per-column bloom-filter enabled overrides'), + new OptionDescriptor(KEY_BLOOM_NDV, Map, null, 'Per-column positive expected distinct values'), new OptionDescriptor(KEY_SIZE_STATISTICS_ENABLED, Boolean, defaults.sizeStatisticsEnabled, 'Collect column size statistics') ] } diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/TemporalUnits.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/TemporalUnits.groovy new file mode 100644 index 000000000..ddbe959ac --- /dev/null +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/TemporalUnits.groovy @@ -0,0 +1,27 @@ +package se.alipsa.matrix.parquet + +import groovy.transform.PackageScope + +import org.apache.parquet.schema.LogicalTypeAnnotation.TimeUnit + +import java.time.Instant + +/** Overflow-safe epoch conversion shared by writers and exact predicate conversion. */ +@PackageScope +final class TemporalUnits { + private TemporalUnits() {} + + static long nanosPerUnit(TimeUnit unit) { + unit == TimeUnit.MILLIS ? 1_000_000L : unit == TimeUnit.MICROS ? 1000L : 1L + } + + static long epochUnits(Instant instant, TimeUnit unit) { + long divisor = nanosPerUnit(unit) + long units = 1_000_000_000L.intdiv(divisor) + // Sub-microsecond digits are intentionally truncated under MICROS; see req/v0.8.0-fixes.md decision D. + long fraction = instant.nano.intdiv(divisor) + instant.epochSecond < 0 ? + Math.addExact(Math.multiplyExact(instant.epochSecond + 1L, units), fraction - units) : + Math.addExact(Math.multiplyExact(instant.epochSecond, units), fraction) + } +} diff --git a/matrix-parquet/src/test/groovy/MatrixParquetBigIntegerTest.groovy b/matrix-parquet/src/test/groovy/MatrixParquetBigIntegerTest.groovy index 6276d950f..08c2121ba 100644 --- a/matrix-parquet/src/test/groovy/MatrixParquetBigIntegerTest.groovy +++ b/matrix-parquet/src/test/groovy/MatrixParquetBigIntegerTest.groovy @@ -129,7 +129,7 @@ class MatrixParquetBigIntegerTest { Matrix matrix = MatrixParquetReader.read(file) - assertEquals([huge as BigDecimal, BigInteger.ONE as BigDecimal], matrix.bigs[0]) + assertEquals([huge, BigInteger.ONE], matrix.bigs[0]) } @Test @@ -157,6 +157,6 @@ class MatrixParquetBigIntegerTest { Matrix matrix = MatrixParquetReader.read(file) - assertEquals([(key as BigDecimal): 'a'], matrix.m[0]) + assertEquals([(key): 'a'], matrix.m[0]) } } diff --git a/matrix-parquet/src/test/groovy/ParquetDatasetAndHttpTest.groovy b/matrix-parquet/src/test/groovy/ParquetDatasetAndHttpTest.groovy index 257de97a9..1980b37b7 100644 --- a/matrix-parquet/src/test/groovy/ParquetDatasetAndHttpTest.groovy +++ b/matrix-parquet/src/test/groovy/ParquetDatasetAndHttpTest.groovy @@ -171,4 +171,39 @@ class ParquetDatasetAndHttpTest { assertThrows(IOException) { input.newStream().withCloseable { it.read() } } } finally { server.stop(0) } } + @Test + void weakEtagsUseLastModifiedAndStillDetectChanges() { + byte[] bytes = MatrixParquetWriter.writeBytes(Matrix.builder().columns(id: [1, 2]).build()) + AtomicLong ifMatch = new AtomicLong() + AtomicLong unmodified = new AtomicLong() + def version = new java.util.concurrent.atomic.AtomicReference('W/"v1"') + def server = HttpServer.create(new InetSocketAddress('127.0.0.1', 0), 0) + server.createContext('/weak') { exchange -> + if (exchange.requestHeaders.getFirst('If-Match') != null) { + ifMatch.incrementAndGet(); exchange.sendResponseHeaders(412, -1); exchange.close(); return + } + if (exchange.requestHeaders.getFirst('If-Unmodified-Since') != null) { unmodified.incrementAndGet() } + def match = exchange.requestHeaders.getFirst('Range') =~ /bytes=(\d+)-(\d+)/ + assertTrue(match.matches()) + int start = Integer.parseInt(match.group(1)), end = Integer.parseInt(match.group(2)) + exchange.responseHeaders.add('ETag', version.get()) + exchange.responseHeaders.add('Last-Modified', 'Wed, 01 Jan 2025 00:00:00 GMT') + exchange.responseHeaders.add('Content-Range', "bytes $start-$end/${bytes.length}") + exchange.sendResponseHeaders(206, end - start + 1) + exchange.responseBody.write(bytes, start, end - start + 1) + exchange.close() + } + server.start() + try { + URL url = new URI("http://127.0.0.1:${server.address.port}/weak").toURL() + ['RANGE', 'AUTO'].each { mode -> assertEquals([1, 2], MatrixParquetReader.builder().remoteReadMode(mode).read(url).column('id')) } + assertEquals(0L, ifMatch.get()) + assertTrue(unmodified.get() > 0) + def input = new HttpRangeInputFile(url, new ParquetReadOptions(remoteReadMode: 'RANGE')) + version.set('W/"v2"') + def failure = assertThrows(IOException) { input.newStream().withCloseable { it.read() } } + assertTrue(failure.message.contains('Remote object/range changed during read')) + } finally { server.stop(0) } + } + } diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/GroupValueReader.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/GroupValueReader.groovy new file mode 100644 index 000000000..2ffd1a02d --- /dev/null +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/GroupValueReader.groovy @@ -0,0 +1,90 @@ +package se.alipsa.matrix.parquet + +import groovy.transform.PackageScope + +import org.apache.parquet.example.data.Group +import org.apache.parquet.schema.* +import org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName + +/** Test-only baseline for the former Group materialization path. */ +class GroupValueReader { + private static final String FIELD_LIST = 'list' + private static final String FIELD_KEY_VALUE = 'key_value' + @PackageScope + static Object readValue(Group group, String fieldName, Type fieldType, Class expectedType) { + if (group.getFieldRepetitionCount(fieldName) == 0) { + return null + } + + if (fieldType.isPrimitive()) { + return readPrimitive(group, fieldName, fieldType.asPrimitiveType(), expectedType) + } + + GroupType groupType = fieldType.asGroupType() + def logical = groupType.logicalTypeAnnotation + if (logical instanceof LogicalTypeAnnotation.ListLogicalTypeAnnotation) { + readList(group, fieldName, groupType) + } else if (logical instanceof LogicalTypeAnnotation.MapLogicalTypeAnnotation) { + readMap(group, fieldName, groupType) + } else { + readStruct(group, fieldName, groupType) + } + } + + private static Object readPrimitive(Group group, String fieldName, PrimitiveType fieldType, Class expectedType) { + Object raw = switch (fieldType.primitiveTypeName) { + case PrimitiveTypeName.INT32 -> group.getInteger(fieldName, 0) + case PrimitiveTypeName.INT64 -> group.getLong(fieldName, 0) + case PrimitiveTypeName.FLOAT -> group.getFloat(fieldName, 0) + case PrimitiveTypeName.DOUBLE -> group.getDouble(fieldName, 0) + case PrimitiveTypeName.BOOLEAN -> group.getBoolean(fieldName, 0) + case PrimitiveTypeName.BINARY, PrimitiveTypeName.FIXED_LEN_BYTE_ARRAY, PrimitiveTypeName.INT96 -> group.getBinary(fieldName, 0) + default -> throw new IllegalArgumentException("Unsupported ${fieldType.primitiveTypeName} field $fieldName") + } + MatrixParquetReader.decodePrimitive(raw, fieldType, expectedType, false) + } + + private static List readList(Group group, String fieldName, GroupType groupType) { + List result = [] + Group listGroup = group.getGroup(fieldName, 0) + GroupType repeatedType = groupType.getType(0).asGroupType() + Type elementType = repeatedType.getType(0) + int elementCount = listGroup.getFieldRepetitionCount(FIELD_LIST) + for (int j = 0; j < elementCount; j++) { + Group elementGroup = listGroup.getGroup(FIELD_LIST, j) + def element = readValue(elementGroup, elementType.name, elementType, null) + result << element + } + result + } + + private static Map readMap(Group group, String fieldName, GroupType groupType) { + Map result = [:] + Group mapGroup = group.getGroup(fieldName, 0) + GroupType keyValueType = groupType.getType(0).asGroupType() + Type keyType = keyValueType.getType(0) + Type valueType = keyValueType.getFieldCount() > 1 ? keyValueType.getType(1) : null + int keyValueCount = mapGroup.getFieldRepetitionCount(FIELD_KEY_VALUE) + for (int j = 0; j < keyValueCount; j++) { + Group kvGroup = mapGroup.getGroup(FIELD_KEY_VALUE, j) + def key = readValue(kvGroup, keyType.name, keyType, null) + def value = valueType == null ? null : readValue(kvGroup, valueType.name, valueType, null) + result[key] = value + } + result + } + + private static Map readStruct(Group group, String fieldName, GroupType groupType) { + if (groupType.fields.isEmpty()) { + [:] + } else { + Group structGroup = group.getGroup(fieldName, 0) + Map result = [:] + groupType.fields.each { Type field -> + def value = readValue(structGroup, field.name, field, null) + result[field.name] = value + } + result + } + } +} diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetFixFixtures.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetFixFixtures.groovy new file mode 100644 index 000000000..ce4add8d3 --- /dev/null +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetFixFixtures.groovy @@ -0,0 +1,26 @@ +package se.alipsa.matrix.parquet + +import org.apache.parquet.conf.PlainParquetConfiguration +import org.apache.parquet.example.data.simple.SimpleGroupFactory +import org.apache.parquet.hadoop.ParquetFileReader +import org.apache.parquet.hadoop.example.ExampleParquetWriter +import org.apache.parquet.io.LocalOutputFile +import org.apache.parquet.schema.MessageType + +/** External-file fixtures without Matrix writer metadata or conversion. */ +class ParquetFixFixtures { + static File write(File directory, String name, MessageType schema, Map metadata = [:], Closure rows) { + File file = new File(directory, name + '.parquet') + ExampleParquetWriter.builder(new LocalOutputFile(file.toPath())).withConf(new PlainParquetConfiguration()) + .withType(schema).withExtraMetaData(metadata).build().withCloseable { writer -> + rows.call(writer, new SimpleGroupFactory(schema)) + } + file + } + + static Map footer(byte[] bytes) { + ParquetFileReader.open(new InMemoryInputFile(bytes), org.apache.parquet.ParquetReadOptions.builder(new PlainParquetConfiguration()).build()).withCloseable { reader -> + [schema: reader.fileMetaData.schema, metadata: reader.fileMetaData.keyValueMetaData] + } + } +} diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetInt96Test.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetInt96Test.groovy new file mode 100644 index 000000000..e3c9d75d0 --- /dev/null +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetInt96Test.groovy @@ -0,0 +1,43 @@ +package se.alipsa.matrix.parquet + +import static org.junit.jupiter.api.Assertions.* + +import org.apache.parquet.io.api.Binary +import org.apache.parquet.schema.MessageTypeParser +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir + +import java.nio.ByteBuffer +import java.nio.ByteOrder +import java.time.* + +class ParquetInt96Test { + @TempDir File directory + + @Test + void legacyTimestampsUseUtcAndReaderZoneAndMetadata() { + def schema = MessageTypeParser.parseMessageType('message legacy { optional int96 ts; }') + def instants = [Instant.EPOCH, Instant.parse('1969-12-31T23:59:59.5Z'), Instant.parse('2024-01-01T00:00:00.123456789Z')] + [false, true].each { metadata -> + File file = ParquetFixFixtures.write(directory, "int96-$metadata", schema, metadata ? ['matrix.columnTypes': Instant.name, 'matrix.parquet.version': '2'] : [:]) { writer, factory -> + instants.each { value -> + long day = Math.floorDiv(value.epochSecond, 86400L) + long nanos = Math.floorMod(value.epochSecond, 86400L) * 1_000_000_000L + value.nano + byte[] bytes = ByteBuffer.allocate(12).order(ByteOrder.LITTLE_ENDIAN).putLong(nanos).putInt((int) (day + 2_440_588L)).array() + writer.write(factory.newGroup().append('ts', Binary.fromConstantByteArray(bytes))) + } + } + def result = MatrixParquetReader.builder().zoneId('Europe/Stockholm').read(file) + assertEquals(metadata ? Instant : LocalDateTime, result.type('ts')) + assertEquals(metadata ? instants : instants.collect { LocalDateTime.ofInstant(it, ZoneId.of('Europe/Stockholm')) }, result.column('ts')) + [Predicates.eq('ts', instants[2]), Predicates.gt('ts', Instant.EPOCH), Predicates.eq('ts', LocalDateTime.ofInstant(instants[2], ZoneId.of('Europe/Stockholm')))].each { predicate -> + assertEquals(1, MatrixParquetReader.builder().zoneId('Europe/Stockholm').filter(predicate).read(file).rowCount()) + assertNull(predicate.pushdown(schema)) + } + } + def field = schema.getType('ts').asPrimitiveType() + assertTrue(assertThrows(IllegalArgumentException) { MatrixParquetReader.decodePrimitive(Binary.fromConstantByteArray(new byte[11]), field, null, false) }.message.contains('ts')) + byte[] invalid = ByteBuffer.allocate(12).order(ByteOrder.LITTLE_ENDIAN).putLong(-1L).putInt(2_440_588).array() + assertThrows(IllegalArgumentException) { MatrixParquetReader.decodePrimitive(Binary.fromConstantByteArray(invalid), field, null, false) } + } +} diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetLayoutBenchmarkTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetLayoutBenchmarkTest.groovy index 7d11a1456..3dd2de7a3 100644 --- a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetLayoutBenchmarkTest.groovy +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetLayoutBenchmarkTest.groovy @@ -32,4 +32,18 @@ class ParquetLayoutBenchmarkTest { } } } + @Test + void wideMatrixWriteThroughput() { + int rows = 50000 + int columns = 200 + def matrix = se.alipsa.matrix.core.Matrix.builder('wide').columns((0.. + [("column$index".toString()): (0.. + def (annotation, repeatedAnnotation, wrapper, key, value) = shape + String valueField = value == null ? '' : "optional int32 $value;" + def schema = MessageTypeParser.parseMessageType("message external { optional group m ($annotation) { repeated group $wrapper$repeatedAnnotation { required binary $key (STRING); $valueField } } }") + File file = ParquetFixFixtures.write(directory, "map-$index", schema) { writer, factory -> + def row = factory.newGroup(); def map = row.addGroup('m') + ['a', 'b'].eachWithIndex { text, number -> + def pair = map.addGroup(wrapper).append(key, text) + if (value != null) { pair.append(value, number + 1) } + } + writer.write(row) + } + def result = MatrixParquetReader.read(file) + assertEquals(Map, result.type('m')) + assertEquals(value == null ? [a: null, b: null] : [a: 1, b: 2], result[0, 'm']) + } + def schema = MessageTypeParser.parseMessageType('message struct { optional group m { optional int32 key_value; } }') + File file = ParquetFixFixtures.write(directory, 'struct', schema) { writer, factory -> + def row = factory.newGroup(); row.addGroup('m').append('key_value', 7); writer.write(row) + } + assertEquals([key_value: 7], MatrixParquetReader.read(file)[0, 'm']) + } +} diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedTypesTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedTypesTest.groovy new file mode 100644 index 000000000..052056d33 --- /dev/null +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedTypesTest.groovy @@ -0,0 +1,79 @@ +package se.alipsa.matrix.parquet + +import static org.junit.jupiter.api.Assertions.* + +import org.apache.parquet.schema.MessageTypeParser +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir + +import se.alipsa.matrix.core.Matrix + +import java.sql.Time +import java.sql.Timestamp +import java.time.* + +class ParquetNestedTypesTest { + @TempDir File directory + + @Test + void leafTypesSurviveListsMapKeysValuesAndStructs() { + [Instant.parse('2024-01-01T00:00:00.123456Z'), new Date(1234), Timestamp.from(Instant.EPOCH), + java.sql.Date.valueOf('2024-01-01'), (byte) 12, (short) 1234, BigInteger.TEN, + LocalTime.of(1, 2, 3, 456789123), Time.valueOf('12:00:00')].each { value -> + def data = Matrix.builder().columns(l: [[value]], m: [[(value): value]], s: [[[field: value]]]).types(List, Map, List).build() + byte[] bytes = MatrixParquetWriter.writeBytes(data) + def result = MatrixParquetReader.builder().zoneId('Pacific/Honolulu').read(bytes) + def actuals = [result[0, 'l'][0], result[0, 'm'].keySet().first(), result[0, 'm'].values().first(), result[0, 's'][0].field] + actuals.each { actual -> assertEquals(value, actual); assertEquals(value.getClass(), actual.getClass()) } + def metadata = ParquetFixFixtures.footer(bytes).metadata + if (value instanceof LocalTime || value instanceof Time) { assertFalse(metadata.containsKey('matrix.nestedTypes'), "$value ${value.getClass()} $metadata") } + } + } + + @Test + void logicalPathsProjectionAndDatasets() { + def data = Matrix.builder().columns(l: [[[Instant.EPOCH]]], m: [[a: [(byte) 1]]]).types(List, Map).build() + byte[] bytes = MatrixParquetWriter.writeBytes(data) + def nested = MatrixParquetReader.parseNestedTypes(ParquetFixFixtures.footer(bytes).metadata['matrix.nestedTypes']) + assertEquals(['l.element.element': Instant.name, 'm.value.element': Byte.name], nested) + assertEquals(Instant.EPOCH, MatrixParquetReader.read(bytes)[0, 'l'][0][0]) + assertEquals(Byte, MatrixParquetReader.builder().columns(['m']).read(bytes)[0, 'm'].a[0].getClass()) + File first = new File(directory, 'a.parquet'); first.bytes = bytes + File second = new File(directory, 'b.parquet'); second.bytes = bytes + assertEquals(2, MatrixParquetReader.read([first, second]).rowCount()) + def other = Matrix.builder().columns(l: [[[LocalDateTime.of(1970, 1, 1, 0, 0)]]], m: [[a: [(byte) 1]]]).types(List, Map).build() + second.bytes = MatrixParquetWriter.writeBytes(other) + assertThrows(IllegalArgumentException) { MatrixParquetReader.read([first, second]) } + } + + @Test + void legacyTwoLevelPathsAndUntrustedEntries() { + def schema = MessageTypeParser.parseMessageType('message legacy { optional group l (LIST) { repeated int32 array; } }') + ['java.lang.Short', 'java.lang.String', 'no.such.Class', null, 'malformed'].eachWithIndex { type, index -> + String json = type == null ? null : type == 'malformed' ? '{broken}' : "{\"l.element\":\"$type\"}" + File file = ParquetFixFixtures.write(directory, "legacy-$index", schema, json == null ? [:] : ['matrix.nestedTypes': json]) { writer, factory -> + def row = factory.newGroup(); row.addGroup('l').append('array', 12); writer.write(row) + } + def value = MatrixParquetReader.read(file)[0, 'l'][0] + assertEquals(12, value) + assertEquals(type == 'java.lang.Short' ? Short : Integer, value.getClass()) + } + assertEquals([:], MatrixParquetReader.parseNestedTypes('{"l.element","java.lang.Short"}')) + assertEquals(['a"b': 'java.lang.Short'], MatrixParquetReader.parseNestedTypes('{"a\\"b":"java.lang.Short"}')) + } + @Test + void nestedMetadataCannotOverrideTopLevelTypesEvenWithDottedColumnNames() { + def schema = MessageTypeParser.parseMessageType('message external { optional int32 value; }') + File file = ParquetFixFixtures.write(directory, 'top-level-guard', schema, + ['matrix.nestedTypes': '{"value":"java.lang.Short"}']) { writer, factory -> + writer.write(factory.newGroup().append('value', 12)) + } + def result = MatrixParquetReader.read(file) + assertEquals(Integer, result.type('value')) + assertEquals(Integer, result[0, 'value'].getClass()) + byte[] dotted = MatrixParquetWriter.writeBytes(Matrix.builder().columns(['a.b': [(byte) 1]]).build()) + assertFalse(ParquetFixFixtures.footer(dotted).metadata.containsKey('matrix.nestedTypes')) + assertEquals(Byte, MatrixParquetReader.read(dotted).type('a.b')) + } + +} diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedWriteTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedWriteTest.groovy new file mode 100644 index 000000000..9e5b8bac1 --- /dev/null +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedWriteTest.groovy @@ -0,0 +1,35 @@ +package se.alipsa.matrix.parquet + +import static org.junit.jupiter.api.Assertions.* + +import org.apache.parquet.schema.LogicalTypeAnnotation +import org.junit.jupiter.api.Test + +import se.alipsa.matrix.core.Matrix + +class ParquetNestedWriteTest { + @Test + void concreteContainerClassesShareOneEncoding() { + [ [a: List.of(1), b: new ArrayList([1, 2, 3]), c: Collections.emptyList()], + [a: new LinkedHashMap([x: 1]), b: new HashMap([x: 2]), c: new TreeMap([x: 3])], + [a: "x${1}", b: 'y'], [(1): "x${1}", (2): 'y'], + [(1): List.of(1), (2): List.of(1, 2, 3)] ].each { map -> + byte[] bytes = MatrixParquetWriter.writeBytes(Matrix.builder().columns(m: [map]).types(Map).build()) + assertInstanceOf(LogicalTypeAnnotation.MapLogicalTypeAnnotation, ParquetFixFixtures.footer(bytes).schema.getType('m').logicalTypeAnnotation) + assertEquals(map.collectEntries { key, value -> [(key): value instanceof CharSequence ? value.toString() : value] }, MatrixParquetReader.read(bytes)[0, 'm']) + } + } + + @Test + void integerKeyedListElementsAreMapsAndStringKeyedElementsRemainStructs() { + [[(1): 'one'], [a: 'one']].each { map -> + byte[] bytes = MatrixParquetWriter.writeBytes(Matrix.builder().columns(l: [[map]]).types(List).build()) + def element = ParquetFixFixtures.footer(bytes).schema.getType('l').asGroupType().getType(0).asGroupType().getType(0) + if (map.containsKey(1)) { assertInstanceOf(LogicalTypeAnnotation.MapLogicalTypeAnnotation, element.logicalTypeAnnotation) } + else { assertNull(element.logicalTypeAnnotation) } + assertEquals([map], MatrixParquetReader.read(bytes)[0, 'l']) + } + byte[] bytes = MatrixParquetWriter.writeBytes(Matrix.builder().columns(s: ["x${1}"]).types(String).build()) + assertEquals('x1', MatrixParquetReader.read(bytes)[0, 's']) + } +} diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetPredicateRepresentabilityTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetPredicateRepresentabilityTest.groovy new file mode 100644 index 000000000..c54af0c92 --- /dev/null +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetPredicateRepresentabilityTest.groovy @@ -0,0 +1,80 @@ +package se.alipsa.matrix.parquet + +import static org.junit.jupiter.api.Assertions.* + +import org.apache.parquet.io.LocalInputFile +import org.apache.parquet.schema.MessageTypeParser +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir + +import se.alipsa.matrix.core.Matrix + +import java.time.* + +class ParquetPredicateRepresentabilityTest { + @TempDir File directory + + @Test + void unrepresentableValuesUseExactResidualsAcrossCompositionsBatchesAndDatasets() { + def schema = MessageTypeParser.parseMessageType('''message external { + optional int32 d (DECIMAL(3,2)); optional int32 i; + optional int64 ts (TIMESTAMP(MILLIS,true)); optional int32 t (TIME(MILLIS,true)); }''') + File file = ParquetFixFixtures.write(directory, 'values', schema) { writer, factory -> + writer.write(factory.newGroup().append('d', 123).append('i', 1).append('ts', 0L).append('t', 43200000)) + writer.write(factory.newGroup().append('d', 250).append('i', 2).append('ts', 1L).append('t', 43200001)) + } + def rows = [[d: 1.23G, i: 1, ts: Instant.EPOCH, t: LocalTime.NOON], + [d: 2.50G, i: 2, ts: Instant.EPOCH.plusMillis(1), t: LocalTime.NOON.plusNanos(1000000)]] + [Predicates.gt('d', 1.005G), Predicates.eq('d', 1.005G), Predicates.le('d', 2.505G), + Predicates.gt('i', 1.5G), Predicates.eq('i', 10_000_000_000L), Predicates.lt('i', 10_000_000_000L), + Predicates.ne('i', Double.NaN), Predicates.lt('ts', Instant.EPOCH.plusNanos(1000)), + Predicates.lt('t', LocalTime.NOON.plusNanos(1))].each { predicate -> + assertNull(predicate.pushdown(schema)) + [predicate, Predicates.and(predicate, Predicates.ge('i', 1)), Predicates.or(predicate, Predicates.eq('i', 2)), Predicates.not(predicate)].each { composed -> + def expected = rows.findAll { composed.test(it) }*.i + assertEquals(expected, MatrixParquetReader.builder().filter(composed).read(file).column('i')) + assertEquals(expected, MatrixParquetReader.builder().filter(composed).read([file]).column('i')) + List actual = [] + MatrixParquetReader.builder().maxBatchRows(1).filter(composed).batches(file).withCloseable { batches -> + while (batches.hasNext()) { actual.addAll(batches.next().column('i')) } + } + assertEquals(expected, actual) + } + } + } + + @Test + void exactIntegerDecimalTimeAndDateValuesRemainPushable() { + def schema = MessageTypeParser.parseMessageType('''message exact { + optional int32 d (DECIMAL(5,2)); optional int64 big (DECIMAL(12,2)); + optional int32 t (TIME(MILLIS,true)); optional int32 date (DATE); }''') + File file = ParquetFixFixtures.write(directory, 'exact', schema) { writer, factory -> + writer.write(factory.newGroup().append('d', 250).append('big', 250L).append('t', 43200000).append('date', 0)) + } + [Predicates.gt('d', 1.25G), Predicates.gt('big', 1.25G), Predicates.ge('t', LocalTime.NOON), Predicates.eq('date', LocalDate.ofEpochDay(0))].each { predicate -> + assertNotNull(predicate.pushdown(schema)) + assertEquals(1, MatrixParquetReader.builder().filter(predicate).read(file).rowCount()) + } + } + + @Test + void partialAndStillPrunesAndRemainsExactUnderOrAndNot() { + File file = new File(directory, 'pruning.parquet') + def data = Matrix.builder().columns(i: (0..<20000).collect { it % 3 }, j: (0..<20000).toList()).build() + MatrixParquetWriter.builder(data).compressionCodec('UNCOMPRESSED').dictionaryEncoding(false).pageSize(1024).write(file) + def partial = Predicates.and(Predicates.gt('i', 1.5G), Predicates.eq('j', 19997)) + def schema = ParquetFixFixtures.footer(file.bytes).schema + assertNotNull(partial.pushdown(schema)) + List counts = [] + [false, true].each { indexed -> + def input = new CountingInput(new LocalInputFile(file.toPath())) + def options = new ParquetReadOptions(filter: partial, useColumnIndexFilter: indexed, useRowGroupFilters: false) + assertEquals([19997], MatrixParquetReader.readInput(input, options, null).column('j')) + counts.add(input.totalBytes) + } + assertTrue(counts[1] < counts[0]) + [Predicates.or(partial, Predicates.eq('j', 7)), Predicates.not(partial)].each { predicate -> + assertEquals((0..<20000).findAll { predicate.test([i: it % 3, j: it]) }, MatrixParquetReader.builder().filter(predicate).read(file).column('j')) + } + } +} diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetReadBenchmarkTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetReadBenchmarkTest.groovy index 4ff865b5b..4cac66715 100644 --- a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetReadBenchmarkTest.groovy +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetReadBenchmarkTest.groovy @@ -64,7 +64,7 @@ class ParquetReadBenchmarkTest { Group row = reader.read() while (row != null) { def values = [] - names.eachWithIndex { String name, int index -> values << MatrixParquetReader.readValue(row, name, schema.getType(name), types[index]) } + names.eachWithIndex { String name, int index -> values << GroupValueReader.readValue(row, name, schema.getType(name), types[index]) } builder.addRow(values as Object[]) row = reader.read() } diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetReaderIoTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetReaderIoTest.groovy new file mode 100644 index 000000000..6a55e8f5c --- /dev/null +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetReaderIoTest.groovy @@ -0,0 +1,78 @@ +package se.alipsa.matrix.parquet + +import static org.junit.jupiter.api.Assertions.* +import static org.junit.jupiter.api.Assumptions.assumeTrue + +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir + +import se.alipsa.matrix.core.Matrix + +import java.nio.file.Files +import java.nio.file.attribute.PosixFilePermission + +class ParquetReaderIoTest { + @TempDir File directory + + @Test + void temporaryReadFilesAreOwnerOnly() { + def path = MatrixParquetReader.createReadTempFile() + try { + assumeTrue(Files.getFileStore(path).supportsFileAttributeView('posix')) + assertEquals([PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE] as Set, Files.getPosixFilePermissions(path)) + } finally { Files.deleteIfExists(path) } + } + + @Test + void datasetNamesAreConsistentForBatchesListsDirectoriesAndGlobs() { + File file = new File(directory, 'part-0.parquet') + MatrixParquetWriter.write(Matrix.builder().columns(id: [1]).build(), file) + assertEquals(directory.name, MatrixParquetReader.read(directory).matrixName) + assertEquals('part-0', MatrixParquetReader.read([file]).matrixName) + assertEquals('part-0', MatrixParquetReader.readGlob(directory.path + '/*.parquet').matrixName) + [directory, [file]].each { source -> + assertEquals('chosen', MatrixParquetReader.builder().matrixName('chosen').read(source).matrixName) + MatrixParquetReader.builder().matrixName('chosen').batches(source).withCloseable { cursor -> + assertEquals('chosen', cursor.emptyMatrix().matrixName) + assertEquals('chosen', cursor.next().matrixName) + } + MatrixParquetReader.builder().batches(source).withCloseable { cursor -> + assertEquals(source instanceof File ? directory.name : 'part-0', cursor.next().matrixName) + } + } + } + + @Test + void globParsingSupportsWindowsSeparatorsAndFixedDepth() { + [['*.parquet', '/', '.', '*.parquet', 1], ['{a,b}/*.parquet', '/', '.', '{a,b}/*.parquet', 2], + ['/*.parquet', '/', '/', '*.parquet', 1], ['C:\\data\\*.parquet', '\\', 'C:/data', '*.parquet', 1], + ['C:\\data\\sub\\*.parquet', '\\', 'C:/data/sub', '*.parquet', 1], + ['C:\\data\\{a,b}\\*.parquet', '\\', 'C:/data', '{a,b}/*.parquet', 2], + ['C:\\data\\**\\*.parquet', '\\', 'C:/data', '**/*.parquet', Integer.MAX_VALUE], + ['C:\\*.parquet', '\\', 'C:/', '*.parquet', 1]].each { source, separator, root, pattern, depth -> + def spec = MatrixParquetReader.parseGlob(source, separator) + assertEquals(root, spec.root); assertEquals(pattern, spec.pattern); assertEquals(depth, spec.depth) + } + assertThrows(IllegalArgumentException) { MatrixParquetReader.parseGlob('C:*.parquet', '\\') } + assertEquals(Integer.MAX_VALUE, MatrixParquetReader.parseGlob('root/{a,b/c}/*.parquet', '/').depth) + } + + @Test + void globsRespectRootsEscapesDepthAndErrors() { + byte[] bytes = MatrixParquetWriter.writeBytes(Matrix.builder().columns(id: [1]).build()) + ['one.parquet', 'a/two.parquet', 'b/c/three.parquet', 'a{b}/four.parquet'].each { name -> + File file = new File(directory, name); file.parentFile.mkdirs(); file.bytes = bytes + } + assertEquals(1, MatrixParquetReader.readGlob(directory.path + '/*.parquet').rowCount()) + assertEquals(3, MatrixParquetReader.readGlob(directory.path + '/**/*.parquet').rowCount()) + assertEquals(2, MatrixParquetReader.readGlob(directory.path + '/{a,b/c}/*.parquet').rowCount()) + assertEquals(1, MatrixParquetReader.parseGlob(directory.path + '/*.parquet', File.separator).depth) + assertTrue(assertThrows(IllegalArgumentException) { MatrixParquetReader.readGlob(directory.path + '/missing/*.parquet') }.message.contains('No such glob root')) + assertTrue(assertThrows(IllegalArgumentException) { MatrixParquetReader.readGlob(directory.path + '/*.missing') }.message.contains('No files match glob')) + assumeTrue(File.separator == '/') + new File(directory, 'x*.parquet').bytes = bytes + new File(directory, 'xy.parquet').bytes = bytes + assertEquals(1, MatrixParquetReader.readGlob(directory.path + '/x\\*.parquet').rowCount()) + assertEquals(1, MatrixParquetReader.readGlob(directory.path + '/a\\{b\\}/*.parquet').rowCount()) + } +} diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetReviewRegressionTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetReviewRegressionTest.groovy index 01d70fb25..f969dfd02 100644 --- a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetReviewRegressionTest.groovy +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetReviewRegressionTest.groovy @@ -234,4 +234,16 @@ class ParquetReviewRegressionTest { MatrixParquetReader.builder().columns(['id']).filter(Predicates.isNull('payload')).read(file) } } + @Test + void nestedFailuresCarryCompleteLogicalPaths() { + [[Matrix.builder().columns(l: [[10.00G]]).types(List).build(), 'l.element', + new ParquetWriteOptions().decimalMeta(['l.element': [3, 2] as int[]])], + [Matrix.builder().columns(l: [[[(null): 1]]]).types(List).build(), 'l.element', new ParquetWriteOptions()], + [Matrix.builder().columns(mm: [[a: [(null): 1]]]).types(Map).build(), 'mm.value', new ParquetWriteOptions()], + [Matrix.builder().columns(l: [[new Object()]]).types(List).build(), 'l.element', new ParquetWriteOptions()]].each { matrix, path, options -> + def failure = assertThrows(IllegalArgumentException) { MatrixParquetWriter.writeBytes(matrix, options) } + assertTrue(failure.message.contains(path), failure.message) + } + } + } diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy new file mode 100644 index 000000000..60d0fa026 --- /dev/null +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy @@ -0,0 +1,92 @@ +package se.alipsa.matrix.parquet + +import static org.junit.jupiter.api.Assertions.* + +import org.apache.parquet.schema.LogicalTypeAnnotation.TimeUnit +import org.junit.jupiter.api.Test + +import se.alipsa.matrix.core.Matrix + +import java.sql.Timestamp +import java.time.* +import java.time.temporal.ChronoUnit + +class ParquetTemporalPrecisionTest { + @Test + void epochConversionIncludesNegativeAndExtremeBounds() { + TimeUnit.values().each { unit -> + assertEquals(0L, TemporalUnits.epochUnits(Instant.EPOCH, unit)) + long units = 1_000_000_000L.intdiv(TemporalUnits.nanosPerUnit(unit)) + assertEquals(-units, TemporalUnits.epochUnits(Instant.ofEpochSecond(-1), unit)) + assertEquals(-units.intdiv(2L), TemporalUnits.epochUnits(Instant.parse('1969-12-31T23:59:59.5Z'), unit)) + } + [[TimeUnit.NANOS, '1677-09-21T00:12:43.145224192Z', '2262-04-11T23:47:16.854775807Z', 1L], + [TimeUnit.MICROS, '-290308-12-21T19:59:05.224192Z', '+294247-01-10T04:00:54.775807Z', 1000L]].each { unit, minimum, maximum, tick -> + Instant lower = Instant.parse(minimum), upper = Instant.parse(maximum) + assertEquals(Long.MIN_VALUE, TemporalUnits.epochUnits(lower, unit)) + assertEquals(Long.MAX_VALUE, TemporalUnits.epochUnits(upper, unit)) + assertThrows(ArithmeticException) { TemporalUnits.epochUnits(lower.minusNanos(tick), unit) } + assertThrows(ArithmeticException) { TemporalUnits.epochUnits(upper.plusNanos(tick), unit) } + } + } + + @Test + void microsTruncateAllTimestampClassesAndExactPredicatesStayExact() { + Instant value = Instant.EPOCH.plusNanos(123456789) + def data = Matrix.builder().columns(ts: [value], local: [LocalDateTime.ofInstant(value, ZoneOffset.UTC)], stamp: [Timestamp.from(value)]).build() + byte[] bytes = MatrixParquetWriter.builder(data).zoneId('UTC').writeBytes() + def read = MatrixParquetReader.builder().zoneId('UTC').read(bytes) + assertEquals(value.truncatedTo(ChronoUnit.MICROS), read[0, 'ts']) + assertEquals(LocalDateTime.ofInstant(value.truncatedTo(ChronoUnit.MICROS), ZoneOffset.UTC), read[0, 'local']) + assertEquals(Timestamp.from(value.truncatedTo(ChronoUnit.MICROS)), read[0, 'stamp']) + [[Predicates.eq('ts', value), 0], [Predicates.eq('ts', value.truncatedTo(ChronoUnit.MICROS)), 1], + [Predicates.ge('ts', value), 0], [Predicates.le('ts', value), 1], [Predicates.lt('ts', value), 1], + [Predicates.ne('ts', value), 1]].each { predicate, count -> + assertEquals(count, MatrixParquetReader.builder().filter(predicate).read(bytes).rowCount()) + } + Instant negative = Instant.parse('1969-12-31T23:59:59.123456789Z') + assertEquals(negative.truncatedTo(ChronoUnit.MICROS), MatrixParquetReader.read(MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [negative]).build()))[0, 'ts']) + assertEquals(Instant.EPOCH, MatrixParquetReader.read(MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [Instant.EPOCH.plusNanos(1)]).build()))[0, 'ts']) + assertEquals(1, MatrixParquetReader.read(MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [Instant.now()], local: [LocalDateTime.now()]).build())).rowCount()) + } + + @Test + void nanosRoundTripBoundsAndPushdownAndSpi() { + [Instant.EPOCH.plusNanos(123456789), Instant.parse('1677-09-21T00:12:43.145224192Z'), Instant.parse('2262-04-11T23:47:16.854775807Z')].each { value -> + def matrix = Matrix.builder().columns(ts: [value], stamp: [Timestamp.from(value)], local: [LocalDateTime.ofInstant(value, ZoneOffset.UTC)]).build() + def options = ParquetWriteOptions.fromMap([timestampUnit: ' nanos ', zoneId: 'UTC']) + byte[] bytes = MatrixParquetWriter.writeBytes(matrix, options) + def schema = ParquetFixFixtures.footer(bytes).schema + assertEquals(TimeUnit.NANOS, schema.getType('ts').logicalTypeAnnotation.unit) + assertNotNull(Predicates.eq('ts', value).pushdown(schema)) + def result = MatrixParquetReader.builder().zoneId('UTC').filter(Predicates.eq('ts', value)).read(bytes) + assertEquals(value, result[0, 'ts']) + assertEquals(Timestamp.from(value), result[0, 'stamp']) + assertEquals(LocalDateTime.ofInstant(value, ZoneOffset.UTC), result[0, 'local']) + assertEquals(value, MatrixParquetReader.read(MatrixParquetWriter.builder(Matrix.builder().columns(ts: [value]).build()).timestampUnit('NANOS').writeBytes())[0, 'ts']) + } + assertTrue(ParquetWriteOptions.descriptors().any { it.name == 'timestampUnit' && it.defaultValue == 'MICROS' }) + [null, 'millis', 'SECONDS'].each { unit -> assertThrows(IllegalArgumentException) { new ParquetWriteOptions().timestampUnit(unit) } } + } + + @Test + void overflowReportsColumnAndRowForEveryTemporalClass() { + ['MICROS', 'NANOS'].each { unit -> + def failure = assertThrows(IllegalArgumentException) { + MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [Instant.MAX]).build(), new ParquetWriteOptions().timestampUnit(unit)) + } + assertTrue(failure.message.contains('ts') && failure.message.contains('row 0')) + } + [Instant.parse('1677-09-21T00:12:43.145224191Z'), Instant.parse('2262-04-11T23:47:16.854775808Z')].each { value -> + [value, Timestamp.from(value), LocalDateTime.ofInstant(value, ZoneOffset.UTC)].each { temporal -> + def matrix = Matrix.builder().columns(ts: [temporal]).build() + def failure = assertThrows(IllegalArgumentException) { MatrixParquetWriter.builder(matrix).zoneId('UTC').timestampUnit('NANOS').writeBytes() } + assertTrue(failure.message.contains('ts') && failure.message.contains('row 0')) + assertEquals(1, MatrixParquetReader.read(MatrixParquetWriter.builder(matrix).zoneId('UTC').writeBytes()).rowCount()) + } + } + Instant minimum = Instant.parse('-290308-12-21T19:59:05.224192Z') + assertEquals(minimum, MatrixParquetReader.read(MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [minimum]).build()))[0, 'ts']) + assertThrows(IllegalArgumentException) { MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [minimum.minusNanos(1000)]).build()) } + } +} diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTypePolicyTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTypePolicyTest.groovy index b7f9824f5..d2329fc2a 100644 --- a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTypePolicyTest.groovy +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTypePolicyTest.groovy @@ -75,7 +75,7 @@ class ParquetTypePolicyTest { [ZonedDateTime.now(), OffsetDateTime.now(), new Object()].each { value -> assertThrows(IllegalArgumentException) { MatrixParquetWriter.writeBytes(Matrix.builder().columns(value: [value]).build()) } } - [Instant.MIN, Instant.MAX, Instant.EPOCH.plusNanos(1)].each { instant -> + [Instant.MIN, Instant.MAX].each { instant -> assertThrows(IllegalArgumentException) { MatrixParquetWriter.writeBytes(Matrix.builder().columns(value: [instant]).build()) } } } @@ -150,12 +150,12 @@ class ParquetTypePolicyTest { @Test void explicitUnsupportedTypesAndUntrustedMetadata() { - ['optional int96 value;', 'optional fixed_len_byte_array(2) value (FLOAT16);'].eachWithIndex { declaration, index -> + ['optional fixed_len_byte_array(2) value (FLOAT16);'].eachWithIndex { declaration, index -> def schema = MessageTypeParser.parseMessageType("message external { $declaration }") File file = new File(directory, "unsupported${index}.parquet") ExampleParquetWriter.builder(new LocalOutputFile(file.toPath())).withConf(new PlainParquetConfiguration()).withType(schema).build().withCloseable { writer -> def group = new SimpleGroupFactory(schema).newGroup() - group.add('value', Binary.fromConstantByteArray(new byte[index == 0 ? 12 : 2])) + group.add('value', Binary.fromConstantByteArray(new byte[2])) writer.write(group) } assertThrows(IllegalArgumentException) { MatrixParquetReader.read(file) } @@ -241,7 +241,7 @@ class ParquetTypePolicyTest { def result = MatrixParquetReader.builder().columns(['price']).filter(filter).read(file) assertEquals(1, result.rowCount()) } - assertThrows(ArithmeticException) { MatrixParquetReader.builder().filter(Predicates.eq('price', 1.251G)).read(file) } - assertThrows(IllegalArgumentException) { MatrixParquetReader.builder().filter(Predicates.eq('instant', Instant.EPOCH.plusNanos(1))).read(file) } + assertEquals(0, MatrixParquetReader.builder().filter(Predicates.eq('price', 1.251G)).read(file).rowCount()) + assertEquals(0, MatrixParquetReader.builder().filter(Predicates.eq('instant', Instant.EPOCH.plusNanos(1))).read(file).rowCount()) } } diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetUnsignedTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetUnsignedTest.groovy new file mode 100644 index 000000000..e44001d19 --- /dev/null +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetUnsignedTest.groovy @@ -0,0 +1,43 @@ +package se.alipsa.matrix.parquet + +import static org.junit.jupiter.api.Assertions.* + +import org.apache.parquet.schema.MessageTypeParser +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir + +class ParquetUnsignedTest { + @TempDir File directory + + @Test + void unsignedValuesMetadataPredicatesAndBatches() { + def schema = MessageTypeParser.parseMessageType('''message unsigned { + optional int32 u32 (INTEGER(32,false)); optional int64 u64 (INTEGER(64,false)); + optional int32 u8 (INTEGER(8,false)); + optional group l (LIST) { repeated group list { optional int32 element (INTEGER(32,false)); } } + optional int32 s16 (INTEGER(16,true)); }''') + [false, true].each { wrong -> + def metadata = wrong ? ['matrix.columnTypes': 'java.lang.Integer,java.lang.Long,java.lang.Byte,java.util.List,java.lang.Short', 'matrix.parquet.version': '2'] : [:] + File file = ParquetFixFixtures.write(directory, "unsigned-$wrong", schema, metadata) { writer, factory -> + def row = factory.newGroup().append('u32', (int) 3_000_000_000L).append('u64', -1L).append('u8', 255).append('s16', -123) + row.addGroup('l').addGroup('list').append('element', (int) 3_000_000_000L) + writer.write(row) + } + def result = MatrixParquetReader.read(file) + assertEquals([Long, BigInteger, Integer, List, wrong ? Short : Integer], result.types()) + assertEquals(3_000_000_000L, result[0, 'u32']) + assertEquals(18446744073709551615G, result[0, 'u64']) + assertEquals(255, result[0, 'u8']) + assertEquals([3_000_000_000L], result[0, 'l']) + assertEquals((short) -123, result[0, 's16']) + [Predicates.gt('u32', 2_000_000_000L), Predicates.eq('u64', 18446744073709551615G)].each { predicate -> + assertNull(predicate.pushdown(schema)) + assertEquals(1, MatrixParquetReader.builder().filter(predicate).limit(1).read(file).rowCount()) + MatrixParquetReader.builder().filter(predicate).maxBatchRows(1).batches(file).withCloseable { batches -> + assertEquals(1, batches.next().rowCount()) + assertFalse(batches.hasNext()) + } + } + } + } +} diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetWriterOptionsValidationTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetWriterOptionsValidationTest.groovy new file mode 100644 index 000000000..ca6489a54 --- /dev/null +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetWriterOptionsValidationTest.groovy @@ -0,0 +1,49 @@ +package se.alipsa.matrix.parquet + +import static org.junit.jupiter.api.Assertions.* + +import org.apache.parquet.conf.PlainParquetConfiguration +import org.apache.parquet.hadoop.ParquetFileReader +import org.apache.parquet.hadoop.metadata.CompressionCodecName +import org.junit.jupiter.api.Test + +import se.alipsa.matrix.core.Matrix + +class ParquetWriterOptionsValidationTest { + @Test + void bloomMapsNormalizeStrictlyAndActuallyWriteBloomFilters() { + def options = ParquetWriteOptions.fromMap([bloomFilterEnabled: [id: 'true'], bloomFilterNDV: [id: '1000']]) + assertEquals([id: true], options.bloomFilterEnabled) + assertEquals([id: 1000L], options.bloomFilterNDV) + byte[] bytes = MatrixParquetWriter.writeBytes(Matrix.builder().columns(id: [1, 2]).build(), options) + ParquetFileReader.open(new InMemoryInputFile(bytes), org.apache.parquet.ParquetReadOptions.builder(new PlainParquetConfiguration()).build()).withCloseable { reader -> + assertNotNull(reader.readBloomFilter(reader.rowGroups[0].columns[0])) + } + [1000.0G, 1000, (short) 1000, 1000G].each { value -> + assertEquals(1000L, ParquetWriteOptions.fromMap([bloomFilterNDV: [id: value]]).bloomFilterNDV.id) + } + [[bloomFilterEnabled: [id: 'maybe']], [bloomFilterEnabled: [id: null]], [bloomFilterNDV: [id: 'x']], + [bloomFilterNDV: [id: 0]], [bloomFilterNDV: [id: 1.5]], [bloomFilterNDV: [id: '1.5']], + [bloomFilterNDV: [id: null]], [bloomFilterNDV: [id: '9223372036854775808']], [bloomFilterNDV: [id: true]]].each { map -> + assertTrue(assertThrows(IllegalArgumentException) { ParquetWriteOptions.fromMap(map) }.message.contains('id')) + } + [[id: 'bad'], [id: null]].each { map -> + assertTrue(assertThrows(IllegalArgumentException) { new ParquetWriteOptions(bloomFilterEnabled: map).validate() }.message.contains('id')) + } + assertTrue(assertThrows(IllegalArgumentException) { new ParquetWriteOptions(bloomFilterNDV: [id: 'bad']).validate() }.message.contains('id')) + assertEquals([id: 1000], new ParquetWriteOptions().bloomFilterNDV([id: 1000]).bloomFilterNDV) + } + + @Test + @SuppressWarnings('LocaleSetDefault') // Regression for locale independence; restored in finally. + void codecNamesIgnoreDefaultLocaleAndWhitespace() { + Locale previous = Locale.default + try { + Locale.default = Locale.forLanguageTag('tr-TR') + assertEquals(CompressionCodecName.GZIP, new ParquetWriteOptions().compressionCodec('gzip').compressionCodec) + assertEquals(CompressionCodecName.ZSTD, new ParquetWriteOptions().compressionCodec(' zstd ').compressionCodec) + assertTrue(assertThrows(IllegalArgumentException) { new ParquetWriteOptions().compressionCodec((String) null) }.message.contains('compressionCodec')) + assertTrue(assertThrows(IllegalArgumentException) { new ParquetWriteOptions().compressionCodec('nope') }.message.contains('GZIP')) + } finally { Locale.default = previous } + } +} From a44471842687bb64e4cc7627a7b47636de930e95 Mon Sep 17 00:00:00 2001 From: Per Nyfelt Date: Sun, 18 Oct 2026 17:31:31 +0200 Subject: [PATCH 2/4] Fix Parquet 0.8.0 review findings: nested path escaping, null options, temporal range checks --- matrix-parquet/req/v0.8.0-fixes.md | 11 +++++ .../matrix/parquet/ColumnMaterializer.groovy | 6 ++- .../matrix/parquet/ColumnTypePolicy.groovy | 6 +++ .../matrix/parquet/MatrixParquetWriter.groovy | 18 +++++---- .../alipsa/matrix/parquet/NestedPaths.groovy | 8 +++- .../matrix/parquet/ParquetBatches.groovy | 4 +- .../matrix/parquet/ParquetPredicate.groovy | 1 - .../matrix/parquet/ParquetValueWriter.groovy | 3 +- .../matrix/parquet/ParquetWriteOptions.groovy | 2 +- .../parquet/ParquetNestedTypesTest.groovy | 26 ++++++++++++ .../ParquetTemporalPrecisionTest.groovy | 40 +++++++++++-------- .../ParquetWriterOptionsValidationTest.groovy | 8 ++++ 12 files changed, 102 insertions(+), 31 deletions(-) diff --git a/matrix-parquet/req/v0.8.0-fixes.md b/matrix-parquet/req/v0.8.0-fixes.md index 802206f54..b63013d7a 100644 --- a/matrix-parquet/req/v0.8.0-fixes.md +++ b/matrix-parquet/req/v0.8.0-fixes.md @@ -78,6 +78,12 @@ column-index filtering. Choose option 2. Older readers ignore unknown keys, so the format stays backward compatible. +**Path escaping (review fix, 2026-10-18):** each path segment has `.` and `\` escaped +(`\..` → `\.`, `\` → `\\`), so dotted column or struct-key names cannot collide with the +path separator (e.g. top-level `s.a` vs struct `s` child `a`). The writer also drops a +nestedTypes entry if the same path is ever recorded with different types, and the reader +only applies nestedTypes to leaves that are not top-level columns. + ### D. Sub-microsecond timestamps (section 8.1) — **decided by the maintainer, 2026-10-18** **Decision:** @@ -88,6 +94,11 @@ Choose option 2. Older readers ignore unknown keys, so the format stays backward - The current `Instant` rejection is a bug and is removed. - Full precision is available with the opt-in `timestampUnit('NANOS')`. +**Range-check extension (review fix, 2026-10-18):** `ColumnTypePolicy.resolve` now runs the +same `TemporalUnits.epochUnits` overflow check for `LocalDateTime` (using the writer's pinned +zone) and `Timestamp` as it already did for `Instant`, so NANOS overflow fails at schema +validation instead of partway through the row loop. + **Context:** - Before this change, `LocalDateTime` and `Timestamp` were silently truncated while `Instant` was rejected (`ColumnTypePolicy.groovy:43`, `ParquetValueWriter.groovy:141-143`). diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnMaterializer.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnMaterializer.groovy index 099ed5220..9163d9fd9 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnMaterializer.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnMaterializer.groovy @@ -32,7 +32,7 @@ final class ColumnMaterializer extends RecordMaterializer { schema.fields.eachWithIndex { Type field, int index -> Closure logicalSink = residual != null && residual.columns().contains(field.name) ? { Object value -> current[field.name] = value } : null children[index] = converter(field, types[index], binaryAsString, - { Object value -> columns[index].add(value) }, logicalSink, field.name, nestedTypes) + { Object value -> columns[index].add(value) }, logicalSink, NestedPaths.root(field.name), nestedTypes) } root = new GroupConverter() { @Override @@ -72,7 +72,9 @@ final class ColumnMaterializer extends RecordMaterializer { private static Converter converter(Type type, Class expected, boolean binaryAsString, Closure sink, Closure logicalSink = null, String path = '', Map nestedTypes = [:], boolean parentIsMap = false, boolean wrapper = false, boolean mapWrapper = false) { if (type.primitive) { - return new ValueConverter(type.asPrimitiveType(), nestedTypes[path] ?: expected, binaryAsString, sink, logicalSink) + // Top-level converters carry their declared type; nestedTypes only applies to nested leaves. + Class leafExpected = expected != null ? expected : (nestedTypes[path] ?: expected) + return new ValueConverter(type.asPrimitiveType(), leafExpected, binaryAsString, sink, logicalSink) } GroupType group = type.asGroupType() Map values = [:] diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnTypePolicy.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnTypePolicy.groovy index 03bc98e13..cb4b0c7e5 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnTypePolicy.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnTypePolicy.groovy @@ -41,6 +41,12 @@ final class ColumnTypePolicy { if (value instanceof Instant) { try { TemporalUnits.epochUnits(value, unit) } catch (ArithmeticException failure) { throw new IllegalArgumentException("Column '$name', row $row: Instant overflows epoch $unit", failure) } + } else if (value instanceof LocalDateTime) { + try { TemporalUnits.epochUnits(((LocalDateTime) value).atZone(ParquetValueWriter.getZoneId()).toInstant(), unit) } + catch (ArithmeticException failure) { throw new IllegalArgumentException("Column '$name', row $row: LocalDateTime overflows epoch $unit", failure) } + } else if (value instanceof Timestamp) { + try { TemporalUnits.epochUnits(((Timestamp) value).toInstant(), unit) } + catch (ArithmeticException failure) { throw new IllegalArgumentException("Column '$name', row $row: Timestamp overflows epoch $unit", failure) } } Class current = policyType(value.getClass()) if (current == null || (policy != null && policy != current)) { diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetWriter.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetWriter.groovy index 9978f9ea3..48c175fba 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetWriter.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetWriter.groovy @@ -463,13 +463,12 @@ class MatrixParquetWriter { } options.validate() File file = determineTargetFile(matrix, fileOrDir) - WriteSchema schema = createSchema(matrix, options) if (options.zoneId != null) { return ParquetValueWriter.withZoneId(options.zoneId) { - writeInternal(matrix, file, schema, options.compressionCodec, options.roundingMode, options) + writeInternal(matrix, file, createSchema(matrix, options), options.compressionCodec, options.roundingMode, options) } } - return writeInternal(matrix, file, schema, options.compressionCodec, options.roundingMode, options) + return writeInternal(matrix, file, createSchema(matrix, options), options.compressionCodec, options.roundingMode, options) } /** @@ -529,13 +528,12 @@ class MatrixParquetWriter { throw new IllegalArgumentException(ERR_OPTIONS_NULL) } options.validate() - WriteSchema schema = createSchema(matrix, options) if (options.zoneId != null) { return ParquetValueWriter.withZoneId(options.zoneId) { - writeBytesInternal(matrix, schema, options.compressionCodec, options.roundingMode, options) + writeBytesInternal(matrix, createSchema(matrix, options), options.compressionCodec, options.roundingMode, options) } } - return writeBytesInternal(matrix, schema, options.compressionCodec, options.roundingMode, options) + return writeBytesInternal(matrix, createSchema(matrix, options), options.compressionCodec, options.roundingMode, options) } private static File determineTargetFile(Matrix matrix, File fileOrDir) { @@ -954,7 +952,13 @@ class MatrixParquetWriter { private static void recordNestedType(DecimalSchemaPolicy decimals, ColumnTypePolicy policy) { if (policy.metadata in [Instant, Date, Timestamp, java.sql.Date, Byte, Short, BigInteger]) { - decimals.nestedTypes[decimals.path] = policy.metadata.name + String existing = decimals.nestedTypes[decimals.path] + if (existing != null && existing != policy.metadata.name) { + // Same path recorded with different types; drop it rather than silently keeping one. + decimals.nestedTypes.remove(decimals.path) + } else { + decimals.nestedTypes[decimals.path] = policy.metadata.name + } } } diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/NestedPaths.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/NestedPaths.groovy index 023145ad3..2ac5eef30 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/NestedPaths.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/NestedPaths.groovy @@ -11,7 +11,13 @@ import org.apache.parquet.schema.Type final class NestedPaths { private NestedPaths() {} - static String child(String path, String name) { path.empty ? name : "$path.$name".toString() } + /** Escapes '.' and '\\' so dotted or backslash-containing names cannot collide with path separators. */ + static String escape(String name) { name.replace('\\', '\\\\').replace('.', '\\.') } + + /** Logical path of a top-level field. */ + static String root(String name) { escape(name) } + + static String child(String path, String name) { path.empty ? escape(name) : "${path}.${escape(name)}".toString() } static boolean isMap(GroupType group, boolean parentIsMap = false) { group.logicalTypeAnnotation instanceof LogicalTypeAnnotation.MapLogicalTypeAnnotation || diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetBatches.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetBatches.groovy index 7afbbab77..25a14420e 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetBatches.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetBatches.groovy @@ -84,9 +84,9 @@ final class ParquetBatches implements MatrixBatchIterator { } Map nested = MatrixParquetReader.parseNestedTypes(metadata[MatrixParquetReader.METADATA_NESTED_TYPES]) requested.fields.each { Type field -> - NestedPaths.leaves(field, field.name).each { String path, Type leaf -> + NestedPaths.leaves(field, NestedPaths.root(field.name)).each { String path, Type leaf -> Class expected = ColumnTypePolicy.METADATA_TYPES[nested[path]] - if (path != field.name && compatibleMetadata(expected, leaf)) { nestedTypes[path] = expected } + if (path != NestedPaths.root(field.name) && compatibleMetadata(expected, leaf)) { nestedTypes[path] = expected } } } selected = options.rowGroups == null ? (0..(options.rowGroups) diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetPredicate.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetPredicate.groovy index f1d6e1cbd..29e081de0 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetPredicate.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetPredicate.groovy @@ -166,7 +166,6 @@ final class ParquetPredicate { private Object physicalValueExact(PrimitiveType field) { if (value == null) { return null } def logical = field.logicalTypeAnnotation - if (logical instanceof LogicalTypeAnnotation.IntLogicalTypeAnnotation && !logical.isSigned()) { return NOT_PUSHABLE } if (logical instanceof LogicalTypeAnnotation.DecimalLogicalTypeAnnotation) { BigDecimal decimal = new BigDecimal(value.toString()).setScale(logical.scale, java.math.RoundingMode.UNNECESSARY) if (decimal.precision() > logical.precision) { throw new IllegalArgumentException("Decimal predicate overflows '${field.name}'") } diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetValueWriter.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetValueWriter.groovy index 8523f6036..4d8b3f2bb 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetValueWriter.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetValueWriter.groovy @@ -73,8 +73,9 @@ final class ParquetValueWriter { /** * Gets the current timezone for timestamp conversion. * Returns the thread-local value if set, otherwise the system default. + * Package scope so schema-time validation ({@link ColumnTypePolicy}) sees the same zone as value writing. */ - private static ZoneId getZoneId() { + static ZoneId getZoneId() { ZoneId zoneId = ZONE_ID_HOLDER.get() return zoneId != null ? zoneId : ZoneId.systemDefault() } diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetWriteOptions.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetWriteOptions.groovy index 5d54a92a9..8fc83bf2c 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetWriteOptions.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ParquetWriteOptions.groovy @@ -263,8 +263,8 @@ class ParquetWriteOptions { Set unknown = new HashSet<>(normalized.keySet()) unknown.removeAll(known) if (!unknown.empty) { throw new IllegalArgumentException("Unknown Parquet options: $unknown") } - if (normalized.containsKey('timestampunit')) { result.timestampUnit((String) normalized.timestampunit) } normalized = normalized.findAll { String key, Object value -> value != null } + if (normalized.containsKey('timestampunit')) { result.timestampUnit((String) normalized.timestampunit) } if (normalized.containsKey('inferprecisionandscale')) { Object inferPrecisionAndScale = normalized.inferprecisionandscale if (inferPrecisionAndScale != null) { diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedTypesTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedTypesTest.groovy index 052056d33..def547e4f 100644 --- a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedTypesTest.groovy +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedTypesTest.groovy @@ -76,4 +76,30 @@ class ParquetNestedTypesTest { assertEquals(Byte, MatrixParquetReader.read(dotted).type('a.b')) } + @Test + void dottedTopLevelColumnDoesNotCollideWithStructChildPath() { + def data = Matrix.builder().columns('s.a': [(short) 1000], s: [[a: (byte) 1, c: 'x']]).types(Short, Map).build() + byte[] bytes = MatrixParquetWriter.writeBytes(data) + assertEquals(['s.a': 'java.lang.Byte'], + MatrixParquetReader.parseNestedTypes(ParquetFixFixtures.footer(bytes).metadata['matrix.nestedTypes'])) + def result = MatrixParquetReader.read(bytes) + assertEquals(Short, result.type('s.a')) + assertEquals((short) 1000, result[0, 's.a']) + assertEquals((byte) 1, result[0, 's'].a) + assertEquals(Byte, result[0, 's'].a.getClass()) + } + + @Test + void dottedStructKeyDoesNotCollideWithNestedStructPath() { + def data = Matrix.builder().columns(s: [['a.b': (short) 1000, a: [b: (byte) 1, c: 'x']]]).types(Map).build() + byte[] bytes = MatrixParquetWriter.writeBytes(data) + assertEquals(['s.a\\.b': 'java.lang.Short', 's.a.b': 'java.lang.Byte'], + MatrixParquetReader.parseNestedTypes(ParquetFixFixtures.footer(bytes).metadata['matrix.nestedTypes'])) + def struct = MatrixParquetReader.read(bytes)[0, 's'] + assertEquals((short) 1000, struct['a.b']) + assertEquals(Short, struct['a.b'].getClass()) + assertEquals((byte) 1, struct.a.b) + assertEquals(Byte, struct.a.b.getClass()) + } + } diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy index 60d0fa026..11364d45c 100644 --- a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.* import org.apache.parquet.schema.LogicalTypeAnnotation.TimeUnit import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir import se.alipsa.matrix.core.Matrix @@ -70,23 +71,30 @@ class ParquetTemporalPrecisionTest { } @Test - void overflowReportsColumnAndRowForEveryTemporalClass() { - ['MICROS', 'NANOS'].each { unit -> - def failure = assertThrows(IllegalArgumentException) { - MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [Instant.MAX]).build(), new ParquetWriteOptions().timestampUnit(unit)) - } - assertTrue(failure.message.contains('ts') && failure.message.contains('row 0')) + void farFutureLocalDateTimeAndTimestampFailAtSchemaValidationBeforeWriting(@TempDir File directory) { + File file = new File(directory, 'overflow.parquet') + LocalDateTime sentinel = LocalDateTime.of(9999, 12, 31, 0, 0) + def localFailure = assertThrows(IllegalArgumentException) { + MatrixParquetWriter.write(Matrix.builder().columns(ts: [sentinel]).build(), file, new ParquetWriteOptions().timestampUnit('NANOS')) } - [Instant.parse('1677-09-21T00:12:43.145224191Z'), Instant.parse('2262-04-11T23:47:16.854775808Z')].each { value -> - [value, Timestamp.from(value), LocalDateTime.ofInstant(value, ZoneOffset.UTC)].each { temporal -> - def matrix = Matrix.builder().columns(ts: [temporal]).build() - def failure = assertThrows(IllegalArgumentException) { MatrixParquetWriter.builder(matrix).zoneId('UTC').timestampUnit('NANOS').writeBytes() } - assertTrue(failure.message.contains('ts') && failure.message.contains('row 0')) - assertEquals(1, MatrixParquetReader.read(MatrixParquetWriter.builder(matrix).zoneId('UTC').writeBytes()).rowCount()) - } + assertTrue(localFailure.message.contains('LocalDateTime') && localFailure.message.contains('row 0'), localFailure.message) + assertFalse(file.exists(), 'schema validation must fail before any file is created') + def timestampFailure = assertThrows(IllegalArgumentException) { + MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [Timestamp.from(Instant.parse('2263-01-01T00:00:00Z'))]).build(), + new ParquetWriteOptions().timestampUnit('NANOS')) } - Instant minimum = Instant.parse('-290308-12-21T19:59:05.224192Z') - assertEquals(minimum, MatrixParquetReader.read(MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [minimum]).build()))[0, 'ts']) - assertThrows(IllegalArgumentException) { MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [minimum.minusNanos(1000)]).build()) } + assertTrue(timestampFailure.message.contains('Timestamp') && timestampFailure.message.contains('row 0'), timestampFailure.message) + } + + @Test + void nanosBoundaryHonorsThePinnedWriterZone() { + LocalDateTime boundary = LocalDateTime.ofInstant(Instant.parse('2262-04-11T23:47:16.854775807Z'), ZoneOffset.UTC) + def matrix = Matrix.builder().columns(ts: [boundary]).build() + byte[] bytes = MatrixParquetWriter.writeBytes(matrix, ParquetWriteOptions.fromMap([timestampUnit: 'NANOS', zoneId: 'UTC'])) + assertEquals(boundary, MatrixParquetReader.builder().zoneId('UTC').read(bytes)[0, 'ts']) + def failure = assertThrows(IllegalArgumentException) { + MatrixParquetWriter.writeBytes(matrix, ParquetWriteOptions.fromMap([timestampUnit: 'NANOS', zoneId: 'Pacific/Honolulu'])) + } + assertTrue(failure.message.contains('LocalDateTime'), failure.message) } } diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetWriterOptionsValidationTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetWriterOptionsValidationTest.groovy index ca6489a54..84c351679 100644 --- a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetWriterOptionsValidationTest.groovy +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetWriterOptionsValidationTest.groovy @@ -34,6 +34,14 @@ class ParquetWriterOptionsValidationTest { assertEquals([id: 1000], new ParquetWriteOptions().bloomFilterNDV([id: 1000]).bloomFilterNDV) } + @Test + void nullOptionValuesMeanDefaultsEverywhere() { + assertEquals('MICROS', ParquetWriteOptions.fromMap([timestampUnit: null]).timestampUnit) + assertEquals('NANOS', ParquetWriteOptions.fromMap([timestampUnit: ' nanos ']).timestampUnit) + assertEquals(CompressionCodecName.SNAPPY, ParquetWriteOptions.fromMap([compressionCodec: null]).compressionCodec) + assertEquals(java.math.RoundingMode.UNNECESSARY, ParquetWriteOptions.fromMap([roundingMode: null]).roundingMode) + } + @Test @SuppressWarnings('LocaleSetDefault') // Regression for locale independence; restored in finally. void codecNamesIgnoreDefaultLocaleAndWhitespace() { From 48f7eb717f88b484fbf6b5d5b52f91f0701a72a1 Mon Sep 17 00:00:00 2001 From: Per Nyfelt Date: Sun, 18 Oct 2026 17:44:06 +0200 Subject: [PATCH 3/4] Fix Parquet 0.8.0 follow-up: raw decimalMeta paths, restored boundary tests, MICROS range-check skip --- matrix-parquet/readme.md | 4 +++- matrix-parquet/req/v0.8.0-fixes.md | 14 +++++++------ .../matrix/parquet/ColumnMaterializer.groovy | 3 +-- .../matrix/parquet/ColumnTypePolicy.groovy | 10 ++++++++- .../matrix/parquet/DecimalSchemaPolicy.groovy | 10 +++++++-- .../matrix/parquet/MatrixParquetWriter.groovy | 8 +------ .../parquet/ParquetNestedTypesTest.groovy | 12 +++++++++++ .../ParquetTemporalPrecisionTest.groovy | 21 +++++++++++++++++++ 8 files changed, 63 insertions(+), 19 deletions(-) diff --git a/matrix-parquet/readme.md b/matrix-parquet/readme.md index bb33120cb..57a71780d 100644 --- a/matrix-parquet/readme.md +++ b/matrix-parquet/readme.md @@ -234,7 +234,9 @@ legacy outer MAP_KEY_VALUE groups and key-only maps (whose values become null). names. It preserves nested Instant, Date, Timestamp, sql.Date, Byte, Short and BigInteger values in lists, map keys/values and struct fields. LocalTime/Time are distinguished by TIME annotations. Older readers ignore the new key; files without it retain schema inference. Unknown classes and -incompatible entries are ignored, with no external class loading. +incompatible entries are ignored, with no external class loading. Segments containing `.` or `\` +are escaped (`\.`, `\\`) so dotted column or struct-key names cannot collide with the path +separator; `decimalMeta` keys remain the raw, unescaped dotted paths. | Parquet / Matrix type | Read or write behavior | |---|---| diff --git a/matrix-parquet/req/v0.8.0-fixes.md b/matrix-parquet/req/v0.8.0-fixes.md index b63013d7a..a300e4a1b 100644 --- a/matrix-parquet/req/v0.8.0-fixes.md +++ b/matrix-parquet/req/v0.8.0-fixes.md @@ -78,11 +78,11 @@ column-index filtering. Choose option 2. Older readers ignore unknown keys, so the format stays backward compatible. -**Path escaping (review fix, 2026-10-18):** each path segment has `.` and `\` escaped -(`\..` → `\.`, `\` → `\\`), so dotted column or struct-key names cannot collide with the -path separator (e.g. top-level `s.a` vs struct `s` child `a`). The writer also drops a -nestedTypes entry if the same path is ever recorded with different types, and the reader -only applies nestedTypes to leaves that are not top-level columns. +**Path escaping (review fix, 2026-10-18):** each `matrix.nestedTypes` path segment has `.` and +`\` escaped (`\.`, `\\`), so dotted column or struct-key names cannot collide with the path +separator (e.g. top-level `s.a` vs struct `s` child `a`). The escaped path is internal to that +footer key only; `decimalMeta` keys and error messages keep the raw, unescaped dotted paths. +The reader only applies nestedTypes to leaves that are not top-level columns. ### D. Sub-microsecond timestamps (section 8.1) — **decided by the maintainer, 2026-10-18** @@ -97,7 +97,9 @@ only applies nestedTypes to leaves that are not top-level columns. **Range-check extension (review fix, 2026-10-18):** `ColumnTypePolicy.resolve` now runs the same `TemporalUnits.epochUnits` overflow check for `LocalDateTime` (using the writer's pinned zone) and `Timestamp` as it already did for `Instant`, so NANOS overflow fails at schema -validation instead of partway through the row loop. +validation instead of partway through the row loop. For `LocalDateTime` under MICROS the exact +per-cell conversion is skipped well inside the ±292,000-year range (|year| < 290,000), since it +cannot overflow there; NANOS always checks exactly. **Context:** - Before this change, `LocalDateTime` and `Timestamp` were silently truncated while `Instant` was diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnMaterializer.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnMaterializer.groovy index 9163d9fd9..4617fdc1c 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnMaterializer.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnMaterializer.groovy @@ -73,8 +73,7 @@ final class ColumnMaterializer extends RecordMaterializer { boolean parentIsMap = false, boolean wrapper = false, boolean mapWrapper = false) { if (type.primitive) { // Top-level converters carry their declared type; nestedTypes only applies to nested leaves. - Class leafExpected = expected != null ? expected : (nestedTypes[path] ?: expected) - return new ValueConverter(type.asPrimitiveType(), leafExpected, binaryAsString, sink, logicalSink) + return new ValueConverter(type.asPrimitiveType(), expected ?: nestedTypes[path], binaryAsString, sink, logicalSink) } GroupType group = type.asGroupType() Map values = [:] diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnTypePolicy.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnTypePolicy.groovy index cb4b0c7e5..5187dec6d 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnTypePolicy.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/ColumnTypePolicy.groovy @@ -41,7 +41,7 @@ final class ColumnTypePolicy { if (value instanceof Instant) { try { TemporalUnits.epochUnits(value, unit) } catch (ArithmeticException failure) { throw new IllegalArgumentException("Column '$name', row $row: Instant overflows epoch $unit", failure) } - } else if (value instanceof LocalDateTime) { + } else if (value instanceof LocalDateTime && requiresEpochCheck((LocalDateTime) value, unit)) { try { TemporalUnits.epochUnits(((LocalDateTime) value).atZone(ParquetValueWriter.getZoneId()).toInstant(), unit) } catch (ArithmeticException failure) { throw new IllegalArgumentException("Column '$name', row $row: LocalDateTime overflows epoch $unit", failure) } } else if (value instanceof Timestamp) { @@ -87,6 +87,14 @@ final class ColumnTypePolicy { SUPPORTED.contains(type) ? type : null } + /** + * MICROS covers about ±292,000 years; well inside {@code ±290,000} the exact epoch conversion + * cannot overflow, so the per-cell zone lookup is skipped. + */ + private static boolean requiresEpochCheck(LocalDateTime value, org.apache.parquet.schema.LogicalTypeAnnotation.TimeUnit unit) { + unit != org.apache.parquet.schema.LogicalTypeAnnotation.TimeUnit.MICROS || Math.abs(value.year) >= 290_000 + } + static Class box(Class type) { if (!type.primitive) { return type diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/DecimalSchemaPolicy.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/DecimalSchemaPolicy.groovy index 86e95258a..ef838fd98 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/DecimalSchemaPolicy.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/DecimalSchemaPolicy.groovy @@ -9,7 +9,10 @@ final class DecimalSchemaPolicy { final boolean infer final Integer precision final Integer scale + /** Raw user-facing dotted path used for decimalMeta settings and error messages. */ final String path + /** Escaped path used as matrix.nestedTypes key; unambiguous even for dotted or backslash-containing names. */ + final String nestedPath final org.apache.parquet.schema.LogicalTypeAnnotation.TimeUnit timestampUnit final Map nestedTypes final Set used @@ -18,7 +21,7 @@ final class DecimalSchemaPolicy { DecimalSchemaPolicy(Map settings, boolean infer = true, Integer precision = null, Integer scale = null, String path = '', Set used = new HashSet<>(), org.apache.parquet.schema.LogicalTypeAnnotation.TimeUnit timestampUnit = org.apache.parquet.schema.LogicalTypeAnnotation.TimeUnit.MICROS, - Map nestedTypes = [:]) { + Map nestedTypes = [:], String nestedPath = '') { this.timestampUnit = timestampUnit this.nestedTypes = nestedTypes this.settings = settings @@ -26,11 +29,14 @@ final class DecimalSchemaPolicy { this.precision = precision this.scale = scale this.path = path + this.nestedPath = nestedPath this.used = used } DecimalSchemaPolicy child(String name) { - new DecimalSchemaPolicy(settings, infer, precision, scale, NestedPaths.child(path, name), used, timestampUnit, nestedTypes) + String childPath = path.empty ? name : "$path.$name".toString() + new DecimalSchemaPolicy(settings, infer, precision, scale, childPath, used, timestampUnit, nestedTypes, + NestedPaths.child(nestedPath, name)) } int[] resolve(List values, int[] supplied) { diff --git a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetWriter.groovy b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetWriter.groovy index 48c175fba..e29d794e7 100644 --- a/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetWriter.groovy +++ b/matrix-parquet/src/main/groovy/se/alipsa/matrix/parquet/MatrixParquetWriter.groovy @@ -952,13 +952,7 @@ class MatrixParquetWriter { private static void recordNestedType(DecimalSchemaPolicy decimals, ColumnTypePolicy policy) { if (policy.metadata in [Instant, Date, Timestamp, java.sql.Date, Byte, Short, BigInteger]) { - String existing = decimals.nestedTypes[decimals.path] - if (existing != null && existing != policy.metadata.name) { - // Same path recorded with different types; drop it rather than silently keeping one. - decimals.nestedTypes.remove(decimals.path) - } else { - decimals.nestedTypes[decimals.path] = policy.metadata.name - } + decimals.nestedTypes[decimals.nestedPath] = policy.metadata.name } } diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedTypesTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedTypesTest.groovy index def547e4f..b0699d5cd 100644 --- a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedTypesTest.groovy +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetNestedTypesTest.groovy @@ -102,4 +102,16 @@ class ParquetNestedTypesTest { assertEquals(Byte, struct.a.b.getClass()) } + @Test + void decimalMetaKeepsRawDottedPathsForColumnsAndStructKeys() { + def top = Matrix.builder().columns('a.b': [1.25G]).build() + byte[] bytes = MatrixParquetWriter.writeBytes(top, new ParquetWriteOptions().decimalMeta(['a.b': [10, 2] as int[]])) + assertEquals(1.25G, MatrixParquetReader.read(bytes)[0, 'a.b']) + def nested = Matrix.builder().columns(s: [['p.q': 1.25G, n: 'x']]).types(Map).build() + bytes = MatrixParquetWriter.writeBytes(nested, new ParquetWriteOptions().decimalMeta(['s.p.q': [10, 2] as int[]])) + def struct = MatrixParquetReader.read(bytes)[0, 's'] + assertEquals(1.25G, struct['p.q']) + assertEquals('x', struct.n) + } + } diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy index 11364d45c..855ca0309 100644 --- a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy @@ -70,6 +70,27 @@ class ParquetTemporalPrecisionTest { [null, 'millis', 'SECONDS'].each { unit -> assertThrows(IllegalArgumentException) { new ParquetWriteOptions().timestampUnit(unit) } } } + @Test + void overflowReportsColumnAndRowForEveryTemporalClass() { + ['MICROS', 'NANOS'].each { unit -> + def failure = assertThrows(IllegalArgumentException) { + MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [Instant.MAX]).build(), new ParquetWriteOptions().timestampUnit(unit)) + } + assertTrue(failure.message.contains('ts') && failure.message.contains('row 0')) + } + [Instant.parse('1677-09-21T00:12:43.145224191Z'), Instant.parse('2262-04-11T23:47:16.854775808Z')].each { value -> + [value, Timestamp.from(value), LocalDateTime.ofInstant(value, ZoneOffset.UTC)].each { temporal -> + def matrix = Matrix.builder().columns(ts: [temporal]).build() + def failure = assertThrows(IllegalArgumentException) { MatrixParquetWriter.builder(matrix).zoneId('UTC').timestampUnit('NANOS').writeBytes() } + assertTrue(failure.message.contains('ts') && failure.message.contains('row 0')) + assertEquals(1, MatrixParquetReader.read(MatrixParquetWriter.builder(matrix).zoneId('UTC').writeBytes()).rowCount()) + } + } + Instant minimum = Instant.parse('-290308-12-21T19:59:05.224192Z') + assertEquals(minimum, MatrixParquetReader.read(MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [minimum]).build()))[0, 'ts']) + assertThrows(IllegalArgumentException) { MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [minimum.minusNanos(1000)]).build()) } + } + @Test void farFutureLocalDateTimeAndTimestampFailAtSchemaValidationBeforeWriting(@TempDir File directory) { File file = new File(directory, 'overflow.parquet') From bdf8f4364e6b56ff73cc95e8cf3c083d78711392 Mon Sep 17 00:00:00 2001 From: Per Nyfelt Date: Sun, 18 Oct 2026 18:08:34 +0200 Subject: [PATCH 4/4] Add MICROS LocalDateTime range-check regression test --- .../parquet/ParquetTemporalPrecisionTest.groovy | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy index 855ca0309..3e8a969c1 100644 --- a/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy +++ b/matrix-parquet/src/test/groovy/se/alipsa/matrix/parquet/ParquetTemporalPrecisionTest.groovy @@ -91,6 +91,18 @@ class ParquetTemporalPrecisionTest { assertThrows(IllegalArgumentException) { MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [minimum.minusNanos(1000)]).build()) } } + @Test + void microsLocalDateTimeOverflowStillFailsAtSchemaValidation() { + [LocalDateTime.MAX, LocalDateTime.of(-291000, 1, 1, 0, 0)].each { value -> + def failure = assertThrows(IllegalArgumentException) { + MatrixParquetWriter.writeBytes(Matrix.builder().columns(ts: [value]).build(), new ParquetWriteOptions().zoneId('UTC')) + } + assertTrue(failure.message.contains('LocalDateTime') && failure.message.contains('row 0'), failure.message) + } + assertEquals(1, MatrixParquetReader.read(MatrixParquetWriter.writeBytes( + Matrix.builder().columns(ts: [LocalDateTime.of(289999, 1, 1, 0, 0)]).build(), new ParquetWriteOptions().zoneId('UTC'))).rowCount()) + } + @Test void farFutureLocalDateTimeAndTimestampFailAtSchemaValidationBeforeWriting(@TempDir File directory) { File file = new File(directory, 'overflow.parquet')