Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org
Cc: korablev@tarantool.org, Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: [tarantool-patches] [PATCH 3/4] sql: simplify lua SQL executor
Date: Tue,  3 Apr 2018 18:34:14 +0300	[thread overview]
Message-ID: <6ae678aac75653b3486c8a194c504c231ea34f56.1522769319.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1522769319.git.v.shpilevoy@tarantool.org>
In-Reply-To: <cover.1522769319.git.v.shpilevoy@tarantool.org>

1) Code is completely unreadable and complicated by strange
   optimizations;
2) It still supposes, that there can be multiple statements in a
   single string;
3) It pushes first letter of affinity together with column names
   - why? It is not used anywhere.

Lets remove prepared statements list, affinity first letters
pushing.
---
 src/box/lua/sql.c | 239 ++++++++----------------------------------------------
 1 file changed, 35 insertions(+), 204 deletions(-)

diff --git a/src/box/lua/sql.c b/src/box/lua/sql.c
index 12b81383d..e693aeaad 100644
--- a/src/box/lua/sql.c
+++ b/src/box/lua/sql.c
@@ -6,132 +6,12 @@
 #include "lua/utils.h"
 #include "info.h"
 
-struct prep_stmt
-{
-	sqlite3_stmt *stmt;
-};
-
-struct prep_stmt_list
-{
-	uint8_t         *mem_end;   /* denotes actual size of sql_ctx struct */
-	uint32_t         pool_size; /* mem at the end used for aux allocations;
-				       pool grows from mem_end
-				       towards stmt[] array */
-	uint32_t         last_select_stmt_index; /* UINT32_MAX if no selects */
-	uint32_t         column_count; /* in last select stmt */
-	uint32_t         stmt_count;
-	struct prep_stmt stmt[6];  /* overlayed with the mem pool;
-				      actual size could be larger or smaller */
-	/* uint8_t mem_pool[] */
-};
-
-static inline int
-prep_stmt_list_needs_free(struct prep_stmt_list *l)
-{
-	return (uint8_t *)(l + 1) != l->mem_end;
-}
-
-/* Release resources and free the list itself, unless it was preallocated
- * (i.e. l points to an automatic variable) */
-static void
-prep_stmt_list_free(struct prep_stmt_list *l)
-{
-	if (l == NULL)
-		return;
-	for (size_t i = 0, n = l->stmt_count; i < n; i++)
-		sqlite3_finalize(l->stmt[i].stmt);
-	if (prep_stmt_list_needs_free(l))
-		free(l);
-}
-
-static struct prep_stmt_list *
-prep_stmt_list_init(struct prep_stmt_list *prealloc)
-{
-	prealloc->mem_end = (uint8_t *)(prealloc + 1);
-	prealloc->pool_size = 0;
-	prealloc->last_select_stmt_index = UINT32_MAX;
-	prealloc->column_count = 0;
-	prealloc->stmt_count = 0;
-	return prealloc;
-}
-
-/* Allocate mem from the prep_stmt_list pool.
- * If not enough space is available, reallocates the list.
- * If reallocation is needed but l was preallocated, old mem is left
- * intact and a new memory chunk is allocated. */
-static void *
-prep_stmt_list_palloc(struct prep_stmt_list **pl,
-		      size_t size, size_t alignment)
-{
-	assert((alignment & (alignment - 1)) == 0); /* 2 ^ k */
-	assert(alignment <= __alignof__((*pl)->stmt[0]));
-
-	struct prep_stmt_list *l = *pl;
-	uint32_t pool_size = l->pool_size;
-	uint32_t pool_size_max = (uint32_t)(
-		l->mem_end - (uint8_t *)(l->stmt + l->stmt_count)
-	);
-
-	assert(UINT32_MAX - pool_size >= size);
-	pool_size += size;
-
-	assert(UINT32_MAX - pool_size >= alignment - 1);
-	pool_size += alignment - 1;
-	pool_size &= ~(alignment - 1);
-
-	if (pool_size > pool_size_max) {
-		size_t prev_size = l->mem_end - (uint8_t *)l;
-		size_t size = prev_size;
-		while (size < prev_size + (pool_size - pool_size_max)) {
-			assert(SIZE_MAX - size >= size);
-			size += size;
-		}
-		if (prep_stmt_list_needs_free(l)) {
-			l = realloc(l, size);
-			if (l == NULL)
-				return NULL;
-		} else {
-			l = malloc(size);
-			if (l == NULL)
-				return NULL;
-			memcpy(l, *pl, prev_size);
-		}
-		l->mem_end = (uint8_t *)l + size;
-		/* move the pool data */
-		memmove((uint8_t *)l + prev_size - l->pool_size,
-			l->mem_end - l->pool_size,
-			l->pool_size);
-		*pl = l;
-	}
-
-	l->pool_size = pool_size;
-	return l->mem_end - pool_size;
-}
-
-/* push new stmt; reallocate memory if needed
- * returns a pointer to the new stmt or NULL if out of memory.
- * If reallocation is needed but l was preallocated, old mem is left
- * intact and a new memory chunk is allocated. */
-static struct prep_stmt *
-prep_stmt_list_push(struct prep_stmt_list **pl)
-{
-	struct prep_stmt_list *l;
-	/* make sure we don't collide with the pool */
-	if (prep_stmt_list_palloc(pl, sizeof(l->stmt[0]), 1
-				  ) == NULL)
-		return NULL;
-	l = *pl;
-	l->pool_size -= sizeof(l->stmt[0]);
-	return l->stmt + (l->stmt_count++);
-}
-
 static void
-lua_push_column_names(struct lua_State *L, struct prep_stmt_list *l)
+lua_push_column_names(struct lua_State *L, struct sqlite3_stmt *stmt)
 {
-	sqlite3_stmt *stmt = l->stmt[l->last_select_stmt_index].stmt;
-	int n = l->column_count;
-	lua_createtable(L, n, 0);
-	for (int i = 0; i < n; i++) {
+	int column_count = sqlite3_column_count(stmt);
+	lua_createtable(L, column_count, 0);
+	for (int i = 0; i < column_count; i++) {
 		const char *name = sqlite3_column_name(stmt, i);
 		lua_pushstring(L, name == NULL ? "" : name);
 		lua_rawseti(L, -2, i+1);
@@ -139,11 +19,9 @@ lua_push_column_names(struct lua_State *L, struct prep_stmt_list *l)
 }
 
 static void
-lua_push_row(struct lua_State *L, struct prep_stmt_list *l)
+lua_push_row(struct lua_State *L, struct sqlite3_stmt *stmt)
 {
-	sqlite3_stmt *stmt = l->stmt[l->last_select_stmt_index].stmt;
-	int column_count = l->column_count;
-	char *typestr = (void *)(l->mem_end - column_count);
+	int column_count = sqlite3_column_count(stmt);
 
 	lua_createtable(L, column_count, 0);
 	lua_rawgeti(L, LUA_REGISTRYINDEX, luaL_array_metatable_ref);
@@ -153,124 +31,77 @@ lua_push_row(struct lua_State *L, struct prep_stmt_list *l)
 		int type = sqlite3_column_type(stmt, i);
 		switch (type) {
 		case SQLITE_INTEGER:
-			typestr[i] = 'i';
-            luaL_pushint64(L, sqlite3_column_int64(stmt, i));
+			luaL_pushint64(L, sqlite3_column_int64(stmt, i));
 			break;
 		case SQLITE_FLOAT:
-			typestr[i] = 'f';
 			lua_pushnumber(L, sqlite3_column_double(stmt, i));
 			break;
 		case SQLITE_TEXT: {
 			const void *text = sqlite3_column_text(stmt, i);
-			typestr[i] = 's';
 			lua_pushlstring(L, text,
 					sqlite3_column_bytes(stmt, i));
 			break;
 		}
 		case SQLITE_BLOB: {
 			const void *blob = sqlite3_column_blob(stmt, i);
-			typestr[i] = 'b';
 			lua_pushlstring(L, blob,
 					sqlite3_column_bytes(stmt, i));
 			break;
 		}
 		case SQLITE_NULL:
-			typestr[i] = '-';
 			lua_rawgeti(L, LUA_REGISTRYINDEX, luaL_nil_ref);
 			break;
 		default:
-			typestr[i] = '?';
 			assert(0);
 		}
 		lua_rawseti(L, -2, i+1);
 	}
-
-	lua_pushlstring(L, typestr, column_count);
-	lua_rawseti(L, -2, 0);
 }
 
 static int
 lua_sql_execute(struct lua_State *L)
 {
-	int rc;
 	sqlite3 *db = sql_get();
-	struct prep_stmt_list *l = NULL, stock_l;
-	size_t length;
-	const char *sql, *sql_end;
-
 	if (db == NULL)
 		return luaL_error(L, "not ready");
 
-	sql = lua_tolstring(L, 1, &length);
+	size_t length;
+	const char *sql = lua_tolstring(L, 1, &length);
 	if (sql == NULL)
 		return luaL_error(L, "usage: box.sql.execute(sqlstring)");
 
-	assert(length <= INT_MAX);
-	sql_end = sql + length;
+	struct sqlite3_stmt *stmt;
+	if (sqlite3_prepare_v2(db, sql, length, &stmt, &sql) != SQLITE_OK)
+		goto sqlerror;
+	assert(stmt != NULL);
 
-	l = prep_stmt_list_init(&stock_l);
-	do {
-
-		struct prep_stmt *ps = prep_stmt_list_push(&l);
-		if (ps == NULL)
-			goto outofmem;
-		rc = sqlite3_prepare_v2(db, sql, (int)(sql_end - sql),
-					&ps->stmt, &sql);
-		if (rc != SQLITE_OK)
-			goto sqlerror;
-
-		if (ps->stmt == NULL) {
-			/* only whitespace */
-			assert(sql == sql_end);
-			l->stmt_count --;
-			break;
-		}
-
-		int column_count = sqlite3_column_count(ps->stmt);
-		if (column_count == 0) {
-			while ((rc = sqlite3_step(ps->stmt)) == SQLITE_ROW) { ; }
-		} else {
-			char *typestr;
-			l->column_count = column_count;
-			l->last_select_stmt_index = l->stmt_count - 1;
-
-			assert(l->pool_size == 0);
-			/* This might possibly call realloc() and ruin *ps.  */
-			typestr = prep_stmt_list_palloc(&l, column_count, 1);
-			if (typestr == NULL)
-				goto outofmem;
-			/* Refill *ps.  */
-			ps = l->stmt + l->stmt_count - 1;
-
-			lua_settop(L, 1); /* discard any results */
-
-			/* create result table */
-			lua_createtable(L, 7, 0);
-			lua_pushvalue(L, lua_upvalueindex(1));
-			lua_setmetatable(L, -2);
-			lua_push_column_names(L, l);
-			lua_rawseti(L, -2, 0);
-
-			int row_count = 0;
-			while ((rc = sqlite3_step(ps->stmt)) == SQLITE_ROW) {
-				lua_push_row(L, l);
-				row_count++;
-				lua_rawseti(L, -2, row_count);
-			}
-			l->pool_size = 0;
+	int rc;
+	int retval_count;
+	if (sqlite3_column_count(stmt) == 0) {
+		while ((rc = sqlite3_step(stmt)) == SQLITE_ROW);
+		retval_count = 0;
+	} else {
+		lua_newtable(L);
+		lua_pushvalue(L, lua_upvalueindex(1));
+		lua_setmetatable(L, -2);
+		lua_push_column_names(L, stmt);
+		lua_rawseti(L, -2, 0);
+
+		int row_count = 0;
+		while ((rc = sqlite3_step(stmt)) == SQLITE_ROW) {
+			lua_push_row(L, stmt);
+			lua_rawseti(L, -2, ++row_count);
 		}
+		retval_count = 1;
+	}
         if (rc != SQLITE_OK && rc != SQLITE_DONE)
-            goto sqlerror;
-	} while (sql != sql_end);
-	prep_stmt_list_free(l);
-	return lua_gettop(L) - 1;
+		goto sqlerror;
+	sqlite3_finalize(stmt);
+	return retval_count;
 sqlerror:
 	lua_pushstring(L, sqlite3_errmsg(db));
-	prep_stmt_list_free(l);	
+	sqlite3_finalize(stmt);
 	return lua_error(L);
-outofmem:
-	prep_stmt_list_free(l);
-	return luaL_error(L, "out of memory");
 }
 
 static int
-- 
2.14.3 (Apple Git-98)

  parent reply	other threads:[~2018-04-03 15:34 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-03 15:34 [tarantool-patches] [PATCH 0/4] sql: refactor insertion and lua Vladislav Shpilevoy
2018-04-03 15:34 ` [tarantool-patches] [PATCH 1/4] sql: remove unused opcodes from OP_[Sorter/Idx][Insert/Replace] Vladislav Shpilevoy
2018-04-03 23:01   ` [tarantool-patches] " n.pettik
2018-04-04 10:26     ` Vladislav Shpilevoy
2018-04-04 11:51       ` n.pettik
2018-04-04 14:48         ` n.pettik
2018-04-03 15:34 ` [tarantool-patches] [PATCH 2/4] sql: remove unused error codes, use enum instead of defines Vladislav Shpilevoy
2018-04-03 23:01   ` [tarantool-patches] " n.pettik
2018-04-04 10:26     ` Vladislav Shpilevoy
2018-04-04 11:30       ` n.pettik
2018-04-03 15:34 ` Vladislav Shpilevoy [this message]
2018-04-03 23:02   ` [tarantool-patches] Re: [PATCH 3/4] sql: simplify lua SQL executor n.pettik
2018-04-03 15:34 ` [tarantool-patches] [PATCH 4/4] sql: remove useless branching in insertOrReplace Vladislav Shpilevoy
2018-04-03 23:01   ` [tarantool-patches] " n.pettik
2018-04-05 11:43 ` [tarantool-patches] Re: [PATCH 0/4] sql: refactor insertion and lua Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6ae678aac75653b3486c8a194c504c231ea34f56.1522769319.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH 3/4] sql: simplify lua SQL executor' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox