[patches] [PATCH 1/4] sql: Return last updated tuple if requested

Konstantin Osipov kostja at tarantool.org
Tue Mar 20 21:09:00 MSK 2018 

* Vladislav Shpilevoy <v.shpilevoy at tarantool.org> [18/03/14 01:14]:
> 
>     If requested by IPROTO protocol , return last updated tuple.
>     Done via extra field in VDBE engine, called sql_options.
>     This new field should in future adsorb most of the sqlite3
>     internals, making it connection-local.
>     Right now this structure contains single field: pointer
>     to pointer to last updated tuple.
> 
>     If non-NULL pointer is passed, tuple_unref() must be called
>     upon end of handling.
> 
>     Part of #2618
> 
> Signed-off-by: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
> ---
>  src/box/execute.c           | 29 ++++++++++++++++++++++-------
>  src/box/execute.h           |  2 +-
>  src/box/iproto.cc           |  4 +++-
>  src/box/lua/sql.c           |  9 ++++++---
>  src/box/sql.c               | 25 ++++++++++++++++---------
>  src/box/sql.h               | 22 ++++++++++++++++++----
>  src/box/sql/analyze.c       |  4 ++--
>  src/box/sql/legacy.c        |  2 +-
>  src/box/sql/sqlite3.h       | 31 ++++++++++++++++++++++++-------
>  src/box/sql/tarantoolInt.h  |  4 ++--
>  src/box/sql/vdbe.c          | 12 ++++++++----
>  src/box/sql/vdbeInt.h       |  5 +++++
>  src/box/sql/vdbeapi.c       |  6 ++++--
>  src/box/sql/vdbeaux.c       | 44 +++++++++++++++++++++++++++-----------------
>  test/unit/sql-bitvec.result | 39 ---------------------------------------
>  15 files changed, 139 insertions(+), 99 deletions(-)
>  delete mode 100644 test/unit/sql-bitvec.result
> 
> diff --git a/src/box/execute.c b/src/box/execute.c
> index 48d166b5b..756db5fd6 100644
> --- a/src/box/execute.c
> +++ b/src/box/execute.c
> @@ -548,12 +548,12 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,
>  
>  static inline int
>  sql_execute(sqlite3 *db, struct sqlite3_stmt *stmt, int column_count,
> -	    struct port *port, struct region *region)
> +	    struct port *port, struct region *region, struct sql_options *opts)

I think sql_options *opts should not be separable from
sqlite3_stmt. Ideally we should not have any options at all, but
either create modified versions of OP_{Insert|Replace} or
add options/extra parameters to existing OP_{Insert|Replace}.

This can be done at prepare() step, not at execute().

In future, some of the user options will affect prepare, some will
affect execute. So I think you don't need to create a separate
struct sql_options, you already pass the option around in
sql_request.

The tuple itself should be returned from sql_step(), not as "out"
parameter of options.


> +		rc = sqlite3_step(stmt, opts);

You should not need to pass options into *every* step, most steps
will not ever need it.

> @@ -586,13 +587,14 @@ sql_execute(sqlite3 *db, struct sqlite3_stmt *stmt, int column_count,
>   */
>  static inline int
>  sql_execute_and_encode(sqlite3 *db, struct sqlite3_stmt *stmt, struct obuf *out,
> -		       uint64_t sync, struct region *region)
> +		       uint64_t sync, struct region *region,
> +		       struct sql_options *opts)
>  {
>  	struct port port;
>  	struct port_tuple *port_tuple = (struct port_tuple *)&port;
>  	port_tuple_create(&port);
>  	int column_count = sqlite3_column_count(stmt);
> -	if (sql_execute(db, stmt, column_count, &port, region) != 0)
> +	if (sql_execute(db, stmt, column_count, &port, region, opts) != 0)
>  		goto err_execute;
>  
>  	/*
> @@ -647,8 +649,11 @@ err_execute:
>  
>  int
>  sql_prepare_and_execute(const struct sql_request *request, struct obuf *out,
> -			struct region *region)
> +			struct region *region, bool is_last_tuple_needed)
>  {
> +	struct sql_options opts;
> +	struct tuple *last_inserted_tuple = NULL;
> +
>  	const char *sql = request->sql_text;
>  	uint32_t len;
>  	sql = mp_decode_str(&sql, &len);
> @@ -665,10 +670,20 @@ sql_prepare_and_execute(const struct sql_request *request, struct obuf *out,
>  	assert(stmt != NULL);
>  	if (sql_bind(request, stmt) != 0)
>  		goto err_stmt;
> +
> +	if (is_last_tuple_needed)
> +		sql_options_create(&opts, &last_inserted_tuple);
> +	else
> +		sql_options_create(&opts, NULL);
> +
>  	if (sql_execute_and_encode(db, stmt, out, request->sync,
> -				   region) != 0)
> +				   region, &opts) != 0)
>  		goto err_stmt;
>  	sqlite3_finalize(stmt);
> +
> +	if (opts.last_tuple != NULL  && *opts.last_tuple != NULL)
> +		tuple_unref(*opts.last_tuple);
> +
>  	return 0;
>  err_stmt:
>  	sqlite3_finalize(stmt);
> diff --git a/src/box/execute.h b/src/box/execute.h
> index 76fdbf5f5..0aef26702 100644
> --- a/src/box/execute.h
> +++ b/src/box/execute.h
> @@ -105,7 +105,7 @@ xrow_decode_sql(const struct xrow_header *row, struct sql_request *request,
>   */
>  int
>  sql_prepare_and_execute(const struct sql_request *request, struct obuf *out,
> -			struct region *region);
> +			struct region *region, bool is_last_tuple_needed);
>  
>  #if defined(__cplusplus)
>  } /* extern "C" { */
> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
> index f7784ef23..9ace0d082 100644
> --- a/src/box/iproto.cc
> +++ b/src/box/iproto.cc
> @@ -1382,13 +1382,15 @@ tx_process_sql(struct cmsg *m)
>  {
>  	struct iproto_msg *msg = tx_accept_msg(m);
>  	struct obuf *out = msg->connection->tx.p_obuf;
> +	bool is_last_tuple_needed = true;
>  
>  	tx_fiber_init(msg->connection->session, msg->header.sync);
>  
>  	if (tx_check_schema(msg->header.schema_version))
>  		goto error;
>  	assert(msg->header.type == IPROTO_EXECUTE);
> -	if (sql_prepare_and_execute(&msg->sql, out, &fiber()->gc) != 0)
> +	if (sql_prepare_and_execute(&msg->sql, out, &fiber()->gc,
> +				    is_last_tuple_needed) != 0)
>  		goto error;
>  	iproto_wpos_create(&msg->wpos, out);
>  	return;
> diff --git a/src/box/lua/sql.c b/src/box/lua/sql.c
> index 16bf2acdb..e45122c9f 100644
> --- a/src/box/lua/sql.c
> +++ b/src/box/lua/sql.c
> @@ -225,10 +225,12 @@ lua_sql_execute(struct lua_State *L)
>  			l->stmt_count --;
>  			break;
>  		}
> -
> +		struct sql_options opts;
> +		sql_options_create(&opts, NULL);
>  		int column_count = sqlite3_column_count(ps->stmt);
>  		if (column_count == 0) {
> -			while ((rc = sqlite3_step(ps->stmt)) == SQLITE_ROW) { ; }
> +			while ((rc = sqlite3_step(ps->stmt,
> +						  &opts)) == SQLITE_ROW);
>  		} else {
>  			char *typestr;
>  			l->column_count = column_count;
> @@ -252,7 +254,8 @@ lua_sql_execute(struct lua_State *L)
>  			lua_rawseti(L, -2, 0);
>  
>  			int row_count = 0;
> -			while ((rc = sqlite3_step(ps->stmt)) == SQLITE_ROW) {
> +			while ((rc = sqlite3_step(ps->stmt,
> +						  &opts)) == SQLITE_ROW) {
>  				lua_push_row(L, l);
>  				row_count++;
>  				lua_rawseti(L, -2, row_count);
> diff --git a/src/box/sql.c b/src/box/sql.c
> index e32ff2383..73fd13996 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -519,7 +519,8 @@ int tarantoolSqlite3EphemeralDrop(BtCursor *pCur)
>  	return SQLITE_OK;
>  }
>  
> -static int insertOrReplace(BtCursor *pCur, int operationType)
> +static int
> +insertOrReplace(BtCursor *pCur, int operationType, struct tuple **tuple)
>  {
>  	assert(pCur->curFlags & BTCF_TaCursor);
>  	assert(operationType == TARANTOOL_INDEX_INSERT ||
> @@ -527,27 +528,33 @@ static int insertOrReplace(BtCursor *pCur, int operationType)
>  
>  	int space_id = SQLITE_PAGENO_TO_SPACEID(pCur->pgnoRoot);
>  	int rc;
> +	if (tuple != NULL && *tuple != NULL) {
> +		tuple_unref(*tuple);
> +		*tuple = NULL;
> +	}

When do you ever need to dereference the previous tuple? If you
simply emit it from sqlite3_step(), you won't have to worry about
dereference.

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.org - www.twitter.com/kostja_osipov

More information about the Tarantool-patches mailing list