[vlc-devel] [PATCH] Sqlite Module for VLC, GSoC 09

[Laurent Aimar]

Hi,

On Sat, Aug 29, 2009, Srikanth Raju wrote:
> >  Is so many functions really needed ?
> >
> >  IMHO, merging pf_set_database and pf_open_database in the module Open
> > and pf_close_database in the module Close function would simplify it
> > without any loss. It would also prevent stupid usage mistakes.
> >
> 
> Perhaps people don't want to create a connection until required?
 You can then load the sql module when you want the connection and
destroy it when you don't want it anymore.

> >> +    /** Perform a query with a row-by-row callback function */
> >> +    int (*pf_query_callback) ( sql_t *, const char *,
> >> +            int (*callback) ( void*, int, char**, char** ), void * );
> >> +
> >> +    /** Perform a query and return result directly */
> >> +    int (*pf_query) ( sql_t *, const char *, char ***, int *, int * );
> >> +
> >> +    /** Get database tables */
> >> +    int (*pf_get_tables) ( sql_t *, char *** );
> >> +
> >
> >> +    /** Free result of a call to sql_Query or sql_GetTables */
> >> +    void (*pf_free) ( sql_t *, char ** );
> >  I am not sure this one is really needed. Couldn't it be made independant
> > of the module implementation ?
> >
> 
> For the sqlite module, this uses a specific sqlite3_free_table.
> I'm not sure if it's safe to free using C, neither does the documentation
> explicitly allow it.
 Ok :(

> >> +    /** Begin transaction */
> >> +    int (*pf_begintransaction) ( sql_t* );
> >> +
> >> +    /** Commit transaction */
> >> +    void (*pf_committransaction) ( sql_t* );
> >> +
> >> +    /** Rollback transaction */
> >> +    void (*pf_rollbacktransaction) ( sql_t* );
> >  Is the separation into 4 functions really needed ? And if so why ?
> >

> The other three are for transactions. We could have a single function
> with a parameter for which transactional procedure to call. But is that really
> easier? It's like having "vlc_mutex_proc(Lock) and vlc_mutex_proc(Unlock)"
 It depends, if the only valid way is:
 begin(); query(); if(commit()) rollback(); end(); then a unique query() doing 
all that is to be prefered.

-- 
fenrir