[patches] [PATCH 1/3] sql: unify SQL cursor interface
Alex Khatskevich
avkhatskevich at tarantool.org
Mon Feb 12 18:11:37 MSK 2018
Minor fixes needed,
then ok.
On 12.02.2018 17:06, Nikita Pettik wrote:
> This patch merge functions for cursor manipulating of ordinary and
> ephemeral spaces in order to avoid code duplication and excess checks of
> cursor types.
> ---
> src/box/sql.c | 214 +++++++++++----------------------------------
> src/box/sql/cursor.c | 11 +--
> src/box/sql/tarantoolInt.h | 4 -
> src/box/sql/vdbe.c | 16 +---
> 4 files changed, 54 insertions(+), 191 deletions(-)
>
> diff --git a/src/box/sql.c b/src/box/sql.c
> index ef73d543a..682f15753 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -175,13 +175,6 @@ cursor_seek(BtCursor *pCur, int *pRes, enum iterator_type type,
> static int
> cursor_advance(BtCursor *pCur, int *pRes);
>
> -static int
> -cursor_ephemeral_seek(BtCursor *pCur, int *pRes, enum iterator_type type,
> - const char *key, const char *key_end);
> -
> -static int
> -cursor_ephemeral_advance(BtCursor *pCur, int *pRes);
> -
> const char *tarantoolErrorMessage()
> {
> return box_error_message(box_error_last());
> @@ -197,9 +190,11 @@ int tarantoolSqlite3CloseCursor(BtCursor *pCur)
> pCur->pTaCursor = NULL;
>
> if (c) {
> - if (c->iter) box_iterator_free(c->iter);
> - if (c->tuple_last) box_tuple_unref(c->tuple_last);
> - free(c);
> + if (c->iter)
> + box_iterator_free(c->iter);
> + if (c->tuple_last)
> + box_tuple_unref(c->tuple_last);
> + free(c);
> }
> return SQLITE_OK;
> }
> @@ -240,28 +235,16 @@ tarantoolSqlite3TupleColumnFast(BtCursor *pCur, u32 fieldno, u32 *field_size)
> }
>
> /*
> - * Set cursor to the first tuple in ephemeral space.
> - * It is a simple wrapper around cursor_ephemeral_seek.
> + * Set cursor to the first tuple in given space.
> + * It is a simple wrapper around cursor_seek().
> */
> -int tarantoolSqlite3EphemeralFirst(BtCursor *pCur, int *pRes)
> -{
> - return cursor_ephemeral_seek(pCur, pRes, ITER_GE,
> - nil_key, nil_key + sizeof(nil_key));
> -}
> -
> int tarantoolSqlite3First(BtCursor *pCur, int *pRes)
> {
> return cursor_seek(pCur, pRes, ITER_GE,
> nil_key, nil_key + sizeof(nil_key));
> }
>
> -/* Set cursor to the first tuple in ephemeral space. */
> -int tarantoolSqlite3EphemeralLast(BtCursor *pCur, int *pRes)
> -{
> - return cursor_ephemeral_seek(pCur, pRes, ITER_LE,
> - nil_key, nil_key + sizeof(nil_key));
> -}
> -
> +/* Set cursor to the last tuple in given space. */
> int tarantoolSqlite3Last(BtCursor *pCur, int *pRes)
> {
> return cursor_seek(pCur, pRes, ITER_LE,
> @@ -269,13 +252,14 @@ int tarantoolSqlite3Last(BtCursor *pCur, int *pRes)
> }
>
> /*
> - * Set cursor to the next entry in ephemeral space.
> + * Set cursor to the next entry in given space.
> * If state of cursor is invalid (e.g. it is still under construction,
> * or already destroyed), it immediately returns.
> + * Second argument is output parameter: success movement of cursor
> + * results in 0 value of pRes, otherwise it is set to 1.
Please, use defined values instead on 1 and 0.
At least in comment you have added.
> */
> -int tarantoolSqlite3EphemeralNext(BtCursor *pCur, int *pRes)
> +int tarantoolSqlite3Next(BtCursor *pCur, int *pRes)
> {
> - assert(pCur->curFlags & BTCF_TEphemCursor);
> if (pCur->eState == CURSOR_INVALID) {
> *pRes = 1;
Please, use defined values instead on 1 and 0.
> return SQLITE_OK;
> @@ -283,20 +267,6 @@ int tarantoolSqlite3EphemeralNext(BtCursor *pCur, int *pRes)
> assert(pCur->pTaCursor);
> assert(iterator_direction(
> ((struct ta_cursor *)pCur->pTaCursor)->type) > 0);
> - return cursor_ephemeral_advance(pCur, pRes);
> -}
> -
> -int tarantoolSqlite3Next(BtCursor *pCur, int *pRes)
> -{
> - assert(pCur->curFlags & BTCF_TaCursor);
> - if (pCur->eState == CURSOR_INVALID) {
> - *pRes = 1;
> - return SQLITE_OK;
> - }
> - assert(pCur->pTaCursor);
> - assert(iterator_direction(
> - ((struct ta_cursor *)pCur->pTaCursor)->type
> - ) > 0);
> return cursor_advance(pCur, pRes);
> }
>
> @@ -305,9 +275,8 @@ int tarantoolSqlite3Next(BtCursor *pCur, int *pRes)
> * If state of cursor is invalid (e.g. it is still under construction,
> * or already destroyed), it immediately returns.
> */
> -int tarantoolSqlite3EphemeralPrevious(BtCursor *pCur, int *pRes)
> +int tarantoolSqlite3Previous(BtCursor *pCur, int *pRes)
> {
> - assert(pCur->curFlags & BTCF_TEphemCursor);
> if (pCur->eState == CURSOR_INVALID) {
> *pRes = 1;
> return SQLITE_OK;
> @@ -315,20 +284,6 @@ int tarantoolSqlite3EphemeralPrevious(BtCursor *pCur, int *pRes)
> assert(pCur->pTaCursor);
> assert(iterator_direction(
> ((struct ta_cursor *)pCur->pTaCursor)->type) < 0);
> - return cursor_ephemeral_advance(pCur, pRes);
> -}
> -
> -int tarantoolSqlite3Previous(BtCursor *pCur, int *pRes)
> -{
> - assert(pCur->curFlags & BTCF_TaCursor);
> - if (pCur->eState == CURSOR_INVALID) {
> - *pRes = 1;
> - return SQLITE_OK;
> - }
> - assert(pCur->pTaCursor);
> - assert(iterator_direction(
> - ((struct ta_cursor *)pCur->pTaCursor)->type
> - ) < 0);
> return cursor_advance(pCur, pRes);
> }
>
> @@ -383,9 +338,7 @@ int tarantoolSqlite3MovetoUnpacked(BtCursor *pCur, UnpackedRecord *pIdxKey,
> res_success = 0;
> break;
> }
> - rc = (pCur->curFlags & BTCF_TaCursor) ?
> - cursor_seek(pCur, pRes, iter_type, k, ke) :
> - cursor_ephemeral_seek(pCur, pRes, iter_type, k, ke);
> + rc = cursor_seek(pCur, pRes, iter_type, k, ke);
> if (*pRes == 0) {
> *pRes = res_success;
> /*
> @@ -495,7 +448,7 @@ int tarantoolSqlite3EphemeralCreate(BtCursor *pCur, uint32_t field_count,
> pCur->pTaCursor = c;
>
> int unused;
> - return tarantoolSqlite3EphemeralFirst(pCur, &unused);
> + return tarantoolSqlite3First(pCur, &unused);
> }
>
> /*
> @@ -1012,7 +965,6 @@ rename_fail:
> return SQLITE_ERROR;
> }
>
> -
> /*
> * Performs exactly as extract_key + sqlite3VdbeCompareMsgpack,
> * only faster.
> @@ -1200,32 +1152,28 @@ cursor_create(struct ta_cursor *c, size_t key_size)
> * Create new Tarantool iterator and set it to the first entry found by
> * given key. If cursor already contains iterator, it will be freed.
> *
> - * @param pCur Cursor which points to ephemeral space.
> + * @param pCur Cursor which points to space.
> * @param pRes Flag which is == 0 if reached end of space, == 1 otherwise.
> - * @param type Type of iterator.
> + * @param type Type of Tarantool iterator.
> * @param key Start of buffer containing key.
> - * @param key_end End of key.
> + * @param key_end End of buffer containing key.
> *
> * @retval SQLITE_OK on success, SQLITE_TARANTOOL_ERROR otherwise.
> */
> static int
> -cursor_ephemeral_seek(BtCursor *pCur, int *pRes, enum iterator_type type,
> - const char *key, const char *key_end)
> +cursor_seek(BtCursor *pCur, int *pRes, enum iterator_type type,
> + const char *key, const char *key_end)
> {
> - assert(pCur->curFlags & BTCF_TEphemCursor);
> - assert(key != NULL && key_end != NULL);
> - assert(type >= 0 && type < iterator_type_MAX);
> - mp_tuple_assert(key, key_end);
> -
> struct ta_cursor *c = pCur->pTaCursor;
> - assert(c->ephem_space);
> -
Please, save as match asserts as possible.
> size_t key_size = 0;
> +
> + /* Close existing iterator, if any */
> if (c && c->iter) {
> box_iterator_free(c->iter);
> c->iter = NULL;
> }
>
> + /* Allocate or grow cursor if needed. */
> if (type == ITER_EQ || type == ITER_REQ) {
> key_size = (size_t)(key_end - key);
> }
> @@ -1236,16 +1184,33 @@ cursor_ephemeral_seek(BtCursor *pCur, int *pRes, enum iterator_type type,
> }
> pCur->pTaCursor = c;
>
> + /* Copy key if necessary. */
> + if (key_size != 0) {
> + memcpy(c->key, key, key_end - key);
> + key_end = c->key + (key_end - key);
> + key = c->key;
> + }
> +
> + struct space *space;
> + struct index *index;
> + uint32_t space_id = SQLITE_PAGENO_TO_SPACEID(pCur->pgnoRoot);
> + if (space_id != 0) {
> + space = space_by_id(space_id);
> + uint32_t index_id = SQLITE_PAGENO_TO_INDEXID(pCur->pgnoRoot);
> + index = index_find(space, index_id);
> + } else {
> + space = c->ephem_space;
> + index = *space->index;
> + }
> +
> uint32_t part_count = mp_decode_array(&key);
> - struct space *ephem_space = c->ephem_space;
> - struct index *index = *ephem_space->index;
> if (key_validate(index->def, type, key, part_count)) {
> diag_log();
> return SQLITE_TARANTOOL_ERROR;
> }
>
> - struct iterator *it = index_create_iterator(*ephem_space->index, type,
> - key, part_count);
> + struct iterator *it = index_create_iterator(index, type, key,
> + part_count);
> if (it == NULL) {
> pCur->eState = CURSOR_INVALID;
> return SQLITE_TARANTOOL_ERROR;
> @@ -1253,70 +1218,22 @@ cursor_ephemeral_seek(BtCursor *pCur, int *pRes, enum iterator_type type,
> c->iter = it;
> c->type = type;
> pCur->eState = CURSOR_VALID;
> -
> - return cursor_ephemeral_advance(pCur, pRes);
> -}
> -
> -/* Cursor positioning. */
> -static int
> -cursor_seek(BtCursor *pCur, int *pRes, enum iterator_type type,
> - const char *k, const char *ke)
> -{
> - assert(pCur->curFlags & BTCF_TaCursor);
> -
> - struct ta_cursor *c = pCur->pTaCursor;
> - uint32_t space_id = SQLITE_PAGENO_TO_SPACEID(pCur->pgnoRoot);
> - uint32_t index_id = SQLITE_PAGENO_TO_INDEXID(pCur->pgnoRoot);
> - size_t key_size = 0;
> -
> - /* Close existing iterator, if any */
> - if (c && c->iter) {
> - box_iterator_free(c->iter);
> - c->iter = NULL;
> - }
> -
> - /* Allocate or grow cursor if needed. */
> - if (type == ITER_EQ || type == ITER_REQ) {
> - key_size = (size_t)(ke - k);
> - }
> - c = cursor_create(c, key_size);
> - if (!c) {
> - *pRes = 1;
> - return SQLITE_NOMEM;
> - }
> - pCur->pTaCursor = c;
> -
> - /* Copy key if necessary. */
> - if (key_size != 0) {
> - memcpy(c->key, k, ke-k);
> - ke = c->key + (ke-k);
> - k = c->key;
> - }
> -
> - c->iter = box_index_iterator(space_id, index_id, type, k, ke);
> - if (c->iter == NULL) {
> - pCur->eState = CURSOR_INVALID;
> - return SQLITE_TARANTOOL_ERROR;
> - }
> - c->type = type;
> - pCur->eState = CURSOR_VALID;
> return cursor_advance(pCur, pRes);
> }
>
> /*
> - * Move cursor to the next entry in ephemeral space.
> - * New tuple is refed and saved in cursord.
> + * Move cursor to the next entry in space.
> + * New tuple is refed and saved in cursor.
> * Tuple from previous call is unrefed.
> *
> - * @param pCur Cursor which will point to the new ephemeral space.
> - * @param pRes Flag which is == 0 if reached end of space, == 1 otherwise.
> + * @param pCur Cursor which contains space and tuple.
> + * @param[out] pRes Flag which is 0 if reached end of space, 1 otherwise.
> *
> * @retval SQLITE_OK on success, SQLITE_TARANTOOL_ERROR otherwise.
> */
> static int
> -cursor_ephemeral_advance(BtCursor *pCur, int *pRes)
> +cursor_advance(BtCursor *pCur, int *pRes)
> {
> - assert(pCur->curFlags & BTCF_TEphemCursor);
> struct ta_cursor *c = pCur->pTaCursor;
> assert(c);
> assert(c->iter);
> @@ -1338,34 +1255,6 @@ cursor_ephemeral_advance(BtCursor *pCur, int *pRes)
> return SQLITE_OK;
> }
>
> -static int
> -cursor_advance(BtCursor *pCur, int *pRes)
> -{
> - assert(pCur->curFlags & BTCF_TaCursor);
> -
> - struct ta_cursor *c;
> - struct tuple *tuple;
> - int rc;
> -
> - c = pCur->pTaCursor;
> - assert(c);
> - assert(c->iter);
> -
> - rc = box_iterator_next(c->iter, &tuple);
> - if (rc)
> - return SQLITE_TARANTOOL_ERROR;
> - if (c->tuple_last) box_tuple_unref(c->tuple_last);
> - if (tuple) {
> - box_tuple_ref(tuple);
> - *pRes = 0;
> - } else {
> - pCur->eState = CURSOR_INVALID;
> - *pRes = 1;
> - }
> - c->tuple_last = tuple;
> - return SQLITE_OK;
> -}
> -
> /*********************************************************************
> * Schema support.
> */
> @@ -1399,11 +1288,6 @@ sql_schema_put(InitData *init,
> sqlite3InitCallback(init, 3, argv, NULL);
> }
>
> -struct space;
> -
> -struct space *
> -space_by_id(uint32_t id);
> -
> static int
> space_foreach_put_cb(struct space *space, void *udata)
> {
> diff --git a/src/box/sql/cursor.c b/src/box/sql/cursor.c
> index 34066ffe4..22ecfe0da 100644
> --- a/src/box/sql/cursor.c
> +++ b/src/box/sql/cursor.c
> @@ -184,10 +184,8 @@ sqlite3CursorNext(BtCursor *pCur, int *pRes)
> (pCur->curFlags & BTCF_TEphemCursor));
>
> *pRes = 0;
> - if (pCur->curFlags & BTCF_TaCursor) {
> - return tarantoolSqlite3Next(pCur, pRes);
> - }
> - return tarantoolSqlite3EphemeralNext(pCur, pRes);
> + return tarantoolSqlite3Next(pCur, pRes);
> +
Extra blank line
> }
>
> int
> @@ -199,10 +197,7 @@ sqlite3CursorPrevious(BtCursor *pCur, int *pRes)
> (pCur->curFlags & BTCF_TEphemCursor));
>
> *pRes = 0;
> - if (pCur->curFlags & BTCF_TaCursor) {
> - return tarantoolSqlite3Previous(pCur, pRes);
> - }
> - return tarantoolSqlite3EphemeralPrevious(pCur, pRes);
> + return tarantoolSqlite3Previous(pCur, pRes);
> }
>
> /*
> diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
> index 134388d61..489d0ec86 100644
> --- a/src/box/sql/tarantoolInt.h
> +++ b/src/box/sql/tarantoolInt.h
> @@ -96,11 +96,7 @@ int tarantoolSqlite3EphemeralCreate(BtCursor * pCur, uint32_t filed_count,
> struct coll *aColl);
> int tarantoolSqlite3EphemeralInsert(BtCursor * pCur, const CursorPayload * pX);
> int tarantoolSqlite3EphemeralDelete(BtCursor * pCur);
> -int tarantoolSqlite3EphemeralFirst(BtCursor * pCur, int * pRes);
> -int tarantoolSqlite3EphemeralNext(BtCursor * pCur, int * pRes);
> -int tarantoolSqlite3EphemeralLast(BtCursor * pCur, int * pRes);
> int tarantoolSqlite3EphemeralCount(BtCursor * pCur, i64 * pnEntry);
> -int tarantoolSqlite3EphemeralPrevious(BtCursor * pCur, int * pRes);
> int tarantoolSqlite3EphemeralDrop(BtCursor * pCur);
> int tarantoolSqlite3EphemeralClearTable(BtCursor * pCur);
> int tarantoolSqlite3EphemeralGetMaxId(BtCursor * pCur, uint32_t fieldno,
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index c3d726672..ddd849903 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -4195,13 +4195,7 @@ case OP_Last: { /* jump */
> pC->seekOp = OP_Last;
> #endif
> if (pOp->p3==0 || !sqlite3CursorIsValidNN(pCrsr)) {
> - if (pCrsr->curFlags & BTCF_TaCursor) {
> - rc = tarantoolSqlite3Last(pCrsr, &res);
> - } else if (pCrsr->curFlags & BTCF_TEphemCursor) {
> - rc = tarantoolSqlite3EphemeralLast(pCrsr, &res);
> - } else {
> - unreachable();
> - }
> + rc = tarantoolSqlite3Last(pCrsr, &res);
> pC->nullRow = (u8)res;
> pC->deferredMoveto = 0;
> pC->cacheStatus = CACHE_STALE;
> @@ -4279,13 +4273,7 @@ case OP_Rewind: { /* jump */
> assert(pC->eCurType==CURTYPE_TARANTOOL);
> pCrsr = pC->uc.pCursor;
> assert(pCrsr);
> - if (pCrsr->curFlags & BTCF_TaCursor) {
> - rc = tarantoolSqlite3First(pCrsr, &res);
> - } else if (pCrsr->curFlags & BTCF_TEphemCursor) {
> - rc = tarantoolSqlite3EphemeralFirst(pCrsr, &res);
> - } else {
> - unreachable();
> - }
> + rc = tarantoolSqlite3First(pCrsr, &res);
> pC->deferredMoveto = 0;
> pC->cacheStatus = CACHE_STALE;
> }
More information about the Tarantool-patches
mailing list