[tarantool-patches] Re: [PATCH 2/4] sql: rework OP_Clear internals
Kirill Yukhin
kyukhin at tarantool.org
Tue Mar 20 13:54:53 MSK 2018
Hello,
My comments are inlined.
On 19 мар 21:10, Nikita Pettik wrote:
> This patch makes OP_Clear use pointer to space instead of space id.
> Moreover, in order to avoid excess function calls (such as box_delete() ->
> box_process1(), which in its turn makes additional space lookup),
> sql_execute_dml() function is introduced. It is an analogue of
> process_rw() from BOX internals. Its purpose is to handle transaction
> routine and call DML executor. This function will be also used later as
> well during insertion and deletion.
>
> Part of #3122
> ---
> src/box/sql.c | 54 +++++++++++++++++++++++++++++++---------------
> src/box/sql/opcodes.c | 2 +-
> src/box/sql/opcodes.h | 2 +-
> src/box/sql/tarantoolInt.h | 2 +-
> src/box/sql/vdbe.c | 28 ++++++++++--------------
> 5 files changed, 51 insertions(+), 37 deletions(-)
>
> diff --git a/src/box/sql.c b/src/box/sql.c
> index 256985793..4f8b39810 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -191,6 +191,27 @@ is_tarantool_error(int rc)
> rc == SQL_TARANTOOL_INSERT_FAIL);
> }
>
> +/**
> + * This function is analogue of process_rw() from box module.
> + * Apart of calling space_execute_dml(), it handles transaction
> + * routine.
> + */
Could you pls describe params as well?
> +static int
> +sql_execute_dml(struct request *request, struct space *space)
> +{
> + struct txn *txn = txn_begin_stmt(space);
> + if (txn == NULL)
> + return -1;
> + struct tuple *unused = NULL;
> + if (space_execute_dml(space, txn, request, &unused) != 0) {
> + txn_rollback_stmt();
> + return -1;
> + }
> + if (txn_commit_stmt(txn, request) != 0)
> + return -1;
> + return 0;
> +}
So, you've copied process_rw, removing statitics udpate and access
checks? I think, this won't work. The only concern I see here is that
process_rw returns a tuple, which is useless for SQL so far.
> @@ -670,30 +689,31 @@ int tarantoolSqlite3ClearTable(int iTable)
> }
> }
> - box_iterator_free(iter);
> - } else if (box_truncate(space_id) != 0) {
> + iterator_delete(iter);
> + } else if (box_truncate(space->def->id) != 0) {
I think we should block box_truncate sp far: too much complications this
DDL operation introduce. We'll re-implement it as optimization in furture.
Also, we'll support ANSI SQL's TRUNCATE TABLE.
--
Kirill
More information about the Tarantool-patches
mailing list