maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Sergey!

On Jul 07, svoj@xxxxxxxxxxx wrote:
> revision-id: 00ed36700addfbce91940223cce4ccd6ee5823a1
> parent(s): f37f19a6c790100cc2ae5f483ba4db34a517f0ae
> committer: Sergey Vojtovich
> branch nick: 10.1
> timestamp: 2014-07-07 14:32:13 +0400
> message:
> 
> MDEV-4262 - P_S discovery
> 
> Discover P_S tables automatically.
> 
> Most of this patch is code clean-up:
> - removed tests and code responsible for P_S tables correctness verification
> - always return error from ha_perfschema::create()
> - install/upgrade scripts won't create P_S tables anymore

Great work, thanks!

The patch is pretty much "ok to push", but, first I want to understand
why you deleted table_file_summary.cc file.

See other minor comments below.

> diff --git a/mysql-test/suite/perfschema/include/upgrade_check.inc b/mysql-test/suite/perfschema/include/upgrade_check.inc
> index 52d4cfd..1532de8 100644
> --- a/mysql-test/suite/perfschema/include/upgrade_check.inc
> +++ b/mysql-test/suite/perfschema/include/upgrade_check.inc
> @@ -3,7 +3,6 @@
>  #
>  
>  --source include/count_sessions.inc
> ---error 1
>  --exec $MYSQL_UPGRADE --skip-verbose --force > $out_file 2> $err_file
>  --source include/wait_until_count_sessions.inc

The rest of this upgrade_check.inc file does (according to the comment)
"Verify that mysql_upgrade complained about the performance_schema"
perhaps you should change the comment to say "does not complain" ?

>  
> diff --git a/scripts/mysql_performance_tables.sql b/scripts/mysql_performance_tables.sql
> index d495578..3061603 100644
> --- a/scripts/mysql_performance_tables.sql
> +++ b/scripts/mysql_performance_tables.sql
> @@ -53,1427 +53,3 @@ SET @str = IF(@broken_pfs = 0, @cmd, 'SET @dummy = 0');
>   PREPARE stmt FROM @str;
>   EXECUTE stmt;
>   DROP PREPARE stmt;

Why did you keep the above? I'd think that a simple

  DROP DATABASE IF EXISTS performance_schema

should be enough to replace the whole file.
And it could be moved back to mysql_system_tables.sql,
so this file could be completely removed.

> -
> -#
> -# From this point, only create the performance schema tables
> -# if the server is built with performance schema
> -#
> -
> -set @have_pfs= (select count(engine) from information_schema.engines where engine='PERFORMANCE_SCHEMA' and support != 'NO');
> -
> -#
> -# TABLE COND_INSTANCES
> -#
...
> diff --git a/storage/perfschema/pfs_engine_table.cc b/storage/perfschema/pfs_engine_table.cc
> index 958a2bd..6f0a363 100644
> --- a/storage/perfschema/pfs_engine_table.cc
> +++ b/storage/perfschema/pfs_engine_table.cc
> @@ -1440,5 +1319,17 @@ end:
>    DBUG_RETURN(false);
>  }
>  
> +int pfs_discover_table_names(handlerton *hton, LEX_STRING *db,
> +                             MY_DIR *dir,

hm, no "unused argument" compiler warnings? okay...

> +                             handlerton::discovered_list *result)
> +{
> +  if (compare_table_names(db->str, PERFORMANCE_SCHEMA_str.str))
> +    return 0;
> +  for (size_t i= 0; i < array_elements(all_shares) - 1; i++)
> +    result->add_table(all_shares[i]->m_name.str,
> +                      all_shares[i]->m_name.length);
> +  return 0;
> +}
> +
>  /** @} */
>  
> diff --git a/storage/perfschema/table_file_summary.cc b/storage/perfschema/table_file_summary.cc
> deleted file mode 100644
> index 104fa0f..0000000
> --- a/storage/perfschema/table_file_summary.cc
> +++ /dev/null
> @@ -1,372 +0,0 @@

Why did you remove this file?

> -/* Copyright (c) 2008, 2010, Oracle and/or its affiliates. All rights reserved.
> -
> -  This program is free software; you can redistribute it and/or modify
> -  it under the terms of the GNU General Public License as published by
> -  the Free Software Foundation; version 2 of the License.

Regards,
Sergei