GH-50944: [C++] Replace RapidJSON with simdjson in JSON chunker - #50945
GH-50944: [C++] Replace RapidJSON with simdjson in JSON chunker#50945Reranko05 wants to merge 2 commits into
Conversation
Reranko05
added
CI: Extra: C++
e9fc9fe to
be4c2a1
Compare
pitrou
left a comment
pitrou
left a comment
There was a problem hiding this comment.
I don't understand why this is parsing JSON by hand?
It seems that we might be able to use simdjson::ondemand::parser::iterate_many.
|
|
||
| namespace arrow { | ||
|
|
||
| using std::string_view; |
There was a problem hiding this comment.
Can we remove this if it's not useful anymore?
| if (escape_next) { | ||
| escape_next = false; | ||
| continue; | ||
| } | ||
|
|
||
| if (c == '\\' && in_string) { | ||
| escape_next = true; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
This does not account for unicode escapes. Do we have to parse JSON by hand like this?
| std::string combined; | ||
| combined.reserve(partial.size() + block.size()); | ||
| combined.append(partial); | ||
| combined.append(block); |
There was a problem hiding this comment.
We should only concatenate if both substrings are non-empty.
| return Status::Invalid("JSON chunk error: invalid data at end of document"); | ||
| return Status::Invalid("JSON parse error: Invalid value"); |
| const size_t start = ConsumeWhitespace(block); | ||
|
|
||
| if (start < block.size()) { | ||
| const char first_char = block[start]; | ||
|
|
||
| if (first_char != '{' && first_char != '[') { | ||
| const size_t remaining_len = block.size() - start; | ||
|
|
||
| if (remaining_len > 1 || (first_char != '}' && first_char != ']')) { | ||
| return Status::Invalid("JSON parse error: Invalid value"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Please add comments explaining what this does and why it is necessary.
github-actions
Bot
added
awaiting committer review
I initially tried using That said, I agree that parsing JSON manually here is not ideal. I'll revisit this using |
Rationale for this change
This PR continues the simdjson migration by replacing the RapidJSON-based JSON boundary detection used by the JSON chunker.
The existing implementation uses RapidJSON's streaming parser to identify complete JSON values. This change replaces that logic with structural boundary detection and simdjson validation.
Changes
simdjson::dom::parser.Fixes: #50944