List:Summer of Code« Previous MessageNext Message »
From:scut_tang Date:July 10 2009 3:42am
Subject:Re: GSoC I_S/P_S storage engine midterm report
View as plain text  
Hi, Sergei. Thanks you very much for your midterm review.

>>3.1 add support for I_S plugins.
>>3.2 make sure all SELECT queries work - for example, without position
 >>   and rnd_pos you cannot support filesort (ORDER BY), so there should
 >>   be some workaround for this case.
What does "add support for I_S plugins" mean?
I knew position and rnd_pos are for non-sequential reads, and position stores the current
position of data file or something else about current record, and rnd_pos reads record
according by pos stored by position. But how do they work for non-sequential reads? I
want to know more them in depth.
Do they connect record with its position and sort the records in MySQL server?

>>select table_schema, table_name from information_schema.tables does not show
>>your tables - as expected, because you didn't implement hton->find_files()
> method,
>>see below.
I have finished infoschema_find_files now, and it works.
"SHOW DATABASES" can list INFOSCHEMA.
But "use INFOSHEMA" does not work, and what functions deal with "use database-name"?

>>SHOW CREATE TABLE INFOSCHEMA.TABLES crashes. not good.
I added "DBUG_RETURN(HA_ERR_COMMAND)" in ha_infoschema::create.
It won't crash.

>>handler.h/handler.cc changes look really good!
>>but here, please move this code to a separate function,
>>in handler.cc, something like binary_frm_to_table_share()
>>or any other name that you think is better.
>>and here simply call that function.
Done.

>>Did you try both static and dynamic builds of your plugin ?
>>I mean, you declare here that your infoschema can be statically compiled
>>into the server or build as a shared library and loaded dynamically.
>>Are you sure that both work ? If you declare both ways, you
>>need to know that they both work. Otherwise, just keep the static one
>>and remove MYSQL_PLUGIN_DYNAMIC line if dynamic loading doesn't work.
>>I don't think that it makes sense to make infoschema dynamically loaded anyway,
>>so feel free to remove MYSQL_PLUGIN_DYNAMIC line.
I am sorry. I have only tried static builds of the plugin.
Why "I don't think that it makes sense to make infoschema dynamically loaded anyway"?

>>> +    private:
>>> +        /** MySQL lock */
>>> +        THR_LOCK_DATA m_thr_lock;
>>> +        /** INFORMATION_SCHEMA table share for this handler. */
>>> +        const infos_table_share *m_table_share;

>>you don't need it here, store it in TABLE_SHARE::ha_data
Why? For saving memory? I think it is just a pointer and it looks more clearly.

>>okay. why did you decide to have a separate hierarchy for the I_S tables,
>>deriving them from infos_table instead of using the handler hierarchy and
>>deriving them from ha_infoschema ?
I have changed the hierarchy a little.
I will send the code diff file to you in this weekly report.

>>are you sure you need #define MYSQL_SERVER here ?
I really do not understand "#define MYSQL_SERVER". Can you explain it? 
I saw almost every storage engine contain it, so I do this.

>>first: I think that - as we have quite a few I_S tables already
>>you need to use a hash here, not a linear search in an array.
>>(check HASH, include/hash.h, mysys/hash.c).
>>second: this is for later, not in the very first version, but as this I_S
>>engine will have to support I_S plugins this array/hash will be modified
>>run-time and we will need some kind of locking here.
Got it. I_S has lots of tables. HASH can get better performance.

>>I wonder, why did you create an intermediate field_list of Items instead of
>>iterating schema_table->m_fields here directly.
I just followed the old implementation. I will try it.

I am sorry for copied so much. My mind is it starts from a skeleton, and remove useless
functions in the development.
OK. I will change it to a minimal framework.

Regards,
Robin







Thread
GSoC I_S/P_S storage engine midterm reportscut_tang5 Jul