Login | Register
My pages Projects Community openCollabNet

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Catacomb] DBMS abstraction

Thanks for your work.

I have several comments on that.

We need dbms.[ch] which is high level of DB interface which can deal all kind of DBMS.
Behind that, we can have dbms_mysql.[ch], dbms_oracle.[ch] and so on. So your work is really great.

In addition, we need to consider high level of data struct as well.

dbms.h can have that kind of handler:

typedef struct dav_repos_dbms_private dav_repos_dbms_private;

typedef struct dav_repos_dbms {
    const char *host;
    const char *user;
    dav_buffer *work_buf;      /* This is convient for make SQL */
    dav_repos_dbms_private *info;   /* Each DB private info */

    apr_pool_t *pool;

} dav_repos_dbms;

Also each dbms_xxx.c can have this info and the srructure must be hidden.

dbms_mysql.c :

/* context needed to identify a each DBMS */
struct dav_repos_dbms_private
    MYSQL *mysql

In that sense, do we really need dbms_mysql.h? The data is in dbms_mysql.c and the interface should be same as dbms_mysql.h and dbms_oracle.h. So I think the low level interface must be defined in dbms.h so that the other dbms_xxxx.c developer can implement the function.
Also we might need to consider *hooker* to support multiple DB.

> * is there a test suite I can use to make sure my changes don't break
> things?

http://ocean.cse.ucsc.edu/litmus. Also would you like to make a new branch?

> * I'm a bit rusty on the use of "const", and the dbms_mysql.c gives a
> warning regarding this...Dunno if that's really much of an issue or not.

> * The dbms struct carries it's own memory pool, should that be a child
> of another pool?  Given that each query will be generating a fair number
> of string allocations, should each query have it's own child pool? I'm
> unclear as to when memory pools are cleaned up, so guidance (or URL's)
> would be appreciated.

I think we should pass pool when we create DB. Since we open DB only when apache start, and close when apache stop, the pool can be larger.

> * coding style/variable naming comments are always appreciated, I can
> work with whatever style you like. (I often set my tab stops at 4, but
> I'll try to make sure it looks ok with 8.)

To avoid name conflict, please use prefix, "dav_repos_".

> I plan on being at the interop event Tuesday afternoon and Wednesday,
> hope to see you there!

Did you aware that there is mod_dav_psql. I think we might look at that one tool.
I'll be at the event only Monday and Tuesday morning. :-)

I appreciate your work again.

Sung Kim