From bdb7096b89d8a71f53feae48e669edb8c6061a3d Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Sun, 25 Oct 2026 16:31:38 +0100 Subject: [PATCH] build: JPMS named modules, tests white-box on the module path (#538) core, fsst, reader, writer, csv, jdbc and inspector are named modules (module name = package root, so inspector is io.github.dfa1.vortex.inspect); integration has a code-less main descriptor so failsafe runs on the module path too. Tests stay in the packages they test and are patched into the module, as surefire/failsafe and IntelliJ both do for a main-only module-info. - core exports fbs/proto only to the modules that need them; a public signature carrying a generated type suppresses the exports lint on that declaration instead of widening the export. - The writer never requires the reader: its tests reach the reader on the test classpath, with the two --add-exports of core's fbs/proto as test-JVM flags (test.modules.args). The one writer test using java.net.http (HTTP zone-map pruning) moved to integration, so the writer test compile needs no flags and IDEs import main and test with one compiler configuration. - parquet and calcite depend on jars with no descriptor, so they pin a stable Automatic-Module-Name instead; cli, jdbc-driver and the generators are never required and pin nothing. - zstd is a named module: native access is enabled by name (--enable-native-access=io.github.dfa1.zstd). - The core test-jar ships only core.testing (re-declaring core's packages is a split package). - JDK 26+: the incubating-module lint is off for javac; javadoc, which cannot silence it, tolerates that one warning in every module. Javadoc no longer auto-links sibling modules (the links pointed at a non-existent ${project.url}//apidocs/ and failed classpath modules linking named ones). - ADR 0024 moves from Proposed to Accepted with these rules. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01SwhK4W5cgj7y7s4ZPCif5x --- AGENTS.md | 12 ++ CHANGELOG.md | 1 + README.md | 2 +- adr/0024-jpms-adoption.md | 171 +++++++++--------- adr/ADR.md | 2 +- calcite/pom.xml | 17 ++ core/pom.xml | 34 +--- core/src/main/java/module-info.java | 18 ++ csv/src/main/java/module-info.java | 10 + fsst/src/main/java/module-info.java | 6 + inspector/src/main/java/module-info.java | 8 + integration/pom.xml | 14 +- integration/src/main/java/module-info.java | 18 ++ .../integration/DocsJavaCompilerTest.java | 19 +- .../ZoneMapHttpPruningIntegrationTest.java | 11 +- jdbc/src/main/java/module-info.java | 10 + parquet/pom.xml | 17 ++ pom.xml | 37 +++- reader/pom.xml | 15 +- .../github/dfa1/vortex/reader/ArrayStats.java | 3 + reader/src/main/java/module-info.java | 20 ++ writer/pom.xml | 21 ++- .../vortex/writer/encode/CascadeStep.java | 2 + .../vortex/writer/encode/EncodeResult.java | 3 + .../dfa1/vortex/writer/encode/MinMax.java | 2 + .../encode/PrimitiveEncodingEncoder.java | 2 + .../vortex/writer/encode/VariantData.java | 4 + writer/src/main/java/module-info.java | 12 ++ .../writer/encode/DecodeTestHelper.java | 7 +- 29 files changed, 364 insertions(+), 134 deletions(-) create mode 100644 core/src/main/java/module-info.java create mode 100644 csv/src/main/java/module-info.java create mode 100644 fsst/src/main/java/module-info.java create mode 100644 inspector/src/main/java/module-info.java create mode 100644 integration/src/main/java/module-info.java rename writer/src/test/java/io/github/dfa1/vortex/writer/WriterZoneMapHttpPruningTest.java => integration/src/test/java/io/github/dfa1/vortex/integration/ZoneMapHttpPruningIntegrationTest.java (94%) create mode 100644 jdbc/src/main/java/module-info.java create mode 100644 reader/src/main/java/module-info.java create mode 100644 writer/src/main/java/module-info.java diff --git a/AGENTS.md b/AGENTS.md index 617126f79..387fcc030 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -310,6 +310,18 @@ When stuck on encode/decode behavior, consult **in this order**: loudly (`VortexException`, Rust default; no allowUnknown for layouts). Scope: the layout SPI covers full-column subtree decode; zone-map pruning, filtered scans, and chunk planning recognize the built-in layouts only. +- **JPMS (ADR 0024).** `core`, `fsst`, `reader`, `writer`, `csv`, `jdbc` and `inspector` are named modules (module name = package + root: `io.github.dfa1.vortex.inspect`, not `...inspector`); `integration` has a main descriptor with no code so failsafe runs on + the module path too. Surefire/failsafe use the module path, so the module graph is checked by the tests. The libraries `parquet` + and `calcite` have no `module-info` (they depend on jars with no module descriptor) and instead pin a stable + `Automatic-Module-Name` in the manifest; `cli`, `jdbc-driver`, `fbs-gen` and `proto-gen` are never `requires`d and pin nothing; `performance`, `demo` and `fuzz` are unpublished and stay on the classpath. Rules: `core` exports `fbs`/`proto` only to the modules that + need them (the generated wire codecs stay out of the public API); `writer` never `requires` the reader, so its round-trip + tests reach the reader on the test classpath and the `--add-exports` of core's `fbs`/`proto` it needs live in the + writer POM's `test.modules.args`, never in `module-info.java` (a test needing more than that, such as + `java.net.http`, belongs in `integration`); a module that exposes a generated type from a public signature + suppresses the `exports` lint on that one declaration with a comment. JVM flags that only some modules' tests need are + the empty `test.native.args`/`test.unsafe.args`/`test.modules.args` properties in the root POM, set per module (a named + module is enabled by name: `--enable-native-access=io.github.dfa1.zstd`, not `ALL-UNNAMED`). - **Options types are plain final classes with a private constructor** (`WriteOptions`, `ScanOptions`, and the CSV/Parquet/JDBC import and export options): a `defaults()` or named factory returns the starting point and `withXxx` methods return copies. Never a record and never a public diff --git a/CHANGELOG.md b/CHANGELOG.md index e5f44378f..f0d06fe36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Folding dict-encoded float columns and scan windows reads their buffers directly, about 3% faster on a two-column taxi scan. ([#600](https://github.com/dfa1/vortex-java/pull/600)) ### Added +- `core`, `fsst`, `reader`, `writer`, `csv`, `jdbc` and `inspector` ship as named JPMS modules (`io.github.dfa1.vortex.*`), and `parquet` and `calcite` declare a stable `Automatic-Module-Name`. ([#538](https://github.com/dfa1/vortex-java/pull/538)) - A CSV import schema can declare any integer type from `I8` to `U64`, range-checked per value, and `F32`; an unsupported type fails in `withSchema` instead of at the first chunk. ([#593](https://github.com/dfa1/vortex-java/pull/593)) - `ScanIterator.chunkStats` is public: each chunk's own array stats, which carry a string column's exact min/max where the zone map only bounds it. ([#608](https://github.com/dfa1/vortex-java/pull/608)) - `vortex explore` shows what is in a file: null %, min/max and a zone sparkline per column at once, then each column's total, mean, distinct count and value counts or histogram, served to the browser or written to one HTML file with `--html`. ([#608](https://github.com/dfa1/vortex-java/pull/608)) diff --git a/README.md b/README.md index 041cf4844..2375213ec 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ If you are an AI agent asked to convert data to Vortex with this library, these would otherwise guess. Working *on* this repository instead? Read [AGENTS.md](AGENTS.md). - **Java 25+.** The library uses FFM, not JNI: run with `--enable-native-access=ALL-UNNAMED` - (the CLI jar's manifest already sets it). + (the CLI jar's manifest already sets it), or `--enable-native-access=io.github.dfa1.zstd` on the module path. - **No code needed for a one-off conversion.** Download `vortex-cli-0.18.1-all.jar` from [Maven Central](https://central.sonatype.com/artifact/io.github.dfa1.vortex/vortex-cli) and run `java -jar vortex-cli-0.18.1-all.jar import data.csv data.vortex`. Add `--delimiter ';'` for diff --git a/adr/0024-jpms-adoption.md b/adr/0024-jpms-adoption.md index cf70d0fda..373db08d6 100644 --- a/adr/0024-jpms-adoption.md +++ b/adr/0024-jpms-adoption.md @@ -1,117 +1,116 @@ -# ADR 0024: JPMS adoption for core/reader/writer +# ADR 0024: JPMS adoption -- **Status:** Proposed +- **Status:** Accepted (2026-10-14); first recorded as Proposed on 2026-09-06 - **Date:** 2026-09-06 - **Deciders:** project maintainer - **Related:** [ADR 0017 — In-house FlatBuffers codegen](0017-in-house-flatbuffers-codegen.md), - [PR #176 — feat(jpms): add module-info for core/reader/writer \[WIP\]](https://github.com/dfa1/vortex-java/pull/176) + [PR #176 — feat(jpms): add module-info for core/reader/writer \[WIP\]](https://github.com/dfa1/vortex-java/pull/176) (superseded by PR #538) ## Context `flatbuffers-java` used to make a real JPMS module graph impossible: it ships as an automatic module with an unstable, jar-filename-derived name, and a named module cannot reliably `requires` one. ADR 0017 removed that dependency in favor of the -in-house `fbs-gen`/`proto-gen` toolchain, which removed the blocker — but nobody has -finished adding `module-info.java` since. TODO.md has carried a single unqualified -line ("use JPMS, watch out for 'dfa1' in package name") since before that ADR, with -no record of *why* it might be worth doing or what it would cost. - -PR #176 is the in-progress attempt, parked as draft ("do not merge yet"). It already -gives a concrete, evidence-based account of both sides: - -**What works today:** `core`, `reader`, and `writer` (main sourcesets) are all -modular with clean, green builds (359 / 857 tests respectively for core/reader). -`writer` correctly does not `require reader`, preserving the architecture rule from -ADR 0001. Downstream modules (`calcite`, `cli`, `csv`, `jdbc`, `parquet`, -`inspector`) are left non-modular by design, consumed as automatic modules. - -**What's blocking it:** `writer`'s **test** sourceset fails under `-Werror`. -Writer's tests decode-verify round-trips using test-scope `reader` types, but they -patch into the `writer` module, which — correctly — does not `require reader` in -its main module graph. The workaround attempted (`testCompile ---add-reads=ALL-MODULE-PATH` plus Surefire `useModulePath=false`) still leaves a -residual exports/reads lint warning. There is, as of this ADR, no clean way to let -the test module read `reader` without polluting the main module graph — this is -the actual reason #176 is still a draft, not the historically-cited flatbuffers -blocker. - -A second, smaller gap: `reader`/`writer` both `requires static zstd` against -`io.github.dfa1.zstd:zstd` (a separate project of this maintainer's), which itself -resolves as an automatic module with no `module-info.java` of its own. Unlike the -former `flatbuffers-java` situation, this one is fully within this maintainer's -control to close — but it hasn't been done, so it remains a live gap today. - -Separately from the mechanics: **JPMS adoption for consumed libraries is low in -practice.** Most consumers put jars on the classpath regardless of whether a -`module-info.java` exists; strict encapsulation and `jlink` custom runtime images -matter far more for applications than for a library shipped to Maven Central. The -actual encapsulation goal this project cares about — internal FlatBuffers/protobuf -runtime classes (`FbsTable`, `FbsMemorySegment`, the proto codec) staying hidden — -is already achieved today via ordinary `public`/package-private visibility, with no -module system required. +in-house `fbs-gen`/`proto-gen` toolchain, which removed the blocker. + +When this ADR was first written the decision was left open, leaning against: PR #176 got +`core`, `reader` and `writer` building as modules, but `writer`'s **test** sourceset failed +under `-Werror` because its round-trip tests decode with test-scope `reader` types while the +writer module (correctly) does not `require reader`. A second gap was that +`io.github.dfa1.zstd:zstd` (a separate project of this maintainer's) was an automatic module; +it now ships a real `module-info`, which closed it. + +The case for doing it anyway: running the tests on the module path makes them check the module +graph (a missing `requires`, an accidental split package, a leaked internal) instead of the +classpath hiding all of it. The counter-argument stays true: JPMS adoption among consumers of a +Maven Central library is low, and the project's encapsulation goal (hiding the FlatBuffers/protobuf +runtime) was already met with ordinary `public`/package-private visibility. The decision is +therefore about what the project gets for its own build, not about downstream demand. ## Decision -Not yet made — this ADR records the tradeoff for a decision that is still open. -Recorded here for future reference so the same ground doesn't need re-deriving. - -The maintainer's current lean: **do not prioritize finishing PR #176** absent a -concrete downstream request for a strict module graph or a `jlink`-based runtime -image. Keep it parked exactly as drafted (do not merge) rather than force through -the test-sourceset workaround under time pressure, and revisit if a real consumer -need surfaces — mirroring the "capture the design now, defer implementation until -a concrete need" posture already used in [ADR 0016](0016-vortex-arrow-bridge.md). +Adopt JPMS for the published library modules and run surefire and failsafe on the module path +(PR #538, replacing the stale draft #176). The rules: + +1. **Module name = package root.** `core`, `fsst`, `reader`, `writer`, `csv`, `jdbc` are + `io.github.dfa1.vortex.`; the inspector is `io.github.dfa1.vortex.inspect` because its + package is `...inspect`, not `...inspector`. The artifact id may differ from the module name. + The `dfa1` terminal-digit lint is suppressed with `@SuppressWarnings("module")` on each + declaration: the name has to match the packages and the published group. +2. **A module with an automatic-module dependency stays descriptor-less.** A published named module + must not `requires` a jar with no descriptor (its derived name is unstable). The libraries `parquet` + (hardwood) and `calcite` (calcite-core/babel) pin a stable `Automatic-Module-Name` in the jar + manifest instead, so a modular application can `requires` them under the name a future descriptor + will keep. `cli` (an application run from its `-all` jar), `jdbc-driver` (loaded by `DriverManager`) + and the unpublished generators `fbs-gen`/`proto-gen` are never required, so they pin nothing. + Unpublished `performance`, `demo` and `fuzz` + stay on the classpath (`performance` also declares a class in `core`'s `simd` package, a split + package on the module path). +3. **Generated wire codecs are not public API.** `core` exports `fbs` and `proto` only to the modules + that use them (a qualified export); a new consumer module is added to that list. Where a public + signature must expose a generated type (`ArrayStats.fromFbs`, `VariantData`), the `exports` lint is + suppressed on that one declaration with a comment rather than widening the export. +4. **The writer never requires the reader.** This is what blocked #176. Its round-trip tests see the + reader on the test classpath, and the `--add-exports` of core's `fbs`/`proto` it needs live in the writer + POM's `test.modules.args`, never in a `module-info`. The writer test compile takes no extra flags, so + IDEs import main and test with one compiler configuration; the one test that needed `java.net.http` + moved to `integration`. +5. **`integration` has a code-less main descriptor.** Failsafe only uses the module path when the module + has a descriptor, and a test-only descriptor would put every test dependency (including the + invalid-named `zstd-native-*` resource jars) on the module path and fail at boot. A main descriptor + requiring the named modules keeps the non-modular libraries (vortex-jni, hardwood, Arrow's netty + bits, calcite) on the classpath. +6. **Shared test code cannot re-declare production packages.** `core`'s test-jar ships only + `core.testing`; shipping its `*Test` classes was a split package. +7. **JVM flags are per module and named where possible.** The shared test `argLine` carries only the + agent and Vector API flags; `test.native.args`, `test.unsafe.args` and `test.modules.args` are empty + root properties that a module sets when its tests need them. A named module is enabled by name + (`--enable-native-access=io.github.dfa1.zstd`), not `ALL-UNNAMED`. +8. **JDK 26 lints.** `-Xlint:-incubating` is set once in the root compiler config because every module + resolving `core` sees `jdk.incubator.vector` (its `requires static`); `missing-explicit-ctor` is off + in `reader`/`writer`, whose public encoders and decoders are stateless and registry-instantiated. ## Consequences ### Positive -- If finished: a verified, reliable module graph for `core`/`reader`/`writer`, - catching missing/duplicate dependency issues at compile time instead of runtime. -- If finished: unlocks `jlink` custom runtime images for anyone embedding - vortex-java in a minimal-footprint deployment (e.g. the `cli` module). -- The investigation itself (PR #176) already validated that ADR 0017's dependency - removal was sufficient to make the main-sourceset module graph work cleanly. +- A broken or missing `requires`, an accidental split package or a leaked internal fails the unit + tests, not a consumer. +- Native access is granted to the one module that needs it, so a new native caller is a visible warning. +- Module-path consumers and `jlink` users get stable names for every published artifact. ### Negative -- **Test friction is the dominant cost.** Package-private internals that tests - reach into (this project's own convention — see CLAUDE.md's `FbsTable`/proto - runtime pattern) turn into split-package errors or `--patch-module`/`--add-reads` - Surefire configuration once a real module boundary exists, as PR #176's own - blocker demonstrates concretely. -- Mockito-based mocking of non-trivial classes needs per-package `opens` directives - — ongoing bookkeeping unrelated to the domain. -- Per-module `module-info.java` maintenance scales with this project's module - count (16 modules today), for a payoff that mostly benefits application-style - consumers, which vortex-java mostly isn't. +- Tests that reach across modules need explicit flags (rule 4), and a new qualified-export consumer must + be added by hand (rule 3). +- `integration` runs half on the module path and half on the classpath (rule 5), so it does not + exercise a fully modular consumer. +- Public APIs that expose a generated type carry a lint suppression. +- Mockito and package-private test access are the standing friction of a real module boundary; they + have been manageable so far because tests are patched into the module they test. ### Risks to manage -- The `io.github.dfa1.zstd:zstd` automatic-module gap is self-inflicted (this - maintainer's own dependency) and cheap to close independently of the rest of - this decision — giving it a real `module-info.java` removes one blocker - regardless of what happens with #176. -- If revisited later, re-verify the FlatBuffers-derived blocker is still fully - gone (ADR 0017 must not have regressed) before resuming from #176's state. +- The `Automatic-Module-Name` modules (rule 2) can become named modules once hardwood and calcite ship + descriptors; revisit then. +- JDK lint categories change between releases (rule 8); CI runs on 25 and 26 to catch it. ## Alternatives considered -- **Finish #176 now.** Rejected for the moment: the test-sourceset blocker needs - a real solution (not a suppressed warning) to land cleanly, and no consumer has - asked for strict modularity yet. -- **Partial modularization (core only).** Would sidestep writer's test-sourceset - blocker, but reader/writer are exactly the modules whose FFM/zero-copy internals - most benefit from encapsulation guarantees — modularizing only the least - interesting module captures little of the value. -- **Abandon JPMS entirely, close #176, delete the TODO item without replacement.** - Rejected: the low-adoption argument is about *downstream* JPMS adoption, not - about whether the maintainer might still want it for `jlink`/internal reasons - someday; parking costs nothing, closing forecloses the option. +- **Keep the deferral.** Rejected: the blockers were closed, and classpath-only tests hide + module-graph errors. +- **Descriptors for every module, `requires` of automatic modules.** Rejected for published modules + (unstable derived names). +- **A test-side descriptor for `integration`.** Tried: it puts invalid-named native resource jars on the + module path and fails at boot. +- **Widening `core`'s `fbs`/`proto` exports.** Rejected: it would publish the whole generated codec. +- **Partial modularization (core only).** Rejected: `reader` and `writer` are the modules whose + FFM/zero-copy internals most benefit from encapsulation. ## References -- [PR #176 — feat(jpms): add module-info for core/reader/writer \[WIP\]](https://github.com/dfa1/vortex-java/pull/176) +- [PR #538 — JPMS modules, surefire and failsafe on the module path](https://github.com/dfa1/vortex-java/pull/538) +- [PR #176 — feat(jpms): add module-info for core/reader/writer \[WIP\]](https://github.com/dfa1/vortex-java/pull/176) (stale draft) - [ADR 0017 — In-house FlatBuffers codegen + MemorySegment-native runtime](0017-in-house-flatbuffers-codegen.md) -- [ADR 0016 — vortex-arrow bridge](0016-vortex-arrow-bridge.md) — precedent for "capture now, defer until a concrete need" -- [ADR 0001 — Split read and write runtimes](0001-split-read-and-write-runtimes.md) — the writer-never-depends-on-reader rule PR #176 preserves and that its test sourceset struggles against +- [ADR 0001 — Split read and write runtimes](0001-split-read-and-write-runtimes.md) — the + writer-never-depends-on-reader rule diff --git a/adr/ADR.md b/adr/ADR.md index e4077f8a5..6a21a92f1 100644 --- a/adr/ADR.md +++ b/adr/ADR.md @@ -38,6 +38,6 @@ the decision shipped in (blank = not yet shipped). | 0021 | Cardinality-bounded global dict buffering | Accepted | | | 0022 | Extract FSST into a standalone module, ported faithfully from the paper | Accepted | | | 0023 | Adopt the Vortex editions model as a client-side write/read policy | Accepted | | -| 0024 | JPMS adoption for core/reader/writer | Proposed | | +| 0024 | JPMS adoption for core/reader/writer | Accepted | | | 0025 | Centralize zone-map MIN/MAX stats computation in the writer | Accepted | | | 0026 | FSST per-row lazy decode | Accepted | | diff --git a/calcite/pom.xml b/calcite/pom.xml index 122cdd0b1..38ccab478 100644 --- a/calcite/pom.xml +++ b/calcite/pom.xml @@ -64,4 +64,21 @@ test + + + + + org.apache.maven.plugins + maven-jar-plugin + + + + io.github.dfa1.vortex.calcite + + + + + + diff --git a/core/pom.xml b/core/pom.xml index cdba6fe15..53c9905c7 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -51,7 +51,9 @@ + helpers (DTypes, EncodeTestHelper) without duplication. Only the + testing package goes in: the module path rejects a test-jar that re-declares packages of the real core + module (a split package), which core's own *Test classes would. --> org.apache.maven.plugins maven-jar-plugin @@ -61,6 +63,11 @@ test-jar + + + io/github/dfa1/vortex/core/testing/** + + @@ -77,31 +84,6 @@ Then commit the updated files. --> - - - javadoc-sees-incubating-module - - [26,) - - - - - org.apache.maven.plugins - maven-javadoc-plugin - - false - - --add-modules - jdk.incubator.vector - - - - - - regenerate-sources diff --git a/core/src/main/java/module-info.java b/core/src/main/java/module-info.java new file mode 100644 index 000000000..35e697586 --- /dev/null +++ b/core/src/main/java/module-info.java @@ -0,0 +1,18 @@ +/// Shared model, wire codecs and SIMD kernels used by the reader and the writer. +// "dfa1" trips the module-name terminal-digit lint; the name has to match the packages (and the published group). +@SuppressWarnings("module") +module io.github.dfa1.vortex.core { + // Opt-in at run time (--add-modules jdk.incubator.vector); the auto-vectorized kernels are the default. + requires static jdk.incubator.vector; + // @Generated on the fbs/proto codecs; compile-time only. + requires static java.compiler; + + exports io.github.dfa1.vortex.core.model; + exports io.github.dfa1.vortex.core.error; + exports io.github.dfa1.vortex.core.io; + exports io.github.dfa1.vortex.core.compute; + exports io.github.dfa1.vortex.core.simd; + // Generated wire codecs: qualified so they stay out of the public API (and out of the exports lint). + exports io.github.dfa1.vortex.core.fbs to io.github.dfa1.vortex.reader, io.github.dfa1.vortex.writer, io.github.dfa1.vortex.inspect; + exports io.github.dfa1.vortex.core.proto to io.github.dfa1.vortex.reader, io.github.dfa1.vortex.writer, io.github.dfa1.vortex.integration; +} diff --git a/csv/src/main/java/module-info.java b/csv/src/main/java/module-info.java new file mode 100644 index 000000000..ff4ba478c --- /dev/null +++ b/csv/src/main/java/module-info.java @@ -0,0 +1,10 @@ +/// CSV import and export for Vortex files. +// "dfa1" trips the module-name terminal-digit lint; the name has to match the packages (and the published group). +@SuppressWarnings("module") +module io.github.dfa1.vortex.csv { + requires transitive io.github.dfa1.vortex.reader; + requires transitive io.github.dfa1.vortex.writer; + requires de.siegmar.fastcsv; + + exports io.github.dfa1.vortex.csv; +} diff --git a/fsst/src/main/java/module-info.java b/fsst/src/main/java/module-info.java new file mode 100644 index 000000000..594b2f2a2 --- /dev/null +++ b/fsst/src/main/java/module-info.java @@ -0,0 +1,6 @@ +/// Standalone FSST (Fast Static Symbol Table) string compression. +// "dfa1" trips the module-name terminal-digit lint; the name has to match the packages (and the published group). +@SuppressWarnings("module") +module io.github.dfa1.vortex.fsst { + exports io.github.dfa1.vortex.fsst; +} diff --git a/inspector/src/main/java/module-info.java b/inspector/src/main/java/module-info.java new file mode 100644 index 000000000..83d08fcdc --- /dev/null +++ b/inspector/src/main/java/module-info.java @@ -0,0 +1,8 @@ +/// Dumps a Vortex file's footer, layout tree and per-column statistics. +// "dfa1" trips the module-name terminal-digit lint; the name has to match the packages (and the published group). +@SuppressWarnings("module") +module io.github.dfa1.vortex.inspect { + requires transitive io.github.dfa1.vortex.reader; + + exports io.github.dfa1.vortex.inspect; +} diff --git a/integration/pom.xml b/integration/pom.xml index 50226a808..cec800efc 100644 --- a/integration/pom.xml +++ b/integration/pom.xml @@ -42,7 +42,6 @@ io.github.dfa1.vortex vortex-writer - test io.github.dfa1.vortex @@ -52,12 +51,10 @@ io.github.dfa1.vortex vortex-csv - test io.github.dfa1.vortex vortex-jdbc - test io.github.dfa1.vortex @@ -84,12 +81,10 @@ io.github.dfa1.vortex vortex-reader - test io.github.dfa1.vortex vortex-inspector - test dev.vortex @@ -126,6 +121,11 @@ assertj-core test + + org.mockito + mockito-junit-jupiter + test + org.slf4j slf4j-simple @@ -226,7 +226,7 @@ dependency we don't own. Drop only that category here; every other lint stays strict via the parent's -Werror. --> - -Xlint:all,-classfile + -Xlint:all,-classfile,-incubating @@ -263,7 +263,7 @@ @{argLine} -javaagent:${net.bytebuddy:byte-buddy-agent:jar} --add-opens java.base/java.nio=ALL-UNNAMED - --enable-native-access=ALL-UNNAMED + --enable-native-access=io.github.dfa1.zstd,ALL-UNNAMED --sun-misc-unsafe-memory-access=allow ${test.vector.args} diff --git a/integration/src/main/java/module-info.java b/integration/src/main/java/module-info.java new file mode 100644 index 000000000..45482f77b --- /dev/null +++ b/integration/src/main/java/module-info.java @@ -0,0 +1,18 @@ +/// Integration tests: Rust interop, vortex-jni comparisons and the cross-format importers/exporters. +/// +/// The module has no production code. This descriptor exists so the tests are patched into a named module and failsafe +/// runs on the module path like the library modules it exercises. Dependencies without a module descriptor +/// (vortex-jni, hardwood, Arrow's netty bits, calcite) deliberately stay on the classpath. +// "dfa1" trips the module-name terminal-digit lint; the name has to match the packages (and the published group). +@SuppressWarnings("module") +module io.github.dfa1.vortex.integration { + requires io.github.dfa1.vortex.core; + requires io.github.dfa1.vortex.reader; + requires io.github.dfa1.vortex.writer; + requires io.github.dfa1.vortex.csv; + requires io.github.dfa1.vortex.jdbc; + requires io.github.dfa1.vortex.inspect; + requires de.siegmar.fastcsv; + requires java.sql; + requires java.compiler; +} diff --git a/integration/src/test/java/io/github/dfa1/vortex/integration/DocsJavaCompilerTest.java b/integration/src/test/java/io/github/dfa1/vortex/integration/DocsJavaCompilerTest.java index a7b8977d3..15ce16e65 100644 --- a/integration/src/test/java/io/github/dfa1/vortex/integration/DocsJavaCompilerTest.java +++ b/integration/src/test/java/io/github/dfa1/vortex/integration/DocsJavaCompilerTest.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; @@ -198,6 +199,14 @@ private static String simpleClassNameOf(JavaFileObject source) { // ── compilation ─────────────────────────────────────────────────────────── + private static String vortexModules() { + return ModuleLayer.boot().modules().stream() + .map(Module::getName) + .filter(name -> name.startsWith("io.github.dfa1.vortex.") && !name.endsWith(".integration")) + .sorted() + .collect(Collectors.joining(",")); + } + private static List> compileErrors(List sources) throws IOException { JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); @@ -208,10 +217,16 @@ private static List> compileErrors(List options = List.of( + List options = new ArrayList<>(List.of( "--release", "25", "-classpath", System.getProperty("java.class.path"), - "-proc:none"); + "-proc:none")); + // The library modules are on the module path (failsafe/surefire run modular), not the classpath, so the + // snippets need that path too, with the vortex modules as roots they can import from. + String modulePath = System.getProperty("jdk.module.path"); + if (modulePath != null) { + options.addAll(List.of("--module-path", modulePath, "--add-modules", vortexModules())); + } compiler.getTask(null, fileManager, diagnostics, options, null, sources).call(); } finally { deleteRecursively(outDir); diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/WriterZoneMapHttpPruningTest.java b/integration/src/test/java/io/github/dfa1/vortex/integration/ZoneMapHttpPruningIntegrationTest.java similarity index 94% rename from writer/src/test/java/io/github/dfa1/vortex/writer/WriterZoneMapHttpPruningTest.java rename to integration/src/test/java/io/github/dfa1/vortex/integration/ZoneMapHttpPruningIntegrationTest.java index 56addb2df..2c36d70bf 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/WriterZoneMapHttpPruningTest.java +++ b/integration/src/test/java/io/github/dfa1/vortex/integration/ZoneMapHttpPruningIntegrationTest.java @@ -1,4 +1,4 @@ -package io.github.dfa1.vortex.writer; +package io.github.dfa1.vortex.integration; import io.github.dfa1.vortex.core.model.ColumnName; import io.github.dfa1.vortex.core.model.DType; @@ -6,6 +6,8 @@ import io.github.dfa1.vortex.reader.ScanFilter; import io.github.dfa1.vortex.reader.ScanOptions; import io.github.dfa1.vortex.reader.VortexHttpReader; +import io.github.dfa1.vortex.writer.VortexWriter; +import io.github.dfa1.vortex.writer.WriteOptions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; @@ -37,10 +39,11 @@ /// segment just to check whether it can be skipped — otherwise "checking" a chunk costs as much /// over the network as simply not pruning it at all. /// -/// Lives here (not in `reader`) because building the fixture needs [VortexWriter]; `vortex-writer` -/// already depends on `vortex-reader` in test scope for round-trip assertions like this one. +/// Lives here (not in `reader`) because building the fixture needs [VortexWriter], and not in +/// `writer` because the writer module does not read `java.net.http`, which this module reaches +/// through the reader. @ExtendWith(MockitoExtension.class) -class WriterZoneMapHttpPruningTest { +class ZoneMapHttpPruningIntegrationTest { @Mock private HttpClient client; diff --git a/jdbc/src/main/java/module-info.java b/jdbc/src/main/java/module-info.java new file mode 100644 index 000000000..9f0ea366e --- /dev/null +++ b/jdbc/src/main/java/module-info.java @@ -0,0 +1,10 @@ +/// JDBC import and export for Vortex files. +// "dfa1" trips the module-name terminal-digit lint; the name has to match the packages (and the published group). +@SuppressWarnings("module") +module io.github.dfa1.vortex.jdbc { + requires transitive io.github.dfa1.vortex.reader; + requires transitive io.github.dfa1.vortex.writer; + requires transitive java.sql; + + exports io.github.dfa1.vortex.jdbc; +} diff --git a/parquet/pom.xml b/parquet/pom.xml index 8d06eb78b..c8b2c59a9 100644 --- a/parquet/pom.xml +++ b/parquet/pom.xml @@ -60,4 +60,21 @@ test + + + + + org.apache.maven.plugins + maven-jar-plugin + + + + io.github.dfa1.vortex.parquet + + + + + + diff --git a/pom.xml b/pom.xml index b2bcc499b..c732c5629 100644 --- a/pom.xml +++ b/pom.xml @@ -140,6 +140,7 @@ tests trip the matching JDK warning (FFM or System.loadLibrary callers; Unsafe users such as protobuf-java). --> + 0.12.0 3.2.8 @@ -390,6 +391,9 @@ true -Xlint:all + + -Xlint:-incubating @@ -405,7 +409,7 @@ {@code net.bytebuddy:byte-buddy-agent:jar} is resolved to an absolute path by {@code maven-dependency-plugin:properties} (declared in build/plugins); loading it as a javaagent suppresses the Mockito self-attach warning. --> - @{argLine} -javaagent:${net.bytebuddy:byte-buddy-agent:jar} ${test.vector.args} ${test.native.args} ${test.unsafe.args} + @{argLine} -javaagent:${net.bytebuddy:byte-buddy-agent:jar} ${test.vector.args} ${test.native.args} ${test.unsafe.args} ${test.modules.args} @@ -454,6 +458,9 @@ ${maven-javadoc-plugin.version} io.github.dfa1.vortex.core.fbs:io.github.dfa1.vortex.core.proto + + false + + javadoc-sees-incubating-module + + [26,) + + + + + + org.apache.maven.plugins + maven-javadoc-plugin + + false + + --add-modules + jdk.incubator.vector + + + + + + + - --enable-native-access=ALL-UNNAMED + --enable-native-access=io.github.dfa1.zstd @@ -62,8 +62,19 @@ + + + org.apache.maven.plugins + maven-compiler-plugin + + + -Xlint:-missing-explicit-ctor + + + + (TestRegistry, TestDecodeContexts). --> org.apache.maven.plugins maven-jar-plugin diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/ArrayStats.java b/reader/src/main/java/io/github/dfa1/vortex/reader/ArrayStats.java index 6e17089e8..061d0cf05 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/ArrayStats.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/ArrayStats.java @@ -56,6 +56,9 @@ public static ArrayStats empty() { /// /// @param fbs the FlatBuffers stats table, or `null` /// @return parsed stats, or an empty instance if no usable data is present + // Low-level entry point for the parse path and the inspector; it intentionally exposes the generated + // FlatBuffers type, which core only qualified-exports to the modules that need it. + @SuppressWarnings("exports") public static ArrayStats fromFbs(io.github.dfa1.vortex.core.fbs.FbsArrayStats fbs) { if (fbs == null) { return EMPTY; diff --git a/reader/src/main/java/module-info.java b/reader/src/main/java/module-info.java new file mode 100644 index 000000000..77e1e40eb --- /dev/null +++ b/reader/src/main/java/module-info.java @@ -0,0 +1,20 @@ +/// Reads Vortex files: memory-maps the file, decodes the layout tree and encodings into arrays. +// "dfa1" trips the module-name terminal-digit lint; the name has to match the packages (and the published group). +@SuppressWarnings("module") +module io.github.dfa1.vortex.reader { + requires transitive io.github.dfa1.vortex.core; + // LazyFsstVarBinArray exposes the fsst Decompressor. + requires transitive io.github.dfa1.vortex.fsst; + // Optional: only the zstd encoding needs it, and it is resolved when present. + requires static io.github.dfa1.zstd; + // VortexHttpReader exposes HttpClient. + requires transitive java.net.http; + + exports io.github.dfa1.vortex.reader; + exports io.github.dfa1.vortex.reader.array; + // RowFilter exposes Predicate. + exports io.github.dfa1.vortex.reader.compute; + exports io.github.dfa1.vortex.reader.decode; + exports io.github.dfa1.vortex.reader.extension; + exports io.github.dfa1.vortex.reader.layout; +} diff --git a/writer/pom.xml b/writer/pom.xml index 59cf02abf..3878a016e 100644 --- a/writer/pom.xml +++ b/writer/pom.xml @@ -16,7 +16,10 @@ - --enable-native-access=ALL-UNNAMED + --enable-native-access=io.github.dfa1.zstd + + --add-exports io.github.dfa1.vortex.core/io.github.dfa1.vortex.core.fbs=ALL-UNNAMED --add-exports io.github.dfa1.vortex.core/io.github.dfa1.vortex.core.proto=ALL-UNNAMED @@ -70,6 +73,22 @@ + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + -Xlint:-missing-explicit-ctor + + + + + +