[tarantool-patches] [PATCH 2/2] sql: refactor cursor closing routine

[Nikita Pettik <korablev@tarantool.org>]

Unite 'close' and 'clear' functions into one. Fix naming and codestyle.
---
 src/box/sql.c              | 15 +--------------
 src/box/sql/cursor.c       | 46 +++++++++++++++++++++++++---------------------
 src/box/sql/cursor.h       |  6 ++++--
 src/box/sql/tarantoolInt.h |  1 -
 src/box/sql/vdbe.c         |  2 +-
 src/box/sql/vdbeaux.c      |  2 +-
 6 files changed, 32 insertions(+), 40 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index dd0cfcc1a..3138d9991 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -114,8 +114,6 @@ sql_get()
 
 /*********************************************************************
  * SQLite cursor implementation on top of Tarantool storage API-s.
- * See the corresponding SQLite function in btree.c for documentation.
- * Ex: sqlite3BtreeCloseCursor -> tarantoolSqlite3CloseCursor
  *
  * NB: SQLite btree cursor emulation is less than perfect. The problem
  * is that btree cursors are more low-level compared to Tarantool
@@ -184,18 +182,6 @@ is_tarantool_error(int rc)
 		rc == SQL_TARANTOOL_INSERT_FAIL);
 }
 
-int tarantoolSqlite3CloseCursor(BtCursor *pCur)
-{
-	assert(pCur->curFlags & BTCF_TaCursor ||
-	       pCur->curFlags & BTCF_TEphemCursor);
-
-	if (pCur->iter)
-		box_iterator_free(pCur->iter);
-	if (pCur->last_tuple)
-		box_tuple_unref(pCur->last_tuple);
-	return SQLITE_OK;
-}
-
 const void *tarantoolSqlite3PayloadFetch(BtCursor *pCur, u32 *pAmt)
 {
 	assert(pCur->curFlags & BTCF_TaCursor ||
@@ -467,6 +453,7 @@ int tarantoolSqlite3EphemeralDrop(BtCursor *pCur)
 	assert(pCur);
 	assert(pCur->curFlags & BTCF_TEphemCursor);
 	space_delete_ephemeral(pCur->space);
+	pCur->space = NULL;
 	return SQLITE_OK;
 }
 
diff --git a/src/box/sql/cursor.c b/src/box/sql/cursor.c
index 533bfb587..c38629f3b 100644
--- a/src/box/sql/cursor.c
+++ b/src/box/sql/cursor.c
@@ -31,18 +31,26 @@
 
 #include "sqliteInt.h"
 #include "tarantoolInt.h"
+#include "box/tuple.h"
 
-/*
- * Clear the current cursor position.
+/**
+ * Release tuple, free iterator, invalidate cursor's state.
+ * Note that this routine doesn't nullify space and index:
+ * it is also used during OP_NullRow opcode to refresh given
+ * cursor.
  */
 void
-sqlite3ClearCursor(BtCursor * pCur)
+sql_cursor_cleanup(struct BtCursor *cursor)
 {
-	free(pCur->key);
-	pCur->key = 0;
-	pCur->iter = NULL;
-	pCur->last_tuple = NULL;
-	pCur->eState = CURSOR_INVALID;
+	if (cursor->iter)
+		box_iterator_free(cursor->iter);
+	if (cursor->last_tuple)
+		tuple_unref(cursor->last_tuple);
+	free(cursor->key);
+	cursor->key = NULL;
+	cursor->iter = NULL;
+	cursor->last_tuple = NULL;
+	cursor->eState = CURSOR_INVALID;
 }
 
 /*
@@ -64,23 +72,19 @@ sqlite3CursorZero(BtCursor * p)
 	memset(p, 0, sizeof(*p));
 }
 
-/*
+/**
  * Close a cursor and invalidate its state. In case of
  * ephemeral cursor, corresponding space should be dropped.
  */
-int
-sqlite3CloseCursor(BtCursor * pCur)
+void
+sql_cursor_close(struct BtCursor *cursor)
 {
-	assert((pCur->curFlags & BTCF_TaCursor) ||
-	       (pCur->curFlags & BTCF_TEphemCursor));
-
-	if (pCur->curFlags & BTCF_TEphemCursor) {
-		tarantoolSqlite3EphemeralDrop(pCur);
-	}
-	tarantoolSqlite3CloseCursor(pCur);
-	sqlite3ClearCursor(pCur);
-
-	return SQLITE_OK;
+	assert(cursor->space != NULL);
+	assert((cursor->curFlags & BTCF_TaCursor) ||
+	       (cursor->curFlags & BTCF_TEphemCursor));
+	if (cursor->curFlags & BTCF_TEphemCursor)
+		tarantoolSqlite3EphemeralDrop(cursor);
+	sql_cursor_cleanup(cursor);
 }
 
 #ifndef NDEBUG			/* The next routine used only within assert() statements */
diff --git a/src/box/sql/cursor.h b/src/box/sql/cursor.h
index 5b8e7c29d..e5d2aae3a 100644
--- a/src/box/sql/cursor.h
+++ b/src/box/sql/cursor.h
@@ -72,14 +72,16 @@ struct BtCursor {
 void sqlite3CursorZero(BtCursor *);
 void sqlite3CursorHintFlags(BtCursor *, unsigned);
 
-int sqlite3CloseCursor(BtCursor *);
+void
+sql_cursor_close(struct BtCursor *cursor);
 int sqlite3CursorMovetoUnpacked(BtCursor *, UnpackedRecord * pUnKey, int *pRes);
 
 int sqlite3CursorNext(BtCursor *, int *pRes);
 int sqlite3CursorPrevious(BtCursor *, int *pRes);
 int sqlite3CursorPayload(BtCursor *, u32 offset, u32 amt, void *);
 
-void sqlite3ClearCursor(BtCursor *);
+void
+sql_cursor_cleanup(struct BtCursor *cursor);
 int sqlite3CursorHasHint(BtCursor *, unsigned int mask);
 
 #ifndef NDEBUG
diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
index 19c6fdca9..ca31013ef 100644
--- a/src/box/sql/tarantoolInt.h
+++ b/src/box/sql/tarantoolInt.h
@@ -52,7 +52,6 @@ const char *tarantoolErrorMessage();
 int is_tarantool_error(int rc);
 
 /* Storage interface. */
-int tarantoolSqlite3CloseCursor(BtCursor * pCur);
 const void *tarantoolSqlite3PayloadFetch(BtCursor * pCur, u32 * pAmt);
 
 /**
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 8151e1557..0fddbcb66 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4061,7 +4061,7 @@ case OP_NullRow: {
 	pC->cacheStatus = CACHE_STALE;
 	if (pC->eCurType==CURTYPE_TARANTOOL) {
 		assert(pC->uc.pCursor!=0);
-		sqlite3ClearCursor(pC->uc.pCursor);
+		sql_cursor_cleanup(pC->uc.pCursor);
 	}
 	break;
 }
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index bb121a318..40572fc97 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -2217,7 +2217,7 @@ sqlite3VdbeFreeCursor(Vdbe * p, VdbeCursor * pCx)
 		}
 	case CURTYPE_TARANTOOL:{
 		assert(pCx->uc.pCursor != 0);
-		sqlite3CloseCursor(pCx->uc.pCursor);
+		sql_cursor_close(pCx->uc.pCursor);
 			break;
 		}
 	}
-- 
2.15.1