Documents better the type method and makes is_null return an error condition in some instances

[lemire]

Some extra documentation and a substantial change: the is_null method will not return an error when the input is obviously not JSON (a token begins with 'n' but is not null).

[TysonAndre]

simdjson_inline bool document::is_null() noexcept {
  return get_root_value_iterator().is_root_null();
}

I wasn't sure about this - I think the root level is special cased for document (not value), and this won't throw for doc specifically. i.e. it returns bool and not simdjson_result<bool>

Makes sense for the value iteration

[TysonAndre]

simdjson_warn_unused simdjson_inline simdjson_result<bool> value_iterator::get_root_bool() noexcept {
  auto max_len = peek_start_length();
  auto json = peek_root_scalar("bool");
  uint8_t tmpbuf[5+1];
  if (!_json_iter->copy_to_buffer(json, max_len, tmpbuf)) { return incorrect_type_error("Not a boolean"); }
  auto result = parse_bool(tmpbuf);
  if(result.error() == SUCCESS) {
    if (!_json_iter->is_single_token()) { return TRAILING_CONTENT; }
    advance_root_scalar("bool");
  }
  return result;
}
simdjson_inline bool value_iterator::is_root_null() noexcept {
  // If there is trailing content, then the document is not null.
  if (!_json_iter->is_single_token()) { return false; }
  auto max_len = peek_start_length();
  auto json = peek_root_scalar("null");
  bool result = (max_len >= 4 && !atomparsing::str4ncmp(json, "null") &&
         (max_len == 4 || jsoncharutils::is_structural_or_whitespace(json[5])));
  if(result) { advance_root_scalar("null"); }
  return result;
}

The implementations of helpers for is_bool throw, but not is_null. (just for the root document)

It seems like use cases like this might benefit from a helper such as doc.throw_if_not_null() or changing doc.is_null to simdjson_result

[TysonAndre]

So examples such as json2json are also affected

» echo tada > example.json
» tools/json2json --file example.json --ondemand
tada

[lemire]

So examples such as json2json are also affected

I don't think that's related.

[lemire]

The json2json utility just calls...

/**
 * Create a string-view instance out of a document instance. The string-view instance
 * contains JSON text that is suitable to be parsed as JSON again.
 */
inline simdjson_result<std::string_view> to_json_string(SIMDJSON_IMPLEMENTATION::ondemand::document& x) noexcept;

It just returns a direct pointer to the source, nothing more.

[lemire]

@TysonAndre It turns out that the design was not ideal. I think we should validate null values for the user. I am now proposing to change the behaviour somewhat so that it is easier to validate the null values. This will be a breaking change, but it will affect few users, I expect, and even the affected users can deal with the change without much hassle.

Note that it is entirely distinct from extracting a string_view instance from a document instance: that is not meant to be a validating operation. The idea in ondemand is that if you have an array, you can just compute the string_view instance and print it out... If you want to actually parse and validate it, then you have to do more work. By default, ondemand is lazy: it does what it says and no more.

[TysonAndre]

Is there a reason to return false instead of error()? If so, document it?

I thought error() would make more sense to return

[lemire]

Good catch.

[TysonAndre]

Nit: Can this be shortened by replacing this and error check with SIMDJSON_TRY(peek_non_root_scalar("null")).get(is_null_value))

[lemire]

Point taken.

[TysonAndre]

I think we should validate null values for the user. I am now proposing to change the behaviour somewhat so that it is easier to validate the null values. This will be a breaking change, but it will affect few users, I expect, and even the affected users can deal with the change without much hassle.

The proposed change makes sense to me

Note that it is entirely distinct from extracting a string_view instance from a document instance: that is not meant to be a validating operation. The idea in ondemand is that if you have an array, you can just compute the string_view instance and print it out... If you want to actually parse and validate it, then you have to do more work. By default, ondemand is lazy: it does what it says and no more.

Got it

[TysonAndre]

lgtm

[lemire]

Merging. I will issue a release.