[tarantool-patches] Re: [PATCH v1 1/1] sql: introduce TRUNCATE TABLE operation

  • From: "n.pettik" <korablev@xxxxxxxxxxxxx>
  • To: tarantool-patches@xxxxxxxxxxxxx
  • Date: Wed, 25 Jul 2018 15:58:10 +0300

Except for comments below, also pls add to tarantool-doc bot request
notice concerning the fact that in our implementation TRUNCATE is DDL operation.


diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
index 1ec1538..705caf1 100644
--- a/extra/mkkeywordhash.c
+++ b/extra/mkkeywordhash.c
@@ -281,6 +281,7 @@ static Keyword aKeywordTable[] = {
  { "VARCHAR",                "TK_ID",          RESERVED,         true  },
  { "WHENEVER",               "TK_STANDARD",    RESERVED,         true  },
  { "WHILE",                  "TK_STANDARD",    RESERVED,         true  },
+  { "TRUNCATE",               "TK_TRUNCATE",    ALWAYS,           true  },
};

/* Number of keywords */
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 0681177..b016084 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -72,6 +72,62 @@ sql_materialize_view(struct Parse *parse, const char 
*name, struct Expr *where,
}

void
+sql_table_truncate(struct Parse *parse, struct SrcList *tab_list)
+{
+     struct sqlite3 *db = parse->db;
+     if (parse->nErr || db->mallocFailed)
+             goto cleanup;

Is this check reasonable?

+     assert(tab_list->nSrc == 1);
+
+     struct Vdbe *v = sqlite3GetVdbe(parse);
+     if (v == NULL)
+             goto cleanup;
+
+     const char *tab_name = tab_list->a->zName;
+     struct Table *table = sqlite3LocateTable(parse, LOCATE_NOERR, tab_name);

I guess here you can explicitly use space_by_id and box_space_id_by_name
to bypass table hash. If you do so, you will be able to get rid off 
if (table == NULL) branch.

+     struct space_def *space_def = NULL;
+     if (table == NULL) {
+             /* Space created with LUA. */
+             uint32_t space_id =
+                     box_space_id_by_name(tab_name, strlen(tab_name));
+             if (space_id == BOX_ID_NIL) {
+                     diag_set(ClientError, ER_NO_SUCH_SPACE, tab_name);
+                     parse->rc = SQL_TARANTOOL_ERROR;
+                     parse->nErr++;

During exception handling you three times process
parse->rc = SQL_TARANTOOL_ERROR;
parse->nErr++;

Lets use separate label for this purpose.

+                     goto cleanup;
+             }
+             struct space *space = space_cache_find(space_id);
+             assert(space != NULL);
+             space_def = space->def;
+     } else {
+             space_def = table->def;
+             if (sqlite3FkRequired(table, NULL) != 0) {

sqlite3FkRequired is too strict condition. I guess you are still
capable of truncating child (or referencing) table, since no
FK violations may take place in this case.

So, what you need is simple check:

sqlite3References() != NULL;

+                     const char *err_msg =
+                             tt_sprintf("cannot truncate %s because it has "
+                                        "foreign keys”);

Lets add ’space’ or ’table' word to message to avoid confusing:

“can no truncate table %s becase other objects depend on it”
OR
“can no truncate table %s becase other objects reference it”

+                     diag_set(ClientError, ER_SQL, err_msg);
+                     parse->rc = SQL_TARANTOOL_ERROR;
+                     parse->nErr++;
+                     goto cleanup;
+             }
+     }
+     assert(space_def != NULL);
+     if (space_def->opts.is_view) {
+             const char *err_msg =
+                     tt_sprintf("cannot truncate %s because it is a view",
+                                space_def->name);
+             diag_set(ClientError, ER_SQL, err_msg);
+             parse->rc = SQL_TARANTOOL_ERROR;
+             parse->nErr++;
+             goto cleanup;
+     }
+     sqlite3VdbeAddOp2(v, OP_Clear, space_def->id, true);
+
+cleanup:
+     sqlite3SrcListDelete(parse->db, tab_list);
+}
+
+void
sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
                    struct Expr *where)
{

diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 07e7829..a90b10e 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3710,11 +3710,22 @@ void sqlite3OpenTable(Parse *, int iCur, Table *, 
int);
 * @param tab_list List of single element which table from which
 * deletetion if performed.
 * @param where The WHERE clause.  May be NULL.
+ * @param is_truncate use OP_Clear to truncate table.

This comment is remained from previous patch version, I guess.

--- a/test/sql/delete.test.lua
+++ b/test/sql/delete.test.lua
@@ -37,3 +37,42 @@ box.sql.execute("CREATE TRIGGER t2 BEFORE INSERT ON t2 
BEGIN DELETE FROM t1; END
box.sql.execute("INSERT INTO t2 VALUES (0);")

box.sql.execute("DROP TABLE t2;")
+
+
+--
+-- gh-2201: TRUNCATE TABLE operation
+--
+
+-- can't truncate system table

Pls start comments (even in tests) from capital letter and end with dot.

+box.sql.execute("TRUNCATE TABLE \"_sql_stat1\";")
+
+box.sql.execute("CREATE TABLE t1(id INTEGER PRIMARY KEY, a, b);”)

Specify types for all columns. Static typing is on the way.


Other related posts:

  1. Home
  2. tarantool-patches
  3. Archive
  4. 07-2018