[tarantool-patches] Re: [PATCH 3/4] sql: simplify lua SQL executor
n.pettik
korablev at tarantool.org
Wed Apr 4 02:02:20 MSK 2018
Ack.
> On 3 Apr 2018, at 18:34, Vladislav Shpilevoy <v.shpilevoy at tarantool.org> wrote:
>
> 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)
>
>
More information about the Tarantool-patches
mailing list