danielaparker / jsoncons

A C++, header-only library for constructing JSON and JSON-like data formats, with JSON Pointer, JSON Patch, JSON Schema, JSONPath, JMESPath, CSV, MessagePack, CBOR, BSON, UBJSON
https://danielaparker.github.io/jsoncons
Other
700 stars 158 forks source link

JSON path issue with custom allocator #407

Closed boazsade closed 1 year ago

boazsade commented 1 year ago

Describe the bug Using jsoncons with custom alloctor is supported. For example you can parse JSON using custom allocator: see for example: sample_allocators.hpp

template<typename T> my_allocator {
// Interface is the same as std::allocator
};

// then:
 using my_json = basic_json<char, sorted_policy, my_allocator<char>>;
 json_decoder<my_json, my_allocator<char>> decoder(result_allocator_arg, my_allocator<char>{},
                                                               my_allocator<char>{});
basic_json_reader<char, string_source<char>, my_allocator<char>> reader(input, decoder, my_allocator<char>{});
reader.read();

 my_json j = decoder.get_result();
 std::cout << pretty_print(j) << "\n";

This code would compile and run as expected. The problem will start when I want use this with json path:

std::string_view p{"$.store.book[?(@.price < 10)].author"};
 auto expr = jsoncons::jsonpath::make_expression<my_json>(p);   // this will not compile
 auto r = expr.evaluate(j);

I'm getting an error that:

/include/jsoncons_ext/jsonpath/jsonpath_selector.hpp:156:75: note:   no known conversion for argument 3 from ‘const key_type’ {aka ‘const std::__cxx11::basic_string<char, std::char_traits<char>, my_allocator<char> >’} to ‘const string_type&’ {aka ‘const std::__cxx11::basic_string<char>&’}
  156 |                                                        const string_type& identifier,
      |                                                        ~~~~~~~~~~~~~~~~~~~^~~~~~~~~

This issue is critical since my use case require that use of custom alloctor that do not use the global heap and require the use of both JSON and JSONPATH.

Expecting: Being able to compile both JSON and JSONPATH using custom alloctors. It would be even nicer to use the new C++17 polymorphic memory resource.

See above - code fail to compile. Enumerate the steps to reproduce the bug You can add the code example above to the file json_reader_examples.cpp line 150. Note that I failed to build this example as well (there is an issue with FreelistAllocator alloctor missing default constructor). Include a small, self-contained example if possible See above for the problem description. Please note that I found the location at the code at which the issue was triggered by, I can PR it, or if you prefer I can send it add it to this issue. What compiler, architecture, and operating system?

What jsoncons library version? Latest from github (commit c369979d8cf12c82b643fc71d5261b3d28f7d4d1).

danielaparker commented 1 year ago

I can fix the custom allocator issues for jsonpath.

I have a question about how you use custom allocators though. Do you need the ability to provide one allocator for results, e.g. a basic_json value, and a different one for memory used internally by jsoncons for work areas? Or would you be fine with providing one allocator that was used for both purposes? I originally followed the first approach (supporting the ability to provide separate allocators), but it leads to complicated interfaces for free functions like decode_json, requiring both result_allocator_arg and temp_allocator_arg options. For free functions it would be simpler to support just one allocator with std::allocator_arg.

Comparing with the standard library, when the standard library has functions such as stable_sort that require internal work areas, they typically don't support any way of changing the internally used allocators.

boazsade commented 1 year ago

Thanks for the quick reply. I think that in our case the simpler option - the second one is better. You can look at our project at this URL We have a model of shared nothing, which mean that each thread should not depend on other threads in term of the execution and data access. This is why we need our own allocator, a one that don't use the global malloc and free, but an allocator that allocate memory from thread local store. In our case, it would best to use our allocator for all use cases, but if this make it easier, it is not critical to have the working area using the standard allocator (since this is a temporary data that would not be saved after it being used). Our use case is: Save a parsed JSON from a string message internally, then update it or extract data from it using json path. See for example this command. You are saving new json with

`JSON.SET doc1 $ '{"a":1, "b": 2, "nested": {"a": 3}, "c": null}'`

Where the "doc1" is a key in the store and "$" is the root for the docuement. So to store it we would parse it and save a basic_json object in memory So we're storing this by:

using json_type = jsoncons::basic_json<char, sorted_policy, thread_local_allocator>;

json_type json = json_type::parse(input);
StoreByKey("doc1", json);

Then you're getting a value with

`JSON.GET doc1 $..a`

In this case we will get back the basic_json object from memory, and run

json_type stored_json = FindByKey("doc1");
auto expr = jsoncons::jsonpath::make_expression<json_type>(path); // path is  $..a
auto r = expr.evaluate(stored_json);
ReportResults(r);

Hope this helps Thanks again BTW in my case in order to make this compile I just wrote a simple conversion function from string to string<char_type, trait, alloc> in the file "jsonpath_selector.hpp" and used it on the key() for "wildcard_selector" "recursive_selector" and "filter_selector"

boazsade commented 1 year ago

Hi @danielaparker - any news on this issue? thanks in advance

danielaparker commented 1 year ago

Apologies, working on it.

danielaparker commented 1 year ago

This code should now work on master:

using my_json = basic_json<char,sorted_policy,FreelistAllocator<char>>;
std::string input = R"(
[ 
  { 
      "author" : "Haruki Murakami",
      "title" : "Hard-Boiled Wonderland and the End of the World",
      "isbn" : "0679743464",
      "publisher" : "Vintage",
      "date" : "1993-03-02",
      "price": 9
  },
  { 
      "author" : "Graham Greene",
      "title" : "The Comedians",
      "isbn" : "0099478374",
      "publisher" : "Vintage Classics",
      "date" : "2005-09-21",
      "price": 15.74
  }
]
)";
std::cout << "Statefull allocator test" << "\n";

json_decoder<my_json,FreelistAllocator<char>> decoder(result_allocator_arg, FreelistAllocator<char>(1),
       FreelistAllocator<char>(2));

auto myAlloc = FreelistAllocator<char>(3);        

basic_json_reader<char,string_source<char>,FreelistAllocator<char>> reader(input, decoder, myAlloc);
reader.read();

my_json j = decoder.get_result();
std::cout << pretty_print(j) << "\n";

std::string_view p{"$[?(@.price < 10)].author"};
auto expr = jsoncons::jsonpath::make_expression<my_json>(std::allocator_arg, myAlloc, p);   
auto r = expr.evaluate(j);
std::cout << pretty_print(r) << "\n";
danielaparker commented 1 year ago

Some test cases here

danielaparker commented 1 year ago

Fix is in 0.170.0