Tarantool development patches archive
 help / color / mirror / Atom feed
From: Imeev Mergen <imeevma@tarantool.org>
To: "n.pettik" <korablev@tarantool.org>, tarantool-patches@freelists.org
Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH v2 1/1] sql: cleanup on failed creation operation
Date: Mon, 8 Oct 2018 22:39:50 +0300	[thread overview]
Message-ID: <51918bf1-14ea-45aa-4e1e-901d2ee9c532@tarantool.org> (raw)
In-Reply-To: <F2EF645E-C5C2-4EAA-B4B3-A9D8D6D6A817@tarantool.org>

[-- Attachment #1: Type: text/plain, Size: 24032 bytes --]

Hello! Thank you for review! New patch below.


On 10/01/2018 04:05 PM, n.pettik wrote:
>
>> commit fe8415a79d401b741dcb565d34eb56495223f8b6
>> Author: Mergen Imeev <imeevma@gmail.com <mailto:imeevma@gmail.com>>
>> Date:   Fri Aug 31 15:50:17 2018 +0300
>>
>>     sql: cleanup on failed creation operation
>>
>>     Some creation operations create objects even on fail. This is
>>     wrong and should be fixed. This patch adds destructors for such
>>     objects.
>
> Kind of poor explanation. Where those destructors are used?
> Which objects are you talking about? AFAIU ‘some creation operations’
> may refer only to table creation; in other cases only one object created
> per SQL statement: CREATE INDEX -> one index, CREATE TRIGGER ->
> one trigger, ADD CONSTRAINT -> one constraint.
> For this reason, you don’t need to add to list of records indexes and fk
> constraints when it comes to separate CREATE INDEX/ADD CONSTRAINT
> statements. Extend your explanation and provide examples, please.
Fixed.
>
>>
>>     Closes #3592
>>
>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>> index 60b49df..2ac86ab 100644
>> --- a/src/box/sql/build.c
>> +++ b/src/box/sql/build.c
>> +/**
>> + * Save inserted in system space record in list.
>> + *
>> + * @param parser SQL Parser object.
>> + * @param space_id Id of table in which record is inserted.
>> + * @param reg_key Register that contains first field of the key.
>> + * @param reg_key_count Exact number of fields of the key.
>> + * @param insertion_opcode Number of OP_SInsert opcode.
>> + */
>> +static inline void
>> +save_record(struct Parse *parser, uint32_t space_id, int reg_key,
>> +        int reg_key_count, int insertion_opcode)
>> +{
>> +    struct saved_record *record =
>> + region_alloc(&fiber()->gc, sizeof(*record));
>
> Why do you use global region? You can use parser’s one.
Fixed.
>
>>  void
>>  sql_finish_coding(struct Parse *parse_context)
>> @@ -62,6 +110,29 @@ sql_finish_coding(struct Parse *parse_context)
>>      struct sqlite3 *db = parse_context->db;
>>      struct Vdbe *v = sqlite3GetVdbe(parse_context);
>>      sqlite3VdbeAddOp0(v, OP_Halt);
>> +    /*
>> +     * Destructors for all created in current statement
>> +     * spaces, indexes, sequences etc. There is no need to
>> +     * create destructor for last SInsert.
>> +     */
>> +    if (rlist_empty(&parse_context->record_list) == 0) {
>
> Nit: if(! rlist_empty())
Fixed.
>
> Also, lets add comments directly to VDBE program indicating labels
> for each clean-up jumps (VdbeComment). It would significantly
> simplify debugging of VDBE programs.
Done.
>
>> +        struct saved_record *record =
>> + rlist_shift_entry(&parse_context->record_list,
>> +                      struct saved_record, link);
>> +        /* Set P2 of SInsert. */
>> + sqlite3VdbeChangeP2(v, record->insertion_opcode, v->nOp);
>> + rlist_foreach_entry(record, &parse_context->record_list, link) {
>> +            int record_reg = ++parse_context->nMem;
>> + sqlite3VdbeAddOp3(v, OP_MakeRecord, record->reg_key,
>> + record->reg_key_count, record_reg);
>> + sqlite3VdbeAddOp2(v, OP_SDelete, record->space_id,
>> + record_reg);
>> +            /* Set P2 of SInsert. */
>> + sqlite3VdbeChangeP2(v, record->insertion_opcode,
>> + v->nOp);
>> +        }
>> +        sqlite3VdbeAddOp2(v, OP_Halt, SQL_TARANTOOL_ERROR, 0);
>> +    }
>>      /* Code creation of FK constraints, if any. */
>>      struct fkey_parse *fk_parse;
>> diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
>> index e98e845..a4b65eb 100644
>> --- a/src/box/sql/prepare.c
>> +++ b/src/box/sql/prepare.c
>> @@ -274,6 +274,7 @@ sql_parser_create(struct Parse *parser, sqlite3 *db)
>>      memset(parser, 0, sizeof(struct Parse));
>>      parser->db = db;
>> rlist_create(&parser->new_fkey);
>> + rlist_create(&parser->record_list);
>> region_create(&parser->region, &cord()->slabc);
>>  }
>>
>> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
>> index f56090d..c5becbd 100644
>> --- a/src/box/sql/sqliteInt.h
>> +++ b/src/box/sql/sqliteInt.h
>> @@ -2764,6 +2764,11 @@ struct Parse {
>>       * Foreign key constraint appeared in CREATE TABLE stmt.
>>       */
>>      struct rlist new_fkey;
>> +    /**
>> +     * List of all records that were inserted in system spaces
>> +     * in current statement.
>> +     */
>> +    struct rlist record_list;
>>      bool initiateTTrans; /* Initiate Tarantool transaction */
>>      /** True, if table to be created has AUTOINCREMENT PK. */
>>      bool is_new_table_autoinc;
>> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
>> index 0efc4dd..fc959bd 100644
>> --- a/src/box/sql/vdbe.c
>> +++ b/src/box/sql/vdbe.c
>> @@ -1010,8 +1010,12 @@ case OP_Halt: {
>>      p->pc = pcx;
>>      if (p->rc) {
>>          if (p->rc == SQL_TARANTOOL_ERROR) {
>> - assert(pOp->p4.z != NULL);
>> - box_error_set(__FILE__, __LINE__, pOp->p5, pOp->p4.z);
>> +            if (pOp->p4.z == NULL) {
>> +                assert(! diag_is_empty(diag_get()));
>> +            } else {
>> + box_error_set(__FILE__, __LINE__, pOp->p5,
>> + pOp->p4.z);
>> +            }
>>          } else if (pOp->p5 != 0) {
>>              static const char * const azType[] = { "NOT NULL", 
>> "UNIQUE", "CHECK",
>>        "FOREIGN KEY" };
>> @@ -4318,8 +4322,8 @@ case OP_IdxInsert: {        /* in2 */
>>      break;
>>  }
>>
>> -/* Opcode: SInsert P1 P2 * * P5
>> - * Synopsis: space id = P1, key = r[P2]
>> +/* Opcode: SInsert P1 P2 P3 * P5
>> + * Synopsis: space id = P1, key = r[P3], on error goto P2
>
> It would be better to swap P2 and P3: if you did so, you wouldn’t need to
> explicitly set 0 as second arg since all unspecified args are 
> nullified by default.
> For instance:
>
> sqlite3VdbeAddOp3(v, OP_SInsert, BOX_SPACE_ID, 0, iRecord);
>
> sqlite3VdbeAddOp2(v, OP_SInsert, BOX_SPACE_ID, iRecord);
>
> On the other hand, I see that many opcodes use exactly p2 operand to
> process jump and label jump_to_p2. So, if you don’t want to break this
> convention, you may leave it as is.
I left as it was before review. I think it would be better to don't break
mentioned convention.
>
>> diff --git a/test/sql/drop-table.test.lua b/test/sql/drop-table.test.lua
>> index 9663074..ba47716 100644
>> --- a/test/sql/drop-table.test.lua
>> +++ b/test/sql/drop-table.test.lua
>> @@ -25,3 +25,24 @@ box.sql.execute("INSERT INTO zzzoobar VALUES (111, 
>> 222, 'c3', 444)")
>>
>>  -- Debug
>>  -- require("console").start()
>> +
>> +--
>> +-- gh-3592: segmentation fault when table with error during
>> +-- creation is dropped.
>> +-- We should grant user enough rights to create space, but not
>> +-- enough to create index.
>> +--
>> +box.schema.user.create('tmp')
>> +box.schema.user.grant('tmp', 'create', 'universe')
>> +box.schema.user.grant('tmp', 'write', 'space', '_space')
>> +box.schema.user.grant('tmp', 'write', 'space', '_schema')
>> +box.session.su('tmp')
>> +--
>> +-- Error: user do not have rights to write in box.space._index.
>> +-- Space that was already created should be automatically dropped.
>> +--
>> +box.sql.execute('create table t1 (id int primary key, a int)')
>> +-- Error: no such table.
>> +box.sql.execute('drop table t1')
>> +box.session.su('admin')
>> +box.schema.user.drop('tmp’)
>
> Em, and where are tests for current feature? This test checks only one
> particular case. AFAIU #3592 is only result of absence of cleanup on
> table creation (original issue should be 
> https://github.com/tarantool/tarantool/issues/3499
> but it is marked as ‘doc’. Now your patch changes this behaviour again).
> Anyway, add test cases when CREATE TABLE fails on VDBE execution,
> e.g. half of unique indexes or referencing constraints are created, but
> smth goes wrong and remains of execution are eliminated correctly.
Done.

New patch:

commit f2fcfec99dfb785c5eb4da6846949cdc5f78d22a
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Fri Aug 31 15:50:17 2018 +0300

     sql: clean-up on failed CREATE TABLE

     In case statement "CREATE TABLE ..." fails it can left some
     records in system spaces that shouldn't be there. These records
     won't be left behind after this patch.

     @TarantoolBot document
     Title: Clean up after failure of CREATE TABLE
     Usually CREATE TABLE creates no less than two objects which are
     space and index. If creation of index (or any object after space)
     failed, created space (and other created objects) won't be deleted
     though operation failed. Now these objects will be deleted
     properly.

     Closes #3592

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index a806fb4..6a97ff9 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -55,6 +55,53 @@
  #include "box/tuple_format.h"
  #include "box/coll_id_cache.h"

+/**
+ * Structure that contains information about record that was
+ * inserted into system space.
+ */
+struct saved_record
+{
+    /** A link in a record list. */
+    struct rlist link;
+    /** Id of space in which the record was inserted. */
+    uint32_t space_id;
+    /** First register of the key of the record. */
+    int reg_key;
+    /** Number of registers the key consists of. */
+    int reg_key_count;
+    /** The number of the OP_SInsert operation. */
+    int insertion_opcode;
+};
+
+/**
+ * Save inserted in system space record in list.
+ *
+ * @param parser SQL Parser object.
+ * @param space_id Id of table in which record is inserted.
+ * @param reg_key Register that contains first field of the key.
+ * @param reg_key_count Exact number of fields of the key.
+ * @param insertion_opcode Number of OP_SInsert opcode.
+ */
+static inline void
+save_record(struct Parse *parser, uint32_t space_id, int reg_key,
+        int reg_key_count, int insertion_opcode)
+{
+    struct saved_record *record =
+        region_alloc(&parser->region, sizeof(*record));
+    if (record == NULL) {
+        diag_set(OutOfMemory, sizeof(*record), "region_alloc",
+             "record");
+        parser->rc = SQL_TARANTOOL_ERROR;
+        parser->nErr++;
+        return;
+    }
+    record->space_id = space_id;
+    record->reg_key = reg_key;
+    record->reg_key_count = reg_key_count;
+    record->insertion_opcode = insertion_opcode;
+    rlist_add_entry(&parser->record_list, record, link);
+}
+
  void
  sql_finish_coding(struct Parse *parse_context)
  {
@@ -62,6 +109,40 @@ sql_finish_coding(struct Parse *parse_context)
      struct sqlite3 *db = parse_context->db;
      struct Vdbe *v = sqlite3GetVdbe(parse_context);
      sqlite3VdbeAddOp0(v, OP_Halt);
+    /*
+     * In case statement "CREATE TABLE ..." fails it can
+     * left some records in system spaces that shouldn't be
+     * there. To clean-up properly this code is added. Last
+     * record isn't deleted because if statement fails than
+     * it won't be created. This code works the same way for
+     * other "CREATE ..." statements but it won't delete
+     * anything as these statements create no more than one
+     * record.
+     */
+    if (rlist_empty(&parse_context->record_list) == 0) {
+        struct saved_record *record =
+            rlist_shift_entry(&parse_context->record_list,
+                      struct saved_record, link);
+        /* Set P2 of SInsert. */
+        sqlite3VdbeChangeP2(v, record->insertion_opcode, v->nOp);
+        char *comment = "Delete entry from %s if CREATE TABLE fails";
+        rlist_foreach_entry(record, &parse_context->record_list, link) {
+            int record_reg = ++parse_context->nMem;
+            sqlite3VdbeAddOp3(v, OP_MakeRecord, record->reg_key,
+                      record->reg_key_count, record_reg);
+            sqlite3VdbeAddOp2(v, OP_SDelete, record->space_id,
+                      record_reg);
+            struct space *space = space_by_id(record->space_id);
+            VdbeComment((v, comment, space_name(space)));
+            /* Set P2 of SInsert. */
+            sqlite3VdbeChangeP2(v, record->insertion_opcode,
+                        v->nOp);
+        }
+        sqlite3VdbeAddOp2(v, OP_Halt, SQL_TARANTOOL_ERROR, 0);
+        VdbeComment((v,
+                 "Exit with an error if CREATE statement fails"));
+    }
+
      if (db->mallocFailed || parse_context->nErr != 0) {
          if (parse_context->rc == SQLITE_OK)
              parse_context->rc = SQLITE_ERROR;
@@ -1101,13 +1182,14 @@ vdbe_emit_create_index(struct Parse *parse, 
struct space_def *def,
      sqlite3VdbeAddOp4(v, OP_Blob, index_parts_sz, entry_reg + 5,
                SQL_SUBTYPE_MSGPACK, index_parts, P4_STATIC);
      sqlite3VdbeAddOp3(v, OP_MakeRecord, entry_reg, 6, tuple_reg);
-    sqlite3VdbeAddOp2(v, OP_SInsert, BOX_INDEX_ID, tuple_reg);
+    sqlite3VdbeAddOp3(v, OP_SInsert, BOX_INDEX_ID, 0, tuple_reg);
      /*
       * Non-NULL value means that index has been created via
       * separate CREATE INDEX statement.
       */
      if (idx_def->opts.sql != NULL)
          sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
+    save_record(parse, BOX_INDEX_ID, entry_reg, 2, v->nOp - 1);
      return;
  error:
      parse->rc = SQL_TARANTOOL_ERROR;
@@ -1165,8 +1247,9 @@ createSpace(Parse * pParse, int iSpaceId, char *zStmt)
      sqlite3VdbeAddOp4(v, OP_Blob, table_stmt_sz, iFirstCol + 6,
                SQL_SUBTYPE_MSGPACK, table_stmt, P4_STATIC);
      sqlite3VdbeAddOp3(v, OP_MakeRecord, iFirstCol, 7, iRecord);
-    sqlite3VdbeAddOp2(v, OP_SInsert, BOX_SPACE_ID, iRecord);
+    sqlite3VdbeAddOp3(v, OP_SInsert, BOX_SPACE_ID, 0, iRecord);
      sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
+    save_record(pParse, BOX_SPACE_ID, iFirstCol, 1, v->nOp - 1);
      return;
  error:
      pParse->nErr++;
@@ -1340,9 +1423,11 @@ vdbe_emit_fkey_create(struct Parse 
*parse_context, const struct fkey_def *fk)
                parent_links, P4_DYNAMIC);
      sqlite3VdbeAddOp3(vdbe, OP_MakeRecord, constr_tuple_reg, 9,
                constr_tuple_reg + 9);
-    sqlite3VdbeAddOp2(vdbe, OP_SInsert, BOX_FK_CONSTRAINT_ID,
+    sqlite3VdbeAddOp3(vdbe, OP_SInsert, BOX_FK_CONSTRAINT_ID, 0,
                constr_tuple_reg + 9);
      sqlite3VdbeChangeP5(vdbe, OPFLAG_NCHANGE);
+    save_record(parse_context, BOX_FK_CONSTRAINT_ID, constr_tuple_reg, 2,
+            vdbe->nOp - 1);
      sqlite3ReleaseTempRange(parse_context, constr_tuple_reg, 10);
      return;
  error:
@@ -1487,14 +1572,18 @@ sqlite3EndTable(Parse * pParse,    /* Parse 
context */
          int reg_seq_record =
              emitNewSysSequenceRecord(pParse, reg_seq_id,
                           p->def->name);
-        sqlite3VdbeAddOp2(v, OP_SInsert, BOX_SEQUENCE_ID,
+        sqlite3VdbeAddOp3(v, OP_SInsert, BOX_SEQUENCE_ID, 0,
                    reg_seq_record);
+        save_record(pParse, BOX_SEQUENCE_ID, reg_seq_record + 1, 1,
+                v->nOp - 1);
          /* Do an insertion into _space_sequence. */
          int reg_space_seq_record =
              emitNewSysSpaceSequenceRecord(pParse, reg_space_id,
                                reg_seq_id);
-        sqlite3VdbeAddOp2(v, OP_SInsert, BOX_SPACE_SEQUENCE_ID,
+        sqlite3VdbeAddOp3(v, OP_SInsert, BOX_SPACE_SEQUENCE_ID, 0,
                    reg_space_seq_record);
+        save_record(pParse, BOX_SPACE_SEQUENCE_ID,
+                reg_space_seq_record + 1, 1, v->nOp - 1);
      }
      /* Code creation of FK constraints, if any. */
      struct fkey_parse *fk_parse;
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index e98e845..a4b65eb 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -274,6 +274,7 @@ sql_parser_create(struct Parse *parser, sqlite3 *db)
      memset(parser, 0, sizeof(struct Parse));
      parser->db = db;
      rlist_create(&parser->new_fkey);
+    rlist_create(&parser->record_list);
      region_create(&parser->region, &cord()->slabc);
  }

diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 744b660..d8eb516 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2765,6 +2765,11 @@ struct Parse {
       * Foreign key constraint appeared in CREATE TABLE stmt.
       */
      struct rlist new_fkey;
+    /**
+     * List of all records that were inserted in system spaces
+     * in current statement.
+     */
+    struct rlist record_list;
      bool initiateTTrans;    /* Initiate Tarantool transaction */
      /** True, if table to be created has AUTOINCREMENT PK. */
      bool is_new_table_autoinc;
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 7c1015c..1bec2fa 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1010,8 +1010,12 @@ case OP_Halt: {
      p->pc = pcx;
      if (p->rc) {
          if (p->rc == SQL_TARANTOOL_ERROR) {
-            assert(pOp->p4.z != NULL);
-            box_error_set(__FILE__, __LINE__, pOp->p5, pOp->p4.z);
+            if (pOp->p4.z == NULL) {
+                assert(! diag_is_empty(diag_get()));
+            } else {
+                box_error_set(__FILE__, __LINE__, pOp->p5,
+                          pOp->p4.z);
+            }
          } else if (pOp->p5 != 0) {
              static const char * const azType[] = { "NOT NULL", 
"UNIQUE", "CHECK",
                                     "FOREIGN KEY" };
@@ -4308,8 +4312,8 @@ case OP_IdxInsert: {
      break;
  }

-/* Opcode: SInsert P1 P2 * * P5
- * Synopsis: space id = P1, key = r[P2]
+/* Opcode: SInsert P1 P2 P3 * P5
+ * Synopsis: space id = P1, key = r[P3], on error goto P2
   *
   * This opcode is used only during DDL routine.
   * In contrast to ordinary insertion, insertion to system spaces
@@ -4322,15 +4326,15 @@ case OP_IdxInsert: {
   */
  case OP_SInsert: {
      assert(pOp->p1 > 0);
-    assert(pOp->p2 >= 0);
+    assert(pOp->p2 > 0);
+    assert(pOp->p3 >= 0);

-    pIn2 = &aMem[pOp->p2];
+    pIn3 = &aMem[pOp->p3];
      struct space *space = space_by_id(pOp->p1);
      assert(space != NULL);
      assert(space_is_system(space));
-    rc = tarantoolSqlite3Insert(space, pIn2->z, pIn2->z + pIn2->n);
-    if (rc)
-        goto abort_due_to_error;
+    if (tarantoolSqlite3Insert(space, pIn3->z, pIn3->z + pIn3->n) != 0)
+        goto jump_to_p2;
      if (pOp->p5 & OPFLAG_NCHANGE)
          p->nChange++;
      break;
diff --git a/test/sql/drop-table.result b/test/sql/drop-table.result
index 08f2496..1659604 100644
--- a/test/sql/drop-table.result
+++ b/test/sql/drop-table.result
@@ -33,3 +33,105 @@ box.sql.execute("INSERT INTO zzzoobar VALUES (111, 
222, 'c3', 444)")
  -- DROP TABLE should do the job
  -- Debug
  -- require("console").start()
+--
+-- gh-3592: clean-up garbage on failed CREATE TABLE statement.
+--
+-- Let user have enough rights to create space, but not enough to
+-- create index.
+--
+box.schema.user.create('tmp')
+---
+...
+box.schema.user.grant('tmp', 'create', 'universe')
+---
+...
+box.schema.user.grant('tmp', 'write', 'space', '_space')
+---
+...
+box.schema.user.grant('tmp', 'write', 'space', '_schema')
+---
+...
+-- Number of records in _space, _index, _sequence:
+space_count = #box.space._space:select()
+---
+...
+index_count = #box.space._index:select()
+---
+...
+sequence_count = #box.space._sequence:select()
+---
+...
+box.session.su('tmp')
+---
+...
+--
+-- Error: user do not have rights to write in box.space._index.
+-- Space that was already created should be automatically dropped.
+--
+box.sql.execute('create table t1 (id int primary key, a int)')
+---
+- error: Write access to space '_index' is denied for user 'tmp'
+...
+-- Error: no such table.
+box.sql.execute('drop table t1')
+---
+- error: 'no such table: T1'
+...
+box.session.su('admin')
+---
+...
+--
+-- Check that _space, _index and _sequence have the same number of
+-- records.
+--
+space_count == #box.space._space:select()
+---
+- true
+...
+index_count == #box.space._index:select()
+---
+- true
+...
+sequence_count == #box.space._sequence:select()
+---
+- true
+...
+--
+-- Give user right to write in _index. Still have not enough
+-- rights to write in _sequence.
+--
+box.schema.user.grant('tmp', 'write', 'space', '_index')
+---
+...
+box.session.su('tmp')
+---
+...
+--
+-- Error: user do not have rights to write in _sequence.
+--
+box.sql.execute('create table t2 (id int primary key autoincrement, a 
unique, b unique, c unique, d unique)')
+---
+- error: Write access to space '_sequence' is denied for user 'tmp'
+...
+box.session.su('admin')
+---
+...
+--
+-- Check that _space, _index and _sequence have the same number of
+-- records.
+--
+space_count == #box.space._space:select()
+---
+- true
+...
+index_count == #box.space._index:select()
+---
+- true
+...
+sequence_count == #box.space._sequence:select()
+---
+- true
+...
+box.schema.user.drop('tmp')
+---
+...
diff --git a/test/sql/drop-table.test.lua b/test/sql/drop-table.test.lua
index 9663074..1bc8894 100644
--- a/test/sql/drop-table.test.lua
+++ b/test/sql/drop-table.test.lua
@@ -25,3 +25,62 @@ box.sql.execute("INSERT INTO zzzoobar VALUES (111, 
222, 'c3', 444)")

  -- Debug
  -- require("console").start()
+
+--
+-- gh-3592: clean-up garbage on failed CREATE TABLE statement.
+--
+-- Let user have enough rights to create space, but not enough to
+-- create index.
+--
+box.schema.user.create('tmp')
+box.schema.user.grant('tmp', 'create', 'universe')
+box.schema.user.grant('tmp', 'write', 'space', '_space')
+box.schema.user.grant('tmp', 'write', 'space', '_schema')
+
+-- Number of records in _space, _index, _sequence:
+space_count = #box.space._space:select()
+index_count = #box.space._index:select()
+sequence_count = #box.space._sequence:select()
+
+box.session.su('tmp')
+--
+-- Error: user do not have rights to write in box.space._index.
+-- Space that was already created should be automatically dropped.
+--
+box.sql.execute('create table t1 (id int primary key, a int)')
+-- Error: no such table.
+box.sql.execute('drop table t1')
+
+box.session.su('admin')
+
+--
+-- Check that _space, _index and _sequence have the same number of
+-- records.
+--
+space_count == #box.space._space:select()
+index_count == #box.space._index:select()
+sequence_count == #box.space._sequence:select()
+
+--
+-- Give user right to write in _index. Still have not enough
+-- rights to write in _sequence.
+--
+box.schema.user.grant('tmp', 'write', 'space', '_index')
+box.session.su('tmp')
+
+--
+-- Error: user do not have rights to write in _sequence.
+--
+box.sql.execute('create table t2 (id int primary key autoincrement, a 
unique, b unique, c unique, d unique)')
+
+box.session.su('admin')
+
+--
+-- Check that _space, _index and _sequence have the same number of
+-- records.
+--
+space_count == #box.space._space:select()
+index_count == #box.space._index:select()
+sequence_count == #box.space._sequence:select()
+
+box.schema.user.drop('tmp')


[-- Attachment #2: Type: text/html, Size: 146119 bytes --]

  reply	other threads:[~2018-10-08 19:39 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-07 19:04 [tarantool-patches] " imeevma
2018-09-18 16:18 ` [tarantool-patches] " Vladislav Shpilevoy
2018-09-20 18:02   ` Imeev Mergen
2018-09-20 19:14     ` Vladislav Shpilevoy
2018-09-22  9:21       ` Imeev Mergen
2018-09-24 10:44         ` Vladislav Shpilevoy
2018-10-01 13:05         ` n.pettik
2018-10-08 19:39           ` Imeev Mergen [this message]
2018-10-09 14:15             ` n.pettik
2018-10-10 16:27               ` Imeev Mergen
2018-10-11 15:09                 ` n.pettik
2018-10-11 17:09                   ` Imeev Mergen
2018-10-12 13:50                     ` n.pettik
2018-11-01 14:37 ` 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=51918bf1-14ea-45aa-4e1e-901d2ee9c532@tarantool.org \
    --to=imeevma@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='[tarantool-patches] Re: [PATCH v2 1/1] sql: cleanup on failed creation operation' \
    /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