← Back to team overview

maria-developers team mailing list archive

Re: Regarding bugs (mysqldump)

 

Hi Rutuja!

Great analysis again and lets share this with the maria-developers mailing
list as well.

I've tested your proposed change and the change for dump_all_tables_in_db
seems to not be necessary. If you look at how dump_all_tables_in_db gets
called, it's followed by dump_all_views_in_db. The actual view creation
happens in dump_all_views_in_db, while dump_all_tables_in_db will create
some dummy tables for views. We create dummy tables first, so as to avoid
having to check dependencies within views. There's more details in the
comments of why we do that in the code of get_table_structure:

        /*
          Create a table with the same name as the view and with columns of
          the same name in order to satisfy views that depend on this view.
          The table will be removed when the actual view is created.

          The properties of each column, are not preserved in this temporary
          table, because they are not necessary.

          This will not be necessary once we can determine dependencies
          between views and can simply dump them in the appropriate order.
        */

For dump_selected_tables, your proposed change should be sufficient and
according to my tests it works. Please submit a pull request for this.
Looking forward to having this fixed soon. :)

Unrelated to your analysis:

There are plenty of edge cases where views won't work:
For example:
If we have 2 databases and one view uses something from a different
database, if you're not careful about the order of things, it's going to
fail. To create a "truly" always working dump one would have to create a
full dependency graph and then do a topological sort for all objects in the
database that depend on the objects we're dumping. This is outside of the
scope of this bug, however it could prove to be a "better way to dump
stuff".

Regards,
Vicentiu

On Sun, 1 Apr 2018 at 06:28 Rutuja Surve <rutuja.r.surve@xxxxxxxxx> wrote:

> Hello,
> Could you review my following approach regarding the mysqldump bug (
> https://jira.mariadb.org/browse/MDEV-15021):
>
> I went through the code and observed the following:
> 1. dump_all_tables_in_db calls dump_table first (for the list of all
> tables), then dump_table calls get_view_structure.
> 2. get_view_structure dumps the view for the corresponding table to
> mysqldump
> 3. After this call, dump_all_tables_in_db calls dump_routines_for_db
>
> dump_selected_tables also behaves in the exact same way as dump_all_tables_in_db,
> following steps 1,2,3, except that it does it for a list of given tables
> instead of doing it for all tables.
>
> We want the routines to get dumped before the views for the dump to become
> importable.
> There are two solutions for this:
> 1. Make dump_all_tables_in_db call dump_routines_for_db before it calls
> dump_table. This will dump routines first and views later.
> This can be done by shifting the call to dump_routines_for_db before the
> loop in which dump_table is called. I.E:
> Shift :
> if (opt_routines && mysql_get_server_version(mysql) >= 50009)
>   {
>     DBUG_PRINT("info", ("Dumping routines for database %s", database));
>     *dump_routines_for_db*(database);
>   }
> Before this loop:
>   while ((table= getTableName(0)))
>   {
>     char *end= strmov(afterdot, table);
>     if (include_table((uchar*) hash_key, end - hash_key))
>     {
>       *dump_table*(table,database);
>       my_free(order_by);
>       ....
>     }
>   }
>
> We should do this for both dump_all_tables_in_db and dump_selected_tables.
>
> 2. Approach 2:
> As suggested in the jira thread, we can use negative logic to call
> dump_table on tables we intend to skip.
> This appears to be a workaround. In the place where dump_all_tables_in_db
> is called, we'd rather call dump_selected_tables
> with a list of tables that don't use the routines we'll be listing after
> that. This would fix the issue but is not a good approach.
>
> If dump_table doesn't alter the values for opt_routines, the first
> approach should fix the issue.
>
> Thanks,
> Rutuja
>
>
>
> On Fri, Mar 23, 2018 at 10:36 PM, Rutuja Surve <rutuja.r.surve@xxxxxxxxx>
> wrote:
>
>> Hello,
>> With respect to https://jira.mariadb.org/browse/MDEV-15021 :
>> From what I understood from the thread, since the routines are placed
>> below the tables/views, they cannot be used in views while importing.
>> I went through the problematic_dump.sql file that's attached there. What
>> does the '@' in the following mean:
>> SET @saved_cs_client     = @@character_set_client;
>> (I'm not familiar with this syntax)
>> Also, what does the following piece of code do: (what does create definer
>> mean and does this function simply return 1 as there are no other
>> statements after begin? This is a routine defined below test_view)
>> CREATE DEFINER=`root`@`localhost` FUNCTION `test_function`() RETURNS
>> int(11)
>> BEGIN
>>
>> RETURN 1;
>> END ;;
>> DELIMITER ;
>> From my understanding, test_view uses test_function and test_view2 would
>> use test_function2.
>> In the mysqldump.c file, I went through the dump_routines_for_db
>> function. We'd require to modify that in order to fix the bug.
>>
>> From the beginner bugs list, I found these 2 bugs that are also related
>> to mysql dump that I could take up:
>> 1. https://jira.mariadb.org/browse/MDEV-9883
>> (Flag for letting mysqldump support the 'auto' option for character set).
>> Where in the code base can I find the file where the ' --default-character-set'
>> flag is defined?)
>> 2. https://jira.mariadb.org/browse/MDEV-8765
>> The solution for this has already been proposed in the thread. For
>> mysqldump to not corrupt 4 byte UTF8 data, "change
>> include/my_global.h:#define MYSQL_UNIVERSAL_CLIENT_CHARSET to utf8mb4"
>>
>> Thanks,
>> Rutuja
>>
>
>