drizzle-discuss team mailing list archive
Mailing list archive
Re: file system storage engine
On Fri, 18 Jun 2010 13:45:54 +0800, ZQ <ziminq@xxxxxxxxx> wrote:
> could you please do some review over my branch? It would be helpful.
Sorry this has taken a little while to get back to you, I've been in
training all week. Hope you were still able to get some things done.
Cool! I've spotted a few things for you to take a look at:
- FilesystemCursor::close should check the return value of close(),
especially for EINTR.
- on non-EINTR errors, you can probably just return them and kind of
hope that the upper layer reports these correctly. Since with buffered
IO close(2) can also return all the errors that write() can, things
like EIO may occur here.
- avoid using things in the drizzled::internal namespace. Instead you
can just cast ref to an off_t and store it in there.
- on startup you should remove the .UPDATE file if it exists
- getAllFields() is a bit misleadingly named. Perhaps recordToString() ?
- is storing '0' for NULL a good idea? Perhaps another engine
option. defaulting to 'NULL' is probably better too.
- you should check the return code of write(2) lots of interesting
things can go wrong.
- instead of validating access to the file in
validateCreateTableOption (which can only return 'success' or
'failure'), check this in ::doCreateTable which can return a real
error message (e.g. ENOENT, EPERM, EIO etc)
- the mutex is destroyed in the destructor, it should be created in
- use an ENUM for share->separator mode instead of number.
I hope to have a closer look at the update/deletion code shortly.
hopefully this helps. things are looking pretty good. One idea may be to
go through the 'load data infile' tests, copy them and rewrite parts to
use the filesystem engine instead.