[Phpmyadmin-devel] GSOC 2011 Drizzle : remarks

Piotr Przybylski piotr.prz at gmail.com
Sun Jun 19 23:27:57 CEST 2011


2011/6/19 Dieter Adriaenssens <dieter.adriaenssens at gmail.com>:
> 2011/6/19 Piotr Przybylski <piotr.prz at gmail.com>:
>> 2011/6/19 Dieter Adriaenssens <dieter.adriaenssens at gmail.com>:
>>> Hi Piotr,
>>>
>>> I hope you are doing well. How did your finals go?
>>
>> I couldn't schedule them for the date I wanted, but at least I am
>> almost done with my master's thesis.
>>
>>>
>>> I have some questions, remarks and suggestion about your latest commits :
>>>
>>> * commit 2e01d01b322 : Change all calls of SHOW KEYS/INDEX that use
>>> MySQL-specific fields to use query generation function
>>>
>>> in db_datadict.php :
>>>
>>> -        $indexes_info[$row['Key_name']]['Non_unique']      =
>>> $row['Non_unique'];
>>> +        $indexes_info[$row['Key_name']]['Non_unique']      =
>>> PMA_DRIZZLE ? $row['Unique'] : $row['Non_unique'];
>>>
>>> Shouldn't $row['Unique'] be inverted?
>>
>> In a later commit I changed the query used here to mimic MySQL's SHOW
>> INDEXES, so I fixed it by using $row['Non_unique'] always.
>>
>>> * commit d607fa3e5c : Make live query chart work in Drizzle
>>>
>>> Please use CONCAT(), it makes the query better readable.
>>
>> Probably depends on what one is used to, for me "||" is a normal operator :)
>> Changed.
>>
>>> * commit a5342c70ea : Server Status: remove warnings, make process
>>> list and query stats work (Drizzle)
>>>
>>> 1) Same remark about CONCAT().
>>>
>>> 2)
>>> -        <td><?php echo $process['User']; ?></td>
>>> +        <td><?php echo PMA_DRIZZLE ? $process['Username'] :
>>> $process['User']; ?></td>
>>>
>>> Is there a way of changing 'Username' to 'User' where the process
>>> array is generated?
>>> I think it's better to have as little PMA_DRIZZLE code in the front
>>> end (i.e. html-generating) php files as possible, and try to catch the
>>> differences in the libraries instead. This makes the html-generating
>>> code better readable, and your project more easy to merge with master.
>>
>> Changed - now I use data_dictionary views to generate that
>> information. As a bonus, I got to report two Drizzle bugs.
>
> Nice!
>
>>> On a side note : I've been thinking about what you did for main.php.
>>> Could you create a lib-function for the servertype check?
>>
>> What should it return? 'MySQL', 'MariaDB' and 'Drizzle' strings?
>
> Yes, or is this one the places you will have a look at when reviewing the diffs?
>

This can be done now.

But I already found a good candidate for refactoring - PMA_isView,
which should be merged with PMA_Table::isView (which, in turn,
contains incorrect PHP code which fortunately is dead) :)

-- 
Piotr Przybylski




More information about the Developers mailing list