js8544 commented on code in PR #14242:
URL: https://github.com/apache/arrow/pull/14242#discussion_r982582506


##########
cpp/src/arrow/json/parser.cc:
##########
@@ -610,12 +627,22 @@ class HandlerBase : public BlockParser,
   }
 
   bool RawNumber(const char* data, rj::SizeType size, ...) {
-    status_ = AppendScalar<Kind::kNumber>(builder_, std::string_view(data, 
size));
+    if (builder_.kind == Kind::kNumberOrString) {
+      status_ =
+          AppendScalar<Kind::kNumberOrString>(builder_, std::string_view(data, 
size));
+    } else {

Review Comment:
   I'm not sure if we want this. This way we need add two if checks. One is for 
`kind_changed` as you write here. The other is when doing `Cast<kind>(builder)` 
later. We need to do
   ```cpp
   if (builder.kind != Kind::kNumberOrString) {
     RETURN_NOT_OK(Cast<kind>(builder)->Append(index, value_length));
   } else {
     RETURN_NOT_OK(Cast<Kind::kNumberOrString>(builder)->Append(index, 
value_length));
   }
   ```
   I think this way the code is more complex. What do you think?



-- 
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.

To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to