(CC'd drizzle-discuss in case anybody else has some thoughts too)
On Mon, 07 Jun 2010 18:58:56 +0800, Zimin <[email protected]> wrote:
> lp:~ziminq/drizzle/filesystem-storage-engine
A few things:
- it looks like you haven't merged from trunk in a while, you should try
to keep rather up to date with trunk.
- in ha_filesystem::open
- the table message is already loaded by the upper layer, so you can
access it through table->share (see table_share.h)
- It looks as though you're not trying to open the actual file there?
- you should probably try to open the file there and report back
errors
- in doStartTableScan
- instead of using cerr for errors, use something like
push_warning_printf:
(for example, from embedded_innodb):
if (innodb_err != DB_SUCCESS)
{
push_warning_printf(&session, DRIZZLE_ERROR::WARN_LEVEL_ERROR,
ER_CANT_CREATE_TABLE,
_("Cannot create table %s. InnoDB Error %d (%s)\n"),
innodb_table_name.c_str(), innodb_err,
ib_strerror(innodb_err));
return HA_ERR_GENERIC;
}
- in rnd_next()
- Don't use drizzled::String, instead use std::string.
- have so you have a row separator as well as a column separator
e.g. a CSV file has ROW_SEPARATOR='\n', COLUMN_SEPARATOR=','
- you should set each field not null using Field instead of using
memset.
- position() and rnd_pos()
- you will need to implement these (check out my rnd_pos test for
embedded_innodb).
- they should be pretty easy. store the position in the file, seek to
it.
- you will need to set ref_length in ::open (see embedded_innodb for example)
- info()
- you'll need some things here, but shouldn't be hard.
- locking
- you probably want to write out the locking strategy.
- possible have options? fcntl locks?
- you have a table share (that isn't used), so the locking doesn't
work :)
- keep in mind that Drizzle is very much about removing horrible table
level locks, so this is the responsibility of the engine.
- insert/update row
- you need to operate on the buffer you're given. this means using
move_field_offset and that awful API (i've written a blog post on
this recently)
> I would like to know whether such approach is OK or not.
> and I use iostream file operation, is it ok? I know that getline() is
> slow on big files.
In theory it should be okay - but what about the case where rows are not
delimited by newlines?
> to delete and update a line, I used a temp file and rename the temp file
> to the original one, it's kind of ugly.
I think it'd be great if you also:
- added more blueprints for each feature
- targeted these blueprints for a series (Dexter)
- targeted these blueprints for milestones (i.e. dates where you aim to
have code with that functionality merged into the main tree)
I do really like that there are tests there too!
great work so far, hopefully the above helps!
--
Stewart Smith
_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to : [email protected]
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help : https://help.launchpad.net/ListHelp
