Re: [Maria-developers] MDEV-23766:Optimizer tracing code is prone to producing invalid JSON
![](https://secure.gravatar.com/avatar/c7422c379fec4aa4d99a8a137bf10594.jpg?s=120&d=mm&r=g)
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? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net
![](https://secure.gravatar.com/avatar/c7422c379fec4aa4d99a8a137bf10594.jpg?s=120&d=mm&r=g)
Hi Serg, This is review input for commit 8fb54fa006367e3a9cbe32dc1734c980cc5d5096. Item #1: I've tried adding some tests and neither of them fails. Can you check what's going on? I'm attaching the patch with tests to this email. Item #2: please improve the commit comment as was requested in my previous e-mail: On Thu, Oct 28, 2021 at 04:58:35PM +0300, Sergey Petrunia wrote:
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).
BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net
participants (1)
-
Sergey Petrunia