feat(go): add GraphAr info metadata support - #938
Conversation
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
left a comment
There was a problem hiding this comment.
@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.
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:
FileType,Cardinality,AdjListType,InfoVersion, andDataTypeProperty,PropertyGroup,AdjacentList,VertexInfo,EdgeInfo, andGraphInfofixtures
Stacking setup:
828-go-sdk-module-bootstrap828-go-sdk-infoAfter the bootstrap PR is merged, this branch should be rebased onto the updated
mainbranch. Because GraphAr squash-merges PRs, the duplicate bootstrap commitshould be removed during that rebase.
Are these changes tested?
The intended local and CI checks are:
cd go/graphar && go mod tidycd go/graphar && go test ./...cd go/graphar && go test -race ./...cd go/graphar && make cipre-commit run --files <changed files>The interop test is expected to use the repository
testing/fixtures when thesubmodule 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
make cpplintbefore submitting when changed files are in thecppdirectory. (Not applicable: this PR does not change files in thecppdirectory.)pre-commit runbefore commit the changed files.