Re: [Tarantool-patches] [PATCH v3 01/20] sql: remove sql_prepare_v2()

[Sergey Ostanevich <sergos@tarantool.org>]

Hi!

Thanks for the patch!

On 20 Dec 15:47, Nikita Pettik wrote:
> There are two versions of the same function (sql_prepare()) which are
> almost identical. Let's keep more relevant version sql_prepare_v2() but
> rename it to sql_prepare() in order to avoid any mess.
> 
> Needed for #3292
> ---
>  src/box/execute.c     |  2 +-
>  src/box/sql/legacy.c  |  2 +-
>  src/box/sql/prepare.c | 32 ++++----------------------------
>  src/box/sql/sqlInt.h  | 25 +++++++++++--------------
>  src/box/sql/vdbeapi.c |  2 +-
>  5 files changed, 18 insertions(+), 45 deletions(-)
> 
> diff --git a/src/box/execute.c b/src/box/execute.c
> index e8b012e5b..130a3f675 100644
> --- a/src/box/execute.c
> +++ b/src/box/execute.c
> @@ -443,7 +443,7 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
>  {
>  	struct sql_stmt *stmt;
>  	struct sql *db = sql_get();
> -	if (sql_prepare_v2(db, sql, len, &stmt, NULL) != 0)
> +	if (sql_prepare(db, sql, len, &stmt, NULL) != 0)
>  		return -1;
>  	assert(stmt != NULL);
>  	port_sql_create(port, stmt);
> diff --git a/src/box/sql/legacy.c b/src/box/sql/legacy.c
> index 0b1370f4a..bfd1e32b9 100644
> --- a/src/box/sql/legacy.c
> +++ b/src/box/sql/legacy.c
> @@ -70,7 +70,7 @@ sql_exec(sql * db,	/* The database on which the SQL executes */
>  		char **azVals = 0;
>  
>  		pStmt = 0;
> -		rc = sql_prepare_v2(db, zSql, -1, &pStmt, &zLeftover);
> +		rc = sql_prepare(db, zSql, -1, &pStmt, &zLeftover);
>  		assert(rc == 0 || pStmt == NULL);
>  		if (rc != 0)
>  			continue;
> diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
> index 0ecc676e2..35e81212d 100644
> --- a/src/box/sql/prepare.c
> +++ b/src/box/sql/prepare.c
> @@ -204,36 +204,12 @@ sqlReprepare(Vdbe * p)
>  	return 0;
>  }
>  
> -/*
> - * Two versions of the official API.  Legacy and new use.  In the legacy
> - * version, the original SQL text is not saved in the prepared statement
> - * and so if a schema change occurs, an error is returned by
> - * sql_step().  In the new version, the original SQL text is retained
> - * and the statement is automatically recompiled if an schema change
> - * occurs.
> - */
> -int
> -sql_prepare(sql * db,		/* Database handle. */
> -		const char *zSql,	/* UTF-8 encoded SQL statement. */
> -		int nBytes,		/* Length of zSql in bytes. */
> -		sql_stmt ** ppStmt,	/* OUT: A pointer to the prepared statement */
> -		const char **pzTail)	/* OUT: End of parsed string */
> -{
> -	int rc = sqlPrepare(db, zSql, nBytes, 0, 0, ppStmt, pzTail);
> -	assert(rc == 0 || ppStmt == NULL || *ppStmt == NULL);	/* VERIFY: F13021 */
> -	return rc;
> -}
> -
>  int
> -sql_prepare_v2(sql * db,	/* Database handle. */
> -		   const char *zSql,	/* UTF-8 encoded SQL statement. */
> -		   int nBytes,	/* Length of zSql in bytes. */
> -		   sql_stmt ** ppStmt,	/* OUT: A pointer to the prepared statement */
> -		   const char **pzTail	/* OUT: End of parsed string */
> -    )
> +sql_prepare(struct sql *db, const char *sql, int length, struct sql_stmt **stmt,
> +	    const char **sql_tail)
>  {
> -	int rc = sqlPrepare(db, zSql, nBytes, 1, 0, ppStmt, pzTail);
> -	assert(rc == 0 || ppStmt == NULL || *ppStmt == NULL);	/* VERIFY: F13021 */
> +	int rc = sqlPrepare(db, sql, length, 1, 0, stmt, sql_tail);
> +	assert(rc == 0 || stmt == NULL || *stmt == NULL);
>  	return rc;
>  }
>  
> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
> index 2594b73e0..7bd952a17 100644
> --- a/src/box/sql/sqlInt.h
> +++ b/src/box/sql/sqlInt.h
> @@ -468,21 +468,18 @@ typedef void (*sql_destructor_type) (void *);
>  #define SQL_STATIC      ((sql_destructor_type)0)
>  #define SQL_TRANSIENT   ((sql_destructor_type)-1)
>  
> +/**
> + * Prepare (compile into VDBE byte-code) statement.

Could you please extend the description with details on SQL text
preservance and recmopilation, same as it was in old version?

> + *
> + * @param db Database handle.
> + * @param sql UTF-8 encoded SQL statement.
> + * @param length Length of @param sql in bytes.
> + * @param[out] stmt A pointer to the prepared statement.
> + * @param[out] sql_tail End of parsed string.
> + */
>  int
> -sql_prepare(sql * db,	/* Database handle */
> -		const char *zSql,	/* SQL statement, UTF-8 encoded */
> -		int nByte,	/* Maximum length of zSql in bytes. */
> -		sql_stmt ** ppStmt,	/* OUT: Statement handle */
> -		const char **pzTail	/* OUT: Pointer to unused portion of zSql */
> -	);
> -
> -int
> -sql_prepare_v2(sql * db,	/* Database handle */
> -		   const char *zSql,	/* SQL statement, UTF-8 encoded */
> -		   int nByte,	/* Maximum length of zSql in bytes. */
> -		   sql_stmt ** ppStmt,	/* OUT: Statement handle */
> -		   const char **pzTail	/* OUT: Pointer to unused portion of zSql */
> -	);
> +sql_prepare(struct sql *db, const char *sql, int length, struct sql_stmt **stmt,
> +	    const char **sql_tail);
>  
>  int
>  sql_step(sql_stmt *);
> diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
> index 685212d91..12449d3bc 100644
> --- a/src/box/sql/vdbeapi.c
> +++ b/src/box/sql/vdbeapi.c
> @@ -452,7 +452,7 @@ sqlStep(Vdbe * p)
>  		checkProfileCallback(db, p);
>  
>  	if (p->isPrepareV2 && rc != SQL_ROW && rc != SQL_DONE) {
> -		/* If this statement was prepared using sql_prepare_v2(), and an
> +		/* If this statement was prepared using sql_prepare(), and an
>  		 * error has occurred, then return an error.
>  		 */
>  		if (p->is_aborted)
> -- 
> 2.15.1
>