maria-developers team mailing list archive

[Sergei Petrunia <psergey@xxxxxxxxxxxx>]

Hello Igor,

Please find some review feedback below.

=====
sql_statistics.cc has this line with comment:

> If the value the variable 'use_stat_tables' is set to
> "preferably" the optimizer uses a particular statistical data only if
> it can't be found in the statistical data.
 
the part

> uses a particular statistical data 
> only if it can't be found 
> in the statistical data.

doesn't seem to make any sense. Is this a typo?

=====
sql_statistics.cc: collect_statistics_for_table() has this code:

>       table_field->collected_stats->cleanup();
>   }
> bitmap_clear_all(table->write_set);

Why suddenly no indentation on the line with the bitmap_clear_all() call? 
This looks as if a merge has gone wrong, please fix.

=====
sql_statistics.cc has this :

> class Table_stat: public Stat_table 
> {
>   
> private:
>
>   Field *db_name_field;     /* Field for the column table_stats.db_name */

Why do all class definitions start with 'private:'?  It is active by
default in C++. The code looks as if it was auto-translated from some 
other language.

=====
I see that my progress reporting patch from November is not part of the tree
yet.

=====
I don't any code that would check if table DDLs of statistical tables are 
what the server expects them to be.

I think the check should be made, just like it is made for mysql.proc and other
tables.

==== 
Then I started to play with the code and have filed
MDEV-4350, MDEV-4357, MDEV-4359, MDEV-4360, MDEV-4362, MDEV-4363, MDEV-4364.

I find it difficult to proceed when there are so many simple counterexamples.

BR
 Sergei
-- 
Sergei Petrunia, Software Developer
Monty Program AB, http://askmonty.org
Blog: http://s.petrunia.net/blog