Skip to content

feat(go): add GraphAr info metadata support - #938

Draft
ZekiLiu wants to merge 7 commits into
apache:mainfrom
ZekiLiu:828-go-sdk-info
Draft

feat(go): add GraphAr info metadata support#938
ZekiLiu wants to merge 7 commits into
apache:mainfrom
ZekiLiu:828-go-sdk-info

Conversation

@ZekiLiu

@ZekiLiu ZekiLiu commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Reason for this PR

This draft PR shows the complete Go SDK metadata implementation direction for
#828.

The Go SDK should be able to represent GraphAr info metadata in Go, load and
save the same YAML schema used by the existing implementations, and validate
that the metadata layer interoperates with the repository fixtures.

This PR is stacked on top of the Go SDK bootstrap PR so reviewers can first
review the small module/CI foundation and then inspect the metadata API design
separately.

Refs #828.

What changes are included in this PR?

This PR adds the Go SDK metadata layer:

  • add primitive metadata types, including FileType, Cardinality,
    AdjListType, InfoVersion, and DataType
  • add GraphAr info model types, including Property, PropertyGroup,
    AdjacentList, VertexInfo, EdgeInfo, and GraphInfo
  • add YAML load/save support for GraphAr info files
  • add validation paths aligned with the existing GraphAr metadata rules
  • add cross-language interoperability and end-to-end tests over GraphAr
    fixtures

Stacking setup:

  • base branch: 828-go-sdk-module-bootstrap
  • head branch: 828-go-sdk-info

After the bootstrap PR is merged, this branch should be rebased onto the updated
main branch. Because GraphAr squash-merges PRs, the duplicate bootstrap commit
should be removed during that rebase.

Are these changes tested?

The intended local and CI checks are:

  • cd go/graphar && go mod tidy
  • cd go/graphar && go test ./...
  • cd go/graphar && go test -race ./...
  • cd go/graphar && make ci
  • pre-commit run --files <changed files>

The interop test is expected to use the repository testing/ fixtures when the
submodule is available.

Are there any user-facing changes?

Yes. This adds new Go SDK metadata APIs for GraphAr info files. It does not
change the existing C++, Java, Rust, Spark, or PySpark APIs.

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. (Not applicable: this PR does not change files 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.

ZekiLiu added 7 commits July 2, 2026 23:17
Module skeleton: go.mod (go 1.23.0), Makefile (fmt/vet/lint/test/coverage/ci
+ license header check), golangci config, GitHub Actions (matrix 1.23 +
stable), // Go mapping in licenserc.toml, package doc and README.
Enum-style value types with parse/format reciprocity, Equal and the package
error sentinels. Table-driven unit tests.
DataType with list and user-defined support, ParseDataType/String reciprocity,
nil-safe Equal, and a structural Validate that keeps list elements to the
subset every SDK parses. Unit tests.
Leaf metadata value types with an immutable queryable PropertyGroups index,
EffectivePrefix derivation, and Validate (CSV/json rules). Package error
sentinels. Unit tests.
Aggregate metadata with functional-options constructors, recursive Validate,
graph-level labels, extra_info and graph file references. Strict NewX; liberal
construction primitives for the load path. Unit tests.
Private DTO layer plus Load/Marshal/Save. Loading is liberal (parse-only; call
Validate to enforce rules); writing emits the standard form (adj lists as
ordered/aligned_by, divergence-only is_nullable) and tolerates legacy dialects.
Round-trips every testing/ graph.yml and asserts parsed content against the
ldbc/modern/nebula/neo4j/java fixtures. Skips when the submodule is absent.

@keksmd keksmd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ZekiLiu triage note rather than a full review — at 7,683 added lines across 44 files this is more than can be reviewed usefully in one pass, and it is currently blocked on two mechanical things.

It conflicts with main. GitHub reports CONFLICTING; the branch needs a rebase before the diff means anything.

golangci-lint is red. Five findings, all in tests:

go/graphar/info/edge_info_test.go:199:2       var-naming: don't use underscores in Go names; var types_ should be types (revive)
go/graphar/info/adjacent_list_test.go:76:6    dupArg: suspicious method call with the same argument and receiver (gocritic)
go/graphar/info/property_group_test.go:156:6  dupArg: suspicious method call with the same argument and receiver (gocritic)
go/graphar/info/property_test.go:75:6         dupArg: suspicious method call with the same argument and receiver (gocritic)
go/graphar/info/integration_test.go:45:10     appendAssign: append result not assigned to the same slice (gocritic)

The three dupArg hits look like deliberate reflexivity assertions (x.Equal(x)); if so they are worth keeping, but they need a //nolint:gocritic // reflexivity check with the reason spelled out, otherwise the next contributor will "fix" them by deleting the case. appendAssign at integration_test.go:45 is more likely a genuine bug — an append whose result goes somewhere other than the slice being appended to silently drops elements.

Suggestion on shape. #941 carves FileType / Cardinality / AdjListType out of this change into a reviewable ~1k-line slice, and that worked well — I was able to actually check it against the C++ and Java behaviour. Continuing that pattern here would help a lot: land the leaf types first, then Property / PropertyGroup, then VertexInfo / EdgeInfo, then GraphInfo and the integration tests. Each slice stays independently testable, and reviewers can give real feedback instead of skimming.

Happy to review the follow-up slices as they come out.

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