fix(query): valToBytes []byte fast path for string-typed values#9710
Open
shaunpatterson wants to merge 1 commit into
Open
fix(query): valToBytes []byte fast path for string-typed values#9710shaunpatterson wants to merge 1 commit into
shaunpatterson wants to merge 1 commit into
Conversation
Before this change, when a types.Val with Tid=StringID or Tid=DefaultID
held its payload as []byte (the storage form coming back from posting
list reads), valToBytes fell through to json.Marshal(v.Value) — and
encoding/json's default for []byte is base64. The JSON response then
contained base64-encoded text where callers expected a plain string.
Callers that already work with []byte payloads include worker/task.go
and query/query.go, which build types.Val{Tid: StringID, Value:
[]byte(...)}; the prior default case mis-encoded such values.
The fix adds an explicit []byte branch that routes through
stringJsonMarshal (the same path the case-string branch uses) via
unsafe.String, avoiding the json.Marshal base64 step entirely. The
zero-length slice is handled separately to keep the empty-string
output as []byte("\"\"") without an intermediate allocation.
Perf (BenchmarkFastJsonNode2Chilren, 5x3s avg, Apple M4 Max):
ns/op 88.13M -> 86.58M (-1.76%)
B/op 50.33M -> 46.14M (-8.32%)
allocs/op 524315 (unchanged)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When a
types.ValwithTid=StringIDorTid=DefaultIDholds its payload as[]byte(the storage form coming back from posting list reads),valToBytespreviously fell through tojson.Marshal(v.Value)— andencoding/json's default for[]byteis base64. The JSON response then contained base64-encoded text where callers expected a plain string.Callers that already work with
[]bytepayloads includeworker/task.goandquery/query.go, which buildtypes.Val{Tid: StringID, Value: []byte(...)}; the prior default case mis-encoded such values.The fix adds an explicit
[]bytebranch that routes throughstringJsonMarshal(the same path thecase stringbranch uses) viaunsafe.String, avoiding thejson.Marshalbase64 step entirely. The zero-length slice is handled separately to keep the empty-string output as[]byte("\"\"")without an intermediate allocation.A new table-driven test
TestValToBytesByteSlicecovers empty, ASCII, UTF-8, JSON-special, control-char, and U+2028/U+2029 inputs against bothStringIDandDefaultID, asserting the output equals the JSON-quoted form (never base64).Perf (
BenchmarkFastJsonNode2Chilren, 5×3s avg, Apple M4 Max):Checklist
fix:,feat:,chore:,ci:, etc.Thank you for your contribution to Dgraph!
🤖 Generated with Claude Code