Skip to content

feat(java): let the storage API reach the local filesystem - #960

Open
keksmd wants to merge 2 commits into
apache:mainfrom
keksmd:java-storage-local
Open

feat(java): let the storage API reach the local filesystem#960
keksmd wants to merge 2 commits into
apache:mainfrom
keksmd:java-storage-local

Conversation

@keksmd

@keksmd keksmd commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Reason for this PR

Closes the remaining half of #953. The storage API merged in #958 defines
Storage, InputFile, OutputFile, SeekableInput, and PositionOutput,
but no implementation ships with it, so nothing in the Java tree can open a
GraphAr file yet. Every later layer of #947 (physical IO, Parquet backend,
reader, writer) needs at least one working adapter to be testable.

What changes are included in this PR?

A local filesystem adapter in a new graphar-storage-local module:

  • LocalStorage resolves a file: URI (and a bare path) to a java.nio.Path
    and hands back input and output files.
  • LocalSeekableInput reads at an absolute position and reports the end of the
    stream instead of a short buffer.
  • LocalOutputFile / LocalPositionOutput create parent directories on demand,
    refuse an existing target unless the caller asked to replace it, and report
    the number of bytes written so far.

The module depends only on graphar-storage-api. It carries no GraphAr layout,
format, projection, or query concern, per the boundary agreed in #953.

Are these changes tested?

Yes. LocalStorageTest covers the round trip, positional reads, end-of-stream
behaviour, directory creation, and the refusal to overwrite an existing target.

mvn --no-transfer-progress -pl storage-local -am clean verify
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

Are there any user-facing changes?

A new module and package org.apache.graphar.storage.local. Nothing existing
changes.

Checklist

  • I have performed a self-review of my own code.
  • I have formatted my own code using make cpplint before submitting when changed files are in the cpp directory.
  • I have performed pre-commit run before commit the changed files.
  • I have added tests to prove my changes are effective.

The storage API merged in apache#958 has no implementation, so nothing can
open a GraphAr file yet. This adds the local filesystem adapter that
the same issue asked for, keeping the module free of any GraphAr
layout, format, or query concern: it resolves a URI to a path, opens a
seekable input over it, and writes through a position-reporting output.

Directories are created on demand for an output file, an existing
target is refused unless the caller asked to replace it, and a read
past the end of a file reports the end of the stream rather than a
partial buffer.

Closes the adapter half of apache#953.

Not-tested: only the file scheme is exercised; other schemes are the
concern of later adapters.
Review feedback on apache#960: a write from a buffer without a backing array
allocated a fresh staging array on every call. The output is single
threaded already, because it tracks its own position, so one instance
buffer can serve every such write. It is allocated on first use, so an
output that only ever sees heap buffers never pays for it.

Adds the missing test for a direct buffer larger than one transfer
chunk, which the previous test did not reach: with the copy loop
reduced to a single pass the new test fails with
"expected:<20000> but was:<8192>".
@SemyonSinchenko

Copy link
Copy Markdown
Member

@SYaoJun @yangxk1 Hi! Let's merge this one? Java module is a work-in-progress so I see no problems at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants