Chuck,
Thank you for the review. I've committed a new patch that includes your
comments unless otherwise noted in-line below.
Chuck Bell wrote:
>> === modified file 'mysql-test/suite/backup/t/backup_logs.test'
>> --- a/mysql-test/suite/backup/t/backup_logs.test 2009-02-06
>> 08:28:24 +0000
>> +++ b/mysql-test/suite/backup/t/backup_logs.test 2009-02-11
>> 09:45:55 +0000
>> @@ -9,6 +9,7 @@
>> --source include/not_embedded.inc
>> --source include/have_debug.inc
>> --source include/blackhole.inc
>> +--source include/have_falcon.inc
>
> [5 - commentary only] I really don't like having to add falcon to this
> test but I understand why it is needed. Compiling falcon on Windows
> poses some interesting problems. I may change this in BUG#42693 by
> making a special (more thorough) test for counting objects.
I removed the tablespace from backup_logs and created a new test file
backup_object_count.
>> +
>> +/// Returns number of routines in the image.
>> +inline
>> +uint Image_info::routine_count() const
>> +{ + return m_routine_count;
>> +}
>> +
>> /// Returns number of databases in the image.
>> inline
>> uint Image_info::db_count() const
>> @@ -737,11 +806,25 @@ uint Image_info::ts_count() const
>>
>> /// Returns total number of tables in the image.
>> inline
>> -ulong Image_info::table_count() const
>> +uint Image_info::table_count() const
>
> [6] Why did you change to uint from ulong? Will this cause issues or are
> you fixing issues?
This is a fix. The method returns the value of a private variable of
type ulong. I think the method signature should use the same number of
bits for the return value.
>
> ...
>
> [7] Was an official decision made as to what will and will not be
> counted? I don't recall seeing that decision and it is not documented in
> the bug report.
Added comment on bug
--
Jørgen Løland
