Skip to content

feat(java): complete GraphAr metadata API parity - #956

Merged
SYaoJun merged 3 commits into
apache:mainfrom
keksmd:codex/java-info-api-parity-upstream
Aug 18, 2026
Merged

feat(java): complete GraphAr metadata API parity#956
SYaoJun merged 3 commits into
apache:mainfrom
keksmd:codex/java-info-api-parity-upstream

Conversation

@keksmd

@keksmd keksmd commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Reason for this PR

Completes the dependency-light Java metadata API compatibility work tracked by #955 and relates to the metadata-round-trip umbrella #944. The PR is deliberately limited to graphar-info; it adds no physical IO, storage, Arrow, or query-engine dependency.

What changes are included in this PR?

  • Preserves GraphAr graph labels, extra_info, and typed list<T> values through YAML loading and saving.
  • Adds indexed metadata/property-group lookup and immutable property-group removal.
  • Gives Property and PropertyGroup value equality and hash codes, so removal works for an equivalent deserialized property group rather than only the same object reference.
  • Makes invalid indexed lookups consistently throw IllegalArgumentException, matching existing Java lookup APIs.
  • Adds YAML round-trip, list-type, value-semantic, removal, and index-boundary tests.

Are these changes tested?

Yes, against current upstream/main with GAR_TEST_DATA=testing:

  • mvn --no-transfer-progress -P!nexus-download -pl info -am spotless:check
  • mvn --no-transfer-progress -P!nexus-download -pl info -am clean verify -Dspotless.check.skip=true — 133 tests, 0 failures/errors/skips.
  • pre-commit run --files for all 23 changed Java files, and git diff --check.

Are there any user-facing changes?

Yes. The Java metadata API gains immutable lookup/removal and full GraphAr metadata round-trip support; no on-disk topology or storage behavior 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.

@SemyonSinchenko
SemyonSinchenko self-requested a review August 18, 2026 07:48

@SemyonSinchenko SemyonSinchenko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For me it looks like without equals for PropertyGroup it won't work correctly.

Comment thread maven-projects/info/src/main/java/org/apache/graphar/info/PropertyGroup.java Outdated
Comment thread maven-projects/info/src/main/java/org/apache/graphar/info/GraphInfo.java Outdated
@codecov-commenter

codecov-commenter commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.26950% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.59%. Comparing base (0a51cd2) to head (91018d0).

Files with missing lines Patch % Lines
...in/java/org/apache/graphar/info/type/DataType.java 74.19% 2 Missing and 6 partials ⚠️
...in/java/org/apache/graphar/info/PropertyGroup.java 68.42% 1 Missing and 5 partials ⚠️
...c/main/java/org/apache/graphar/info/GraphInfo.java 80.00% 3 Missing and 2 partials ⚠️
...rc/main/java/org/apache/graphar/info/Property.java 66.66% 1 Missing and 3 partials ⚠️
...pache/graphar/info/loader/BaseGraphInfoLoader.java 90.00% 1 Missing ⚠️
...in/java/org/apache/graphar/info/type/FileType.java 75.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #956      +/-   ##
============================================
+ Coverage     77.40%   77.59%   +0.18%     
- Complexity      621      682      +61     
============================================
  Files            84       85       +1     
  Lines          8972     9091     +119     
  Branches       1074     1098      +24     
============================================
+ Hits           6945     7054     +109     
+ Misses         1785     1780       -5     
- Partials        242      257      +15     
Flag Coverage Δ
java-info 85.48% <82.26%> (+0.59%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

keksmd added 3 commits August 18, 2026 15:23
Expose the indexed lookups and immutable property-group removal operations that match the C++ GraphInfo metadata surface.\n\nConstraint: Product-fork slice only; Phase 3 io-api remains owned by the parallel session.
@keksmd
keksmd force-pushed the codex/java-info-api-parity-upstream branch from e47fc30 to 91018d0 Compare August 18, 2026 12:24
@keksmd keksmd changed the title feat(java): complete immutable metadata API parity feat(java): complete GraphAr metadata API parity Aug 18, 2026
@keksmd

keksmd commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

@SemyonSinchenko спасибо за конкретные замечания — все три исправил в 91018d0:

Заодно объединил это с близким metadata round-trip slice: labels, extra_info и typed list<T> теперь сохраняются через YAML. Локально graphar-info проходит 133 теста; upstream title/pre-commit/license уже зелёные, Java-info job ещё ожидает запуска.

Отдельно открыл небольшой независимый Java API PR #958 (issue #957): только storage interfaces, без adapter implementation и без зависимости от этого PR. Буду рад твоему ревью, когда будет время.

@SemyonSinchenko SemyonSinchenko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

~LGTM

@SemyonSinchenko

Copy link
Copy Markdown
Member

@SYaoJun @yangxk1 @Thespica Hi! Let's merge this one? LGTM overall. GAR Java is not published, so there cannot be any breaking changes and it is safe to merge imo.

@SYaoJun
SYaoJun merged commit 923f595 into apache:main Aug 18, 2026
4 of 6 checks passed
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.

4 participants