[vlc-devel] [PATCH] Prepared statements for SQL

[Srikanth Raju]

Hi,


> >      > +#define sql_GetIntegerColumn( A, B, C, D ) sql_GetColumn( A, B,
> C,
> >      SQL_INT, D )
> >      > +#define sql_GetDoubleColumn( A, B, C, D ) sql_GetColumn( A, B, C,
> >      SQL_DOUBLE, D )
> >      > +#define sql_GetTextColumn( A, B, C, D ) sql_GetColumn( A, B, C,
> >      SQL_TEXT, D )
> >      > +#define sql_GetBlobColumn( A, B, C, D ) sql_GetColumn( A, B, C,
> >      SQL_BLOB, D )
> >       I think more advanced wrapper would be helpful, they could
> completly
> >      hide sql_value_t.
> >
> >
> >    I'm not sure why one would want to hide the type of the value, because
> >    that was one of the reasons the prepared statement interface is being
> >    included. So that there may be stronger type checking.
>  That's why better wrappers will help.
>  Something like that for example:
> static inline int sql_GetColumnInteger( sql_t* p_sql, sql_stmt_t* p_stmt,
> int i_col, int *pi_res)
> {
>  sql_value_t v;
>  if(sql_GetColumn(..., &v))
>  return VLC_EGENERIC;
>  *pi_res = v.i_int;
>  return VLC_SUCCESS;
> }
>  It will make sure that you don't do a sql_GetColumnInteger(&v) and then
> uses
> v.dbl for example.
>

I did this. But it seems to make the include a lot heavier. Not too heavy
though.

> +    /** Bind parameters to a statement */
> > +    int (*pf_bind) ( sql_t* p_sql, sql_stmt_t* p_stmt, sql_type_e type,
> > +                             sql_value_t* p_value, int i_pos );
>  How is p_value used ? As an input or as a pointer where the output value
> will be stored ? (If as an input, a const is probably needed).
>
>
It's an input. const added.

> +
> > +    /** Get the datatype for a specified column */
> > +    int (*pf_gettype) ( sql_t* p_sql, sql_stmt_t* p_stmt, int i_col,
> > +                        sql_type_e* type );
> > +
> > +    /** Get the data from a specified column */
> > +    int (*pf_getcolumn) ( sql_t* p_sql, sql_stmt_t* p_stmt, int i_col,
> > +                          sql_type_e type, sql_value_t *p_res );
> >  };
>  As a general remark, I don't think the order of the arguments of
> pf_bind/pf_gettype/pf_getcolumn are consistant (or I missed the logic).
>
>
You're right. Fixed.

I've also made the other minor changes.


-- 
Regards,
Srikanth Raju
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20091106/628aa5f6/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Sql-Include-interface-for-prepared-statements.patch
Type: text/x-diff
Size: 11400 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20091106/628aa5f6/attachment.patch>