pitrou commented on a change in pull request #9629:
URL: https://github.com/apache/arrow/pull/9629#discussion_r601513130
##########
File path: cpp/src/arrow/testing/json_internal.cc
##########
@@ -1231,12 +1231,29 @@ class ArrayReader {
return FinishBuilder(&builder);
}
+ int64_t parseOffset(const rj::Value& json_offset) {
Review comment:
This should be `ParseOffset` as per the style guide and naming
conventions.
##########
File path: cpp/src/arrow/testing/json_internal.cc
##########
@@ -1246,8 +1263,14 @@ class ArrayReader {
const rj::Value& val = json_data_arr[i];
DCHECK(val.IsString());
+ int64_t offsetStart = parseOffset(json_offsets[i]);
Review comment:
Should be `offset_start` and `offset_end`
##########
File path: cpp/src/arrow/testing/json_internal.cc
##########
@@ -1231,12 +1231,29 @@ class ArrayReader {
return FinishBuilder(&builder);
}
+ int64_t parseOffset(const rj::Value& json_offset) {
+ DCHECK(json_offset.IsInt() || json_offset.IsInt64() ||
json_offset.IsString());
+
+ if (json_offset.IsInt()) {
Review comment:
Suggestion:
```c++
if (json_offset.IsInt64()) {
return json_offset.GetInt64();
} else {
return UnboxValue<Int64Type>(json_offset);
}
```
##########
File path: cpp/src/arrow/ipc/dictionary.h
##########
@@ -79,6 +79,7 @@ class ARROW_EXPORT DictionaryFieldMapper {
Result<int64_t> GetFieldId(std::vector<int> field_path) const;
int num_fields() const;
+ int num_dicts() const;
Review comment:
Add a comment or docstring here explaining the difference with
`num_fields`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
