[tarantool-patches] Re: [PATCH v1 1/1] sql: introduce TRUNCATE TABLE operation
n.pettik
korablev at tarantool.org
Wed Jul 25 15:58:10 MSK 2018
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.
More information about the Tarantool-patches
mailing list