jmespath issue with reusing compiled expressions

[siemib]

First to all thank you providing an incredible library!

We use JMESPath implementation quite extensively but recently we had a big higher throughput requirements which forced use to reuse compiled expression. The usual workaround for this bug (which has been around for a while) was to compile expression every time before usage. jsoncons::jmespath::search takes around 3x longer than evaluate on expression

Describe the bug

I did a bit more digging, please have a look at the snippet

#include <jsoncons/json.hpp>
#include <jsoncons_ext/jmespath/jmespath.hpp>

int main(int argc, char *argv[]) {
  using namespace jsoncons; // for convenience

  std::string jtext = R"(
            {
              "people": [
                {
                  "age": 20,
                  "other": "foo",
                  "name": "Bob"
                },
                {
                  "age": 25,
                  "other": "bar",
                  "name": "Fred"
                },
                {
                  "age": 30,
                  "other": "baz",
                  "name": "George"
                }
              ]
            }
        )";

  auto expr = jmespath::jmespath_expression<json>::compile("sum(people[].age)");

  json doc = json::parse(jtext);

  json result = expr.evaluate(doc);
  std::cout << pretty_print(result) << "\n\n";
}

It will result in exception Function called with wrong number of arguments or just outright SIGSEGV
but this problem will never occur when using jsoncons::jmespath::search. This seem not to happen with every expression. We seem to have issues with functions like max or sum i.e max([].my_object.sub_field)

But now if we re-arrange to first ::parse and then ::compile everything is OK and I get result 75.0

json doc = json::parse(jtext);
auto expr = jmespath::jmespath_expression<json>::compile("sum(people[].age)");

It looks to me like ::parse breaks a global state that compile introduces

What compiler, architecture, and operating system?

• Compiler: gcc 9.2 / gcc 10.2
• Architecture (e.g. x86, x64) x64
• Operating system: linux

What jsoncons library version?
release v0.162.2, however this problem has been reoccurring for all previous versions

[siemib]

updated with more clear example & way to reproduce

[siemib]

interesting it seems like when the

jsoncons::jmespath::detail::jmespath_evaluator<json, const json &> evaluator{};

goes out of scope the constructed expression becomes invalid

try with

  std::string expr = "sum(people[].age)";

  std::error_code ec;

  std::unique_ptr<jmespath::jmespath_expression<json>> expr3 = nullptr;

  {
    jsoncons::jmespath::detail::jmespath_evaluator<json, const json &>
        evaluator{};

    expr3 = std::make_unique<jmespath::jmespath_expression<json>>(
        evaluator.compile(expr.data(), expr.size(), ec));
  }

  json doc = json::parse(jtext);

  json result = expr3->evaluate(doc);
  std::cout << pretty_print(result) << "\n\n";

remove {} around expr3 block to make this work

[siemib]

indeed I just created a struct to hold number of different expression to evaluate and that fixed the problem

struct FieldConfig {
  using Expression = jsoncons::jmespath::jmespath_expression<jsoncons::json>;
  using Evaluator =
      jsoncons::jmespath::detail::jmespath_evaluator<jsoncons::json,
                                                     const jsoncons::json &>;

  const std::string name;
  const std::string expression_str;

  Evaluator evaluator;
  Expression expression;

  std::error_code ec;
}

std::vector<std::unique_ptr<FieldConfig>> configs_;
}

it looks like as long as Evaluator is alive the expression works fine. I noticed there is a lot of std::move when the expression is created - it makes me thing that the original context & stack that is moved into the expression has some reference to perhaps dynamic_storage that has a vector to some unique_ptrs

[siemib]

just trying that branch :)

[siemib]

as to my last comment -> unfortunately my testing suite just finished and found a regression of functionality if I use the evaluator in FieldConfig. It seems like it's not a valid proposal.

I just tried the commit #281541d and unfortunately it fails

C++ exception with description "assertion 'state_stack_.back() == path_state::rhs_expression' failed at  <> :0" thrown in the test body.

I tried to evaluate 3 expression on single json object

[
    {"name" : "field_1", "expression" : "group.value"},
    {"name" : "field_2", "expression" : "array[0].value"},
    {"name" : "field_3", "expression" : "nullable.value"}
]

and the object tested against

{
    "group": {
      "value": 1
    },
    "array": [
      {"value": 2}
    ]
}

[danielaparker]

Work in progress :-) Thanks for reporting.

I think the original issue was about memory ownership for function objects, that they were being owned by the original Evaluator rather than being moved into the jmespath_expression. I believe that's been fixed on the branch.

Can you provide a complete example for a failing instance?

[danielaparker]

I tried

        std::string jtext = R"(
{
    "group": {
      "value": 1
    },
    "array": [
      {"value": 2}
    ]
}
        )";

        json doc = json::parse(jtext);

        std::unique_ptr<jmespath::jmespath_expression<json>> expr1;
        {
            expr1 = jsoncons::make_unique<jmespath::jmespath_expression<json >>(jmespath::jmespath_expression<json>::compile("group.value"));
        }
        json result1 = expr1->evaluate(doc);
        std::cout << pretty_print(result1) << "\n\n";

        std::unique_ptr<jmespath::jmespath_expression<json>> expr2;
        {
            expr2 = jsoncons::make_unique<jmespath::jmespath_expression<json >>(jmespath::jmespath_expression<json>::compile("array[0].value"));
        }
        json result2 = expr2->evaluate(doc);
        std::cout << pretty_print(result2) << "\n\n";

        std::unique_ptr<jmespath::jmespath_expression<json>> expr3;
        {
            expr3 = jsoncons::make_unique<jmespath::jmespath_expression<json >>(jmespath::jmespath_expression<json>::compile("nullable.value"));
        }
        json result3 = expr3->evaluate(doc);
        std::cout << pretty_print(result3) << "\n\n";

and that seems to be passing in all test configurations on branch jmespath_317. I need a case that fails.

[danielaparker]

I just tried the commit #281541d and unfortunately it fails

C++ exception with description "assertion 'state_stack_.back() == path_state::rhs_expression' failed at  <> :0" thrown in the test body.

That assertion (state_stack_.back() == path_state::rhs_expression) is triggered during parsing of the JMESPath expression, and indicates that an invalid state was reached during parsing, so I don't see how that could be related to the changes made on branch_317. But I'd like to see the JMESPath expression that resulted in that.

[siemib]

Yes that last exception on was an error on my side. I just tested your branch and it all seems now correct. Thanks a lot for very quick response!

[danielaparker]

The changes have been uploaded to master, once all tests have passed, we'll put out a patched release 0.163.2.