[vlc-devel] [PATCH] Prepared statements for SQL
Laurent Aimar
fenrir at via.ecp.fr
Tue Oct 20 19:18:03 CEST 2009
Hi,
> +#define VLC_SQL_ROW 1
> +#define VLC_SQL_DONE 2
What are they used for? A doxy comment would be usefull.
> +typedef enum {
> + SQL_INT,
> + SQL_DOUBLE,
> + SQL_TEXT,
> + SQL_BLOB,
> + SQL_NULL
> +} sql_type_e;
sql_type_t for the typedef.
What are BLOB and NULL?
> +struct sql_value_t
> +{
> + int length;
> + union
> + {
> + int i;
> + double dbl;
> + char* psz;
> + void* ptr;
> + } value;
> +};
You could merge it with:
> +typedef struct sql_value_t sql_value_t;
> + /** Create a statement object */
> + sql_stmt_t* (*pf_prepare) ( sql_t* p_sql, const char* psz_fmt,
> + int i_length );
> +
> + /** 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 );
> +
> + /** Run the prepared statement */
> + int (*pf_run) ( sql_t* p_sql, sql_stmt_t* p_stmt );
> +
> + /** Reset the prepared statement */
> + int (*pf_reset) ( sql_t* p_sql, sql_stmt_t* p_stmt );
> +
> + /** Destroy the statement object */
> + int (*pf_finalize) ( sql_t* p_sql, sql_stmt_t* p_stmt );
> +
> + /** 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 );
I am not sure how to improve this, but it begins to make sql_t having an awful lot
of functions, and also to be hard to know which ones work together.
Anyone has an idea?
> +struct sql_stmt_t
> +{
> + sql_t* p_sql;
> + sql_stmt_sys_t *p_sys;
> +};
I don't think that explicitly declaring sql_stmt_t is needed.
It seems that is only needed by the plugin implementation.
> +/**
> + * @brief Prepare an sql statement
> + * @param p_sql The SQL object
> + * @param psz_fmt SQL query string
> + * @param i_length length of the string
> + * @return a sql_stmt_t pointer or NULL on failure
> + */
> +static inline sql_stmt_t* sql_Prepare( sql_t* p_sql, const char* psz_fmt,
> + int i_length )
> +{
> + return p_sql->pf_prepare( p_sql, psz_fmt, i_length );
> +}
I don't understand what i_length is. If it is strlen(psz_fmt), then it is
useless, otherwise a more explicit documentation is needed.
> +/**
> + * @brief Bind arguments to a sql_stmt_t object
> + * @param p_sql The SQL object
> + * @param p_stmt Statement Object
> + * @param type Data type of the value
> + * @param p_value Value to be bound
> + * @param i_length For values which require a length
> + * @param i_pos Position at which the parameter should be bound
> + * @return VLC_SUCCESS or VLC_EGENERIC
> + */
> +static inline int sql_BindGeneric( sql_t* p_sql, sql_stmt_t* p_stmt,
> + sql_type_e type, sql_value_t* p_value, int i_pos )
I think using int instead of sql_type_e is better for binary compatibility.
Is p_value modified by the function? (If not, a const is needed).
I think more advanced wrapper would be helpful, they could completly
hide sql_value_t.
> +/**
> + * @brief Reset the SQL statement
> + * @param p_sql The SQL object
> + * @param p_stmt The sql statement object
> + * @return VLC_SUCCESS or VLC_EGENERIC
> + */
> +static inline int sql_Reset( sql_t* p_sql, sql_stmt_t* p_stmt )
> +{
> + return p_sql->pf_reset( p_sql, p_stmt );
> +}
What exactly does this function? Is it the same than finalize+prepare?
> +/**
> + * @brief Get the datatype of the result of the column
> + * @param p_sql The SQL object
> + * @param p_stmt The sql statement object
> + * @param i_col The column
> + * @param type pointer to datatype of the given column
> + * @return VLC_SUCCESS or VLC_EGENERIC
> + */
> +static inline int sql_GetColumnType( sql_t* p_sql, sql_stmt_t* p_stmt,
> + int i_col, sql_type_e* type )
> +{
> + return p_sql->pf_gettype( p_sql, p_stmt, i_col, type );
> +}
> +
> +/**
> + * @brief Get the column data
> + * @param p_sql The SQL object
> + * @param p_stmt The statement object
> + * @param i_col The column number
> + * @param type Datatype of result
> + * @param p_res The structure which contains the value of the result
> + * @return VLC_SUCCESS or VLC_EGENERIC
> + */
> +static inline int sql_GetColumn( sql_t* p_sql, sql_stmt_t* p_stmt,
> + int i_col, sql_type_e type, sql_value_t *p_res )
> +{
> + return p_sql->pf_getcolumn( p_sql, p_stmt, i_col, type, p_res );
> +}
Why is type needed? (Especially when sql_GetColumnType seems to be able
to retreive it).
> +#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.
--
fenrir
More information about the vlc-devel
mailing list