MDEV-36523: Load basic stats from trace into optimizer

[bsrikanth-mariadb]

This task loads the stats of the tables that are used in a query,
from the trace into the optimizer. This feature is controlled
by optimizer_replay_context, and points to an user defined variable name,
that has the context information.
The stats such as num_of_records present in the table, indexes if present,
their names, along with the average number of records_per_key
with in each index, cost of reading an index are loaded from the trace.
Additionally, stats from range analysis i.e. ranges, and the
corresponding number of records, along with their costs are also
loaded from the trace.

The trace context which is in JSON format is firstly set into the
user defined session variable, and this variable name is set to
optimizer_replay_context system variable.
Later, when a user issues a query, the contents of the json context is
parsed and an in memory representation is built using the class
Optimizer_context_replay. This class is then used by the optimizer
to update and save original values of the tables, indexes, and range stats
in the methods "set_statistics_for_table()" of sql_statistics.cc, and
"check_quick_select()" of opt_range.cc.
After the query gets finished, the statistics that were updated
in the optimizer are restored back to the saved original values.

The entry point for parsing the json structure is in
"mysql_execute_command()" of sql_parse.cc, and similarly exit point
i.e. to restore the saved stats of the optimizer is at the end
of the same method.

Two new warning/error messages are introduced:
ER_JSON_OPTIMIZER_REPLAY_CONTEXT_PARSE_FAILED, and
ER_JSON_OPTIMIZER_REPLAY_CONTEXT_MATCH_FAILED

[spetrunia]

class Read_named_member
{
public:
  const char *name;
  Read_value &&value;
  const bool &&is_optional;

Q1: Why is "is_optional" rvalue reference?
Q2: And is it ever used?

[spetrunia]

Do we need two functions: parse_named_members_array and read_all_elements . Should it be parse_named_members_array everywhere?

[spetrunia]

Please also go through class Optimizer_context_replay and see if names of members are good. It looks like some names could be made better .
Rename opt_trace_ddl_info.* to opt_replay_context.* or something like that. it is not ddl_info anymore...

[bsrikanth-mariadb]

I repeat myself: handler.cc:

/*
  Time for a full table data scan. To be overrided by engines, should not
  be used by the sql level.
*/
IO_AND_CPU_COST handler::scan_time()
{
  IO_AND_CPU_COST cost;
  bool rc= current_thd->opt_ctx_replay
               ? current_thd->opt_ctx_replay->get_read_cost(table, &cost)
               : true;
  if (rc)

Q1: Don't use current_thd, use table->in_use. Q2: the comment says "to be overrided by engines". If this is the case, how would saving/replaying context here work? If an engine can override this?

changed to use ha_scan_time()

[bsrikanth-mariadb]

Indentation is STILL not fixed:

In Read_list_of_ha_rows, can't agree whether we're returning true/false or 1/0:

  int read_container(json_engine_t *je, String *err_buf) override
  ...
      if (list_values->push_back(records_ptr) || json_scan_next(je))
        return 1;
    }
    return false;

oh oh! Taken care now.

[bsrikanth-mariadb]

I see optimizer_replay_context as a mysqld parameter? Should it really be there? If not , can we remove? (I can see this in mysqld--help.result change).

Optimizer_record_context SHOULD be a parameter (at least in debug version), so we can run everything with

--mysqld="optimizer_trace=enabled  --optimizer_record_context=on"

and catch obvious failures. (Did you do this recently).

removed optimizer_replay_context from being the mysqld command line argument

[bsrikanth-mariadb]

VARIABLE_NAME	OPTIMIZER_RECORD_CONTEXT
VARIABLE_SCOPE	SESSION ONLY
VARIABLE_TYPE	BOOLEAN
VARIABLE_COMMENT	Controls storing of optmizer context of all the tables that are referenced in a query

Typo, optimzer. Consider changing to:

If set, record the query optimizer context (currently into the Optimizer trace). Recorded context can be later
replayed to reproduce the query optimizer behavior

sure, done.

[bsrikanth-mariadb]

VARIABLE_NAME	OPTIMIZER_REPLAY_CONTEXT
VARIABLE_SCOPE	SESSION ONLY
VARIABLE_TYPE	VARCHAR
VARIABLE_COMMENT	system variable that points to a user variable containing the context of a query in JSON format, so as to reuse the stored stats

Consider changing to Variant 1:

Name of the user-defined variable (without the @) which has the optimizer context for replay. 

Variant 2 :

Saved optimizer context to replay. The context should be put into a @user_var, and this variable should be set to "user_var."

Variant 3:

If set, provides optimizer context to replay.  The context should be put into a user-defined variable (@var),
this variable should contain its name (var).

sure, picked the third variant.

[bsrikanth-mariadb]

Warnings:
Warning	4243	Failed to parse json structure present in the optimizer_replay_context variable: "copy_cost" element not present at offset 1519.

Let's re-word

Failed to parse saved optimizer context: %s 

sure, done.

[bsrikanth-mariadb]

class Read_named_member
{
public:
  const char *name;
  Read_value &&value;
  const bool &&is_optional;

Q1: Why is "is_optional" rvalue reference? Q2: And is it ever used?

treating is_optional as a regular bool field, and yes it is used.

[bsrikanth-mariadb]

Do we need two functions: parse_named_members_array and read_all_elements . Should it be parse_named_members_array everywhere?

Actually, changed the names for better readability now. Yes, we require both

[bsrikanth-mariadb]

Please also go through class Optimizer_context_replay and see if names of members are good. It looks like some names could be made better . Rename opt_trace_ddl_info.* to opt_replay_context.* or something like that. it is not ddl_info anymore...

yes, changed them.