← Back to team overview

maria-developers team mailing list archive

Re: MDEV-23766:Optimizer tracing code is prone to producing invalid JSON


Hi Serg,

Ok the patch moving in the right direction but it's not there, yet.

Please find the input below.

First, input on the commit comment. Please consider it as a request applying
to ALL further commits to the MariaDB codebase.

> MDEV-23766: implemented requested debug checks

Imagine somebody looking at this in a few years. Will they know what checks
were requested? They might get a clue by looking at the Jira text, but the
idea to make the commit comments concise and self-contained descriptions.

Something like this:

Line #1: one-line description of the patch:

>> MDEV-23766: Make attempts to write invalid JSON cause assertion failures

Subsequent lines:

>> Make JSON writing API (class Json_writer) check the produced JSON document
>> is valid. The following checks are made:
>> - JSON objects must contain named members
>> - JSON arrays must contain unnamed members.
>> - (TODO: add other restrictions we're enforcing).

It is not clear how the checker interacts with Single_line_formatting_helper.
The code like one the one I'm quoting below shows some non-trivial overlap:

> @@ -28,7 +36,16 @@ void Json_writer::append_indent()
>  void Json_writer::start_object()
>  {
> +#ifndef NDEBUG
> +  DBUG_ASSERT(got_name == named_item_expected());
> +  named_items_expectation.push_back(true);
> +  size_t depth= named_items_expectation.size();
> +#endif
>    fmt_helper.on_start_object();
> +#ifndef NDEBUG
> +  if (depth != named_items_expectation.size())
> +    named_items_expectation.pop_back();
> +#endif
>    if (!element_started)
>      start_element();

This looks very cryptic. Can you elaborate about this?

Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

Follow ups