[tarantool-patches] Re: [PATCH 4/5] sql: display error on FK creation and drop failure

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


Before insertion to _fk_constraint we must be sure that there in no
entry with given <name, child id>. Otherwise, insertion will fail and
'duplicate key' will be shown. Such error message doesn't seem to be
informative enough, so lets manually iterate through whole space looking
for appropriate record.

1. As I know, vdbe_emit_halt_with_presence_test do not iterate through
the whole space. It uses an index to search for the record fast.

You are right, I’ve fixed commit message.


The same is for dropping constraint, but here vice versa: we test
that _fk_contraint contains entry with given name and child id.

2. Typo: _fk_contraint -> _fk_constraint.

Fixed.


It is worth mentioning that during CREATE TABLE processing schema id
changes and check in OP_OpenRead opcode fails (which in turn shows that
pointer to space may expire). On the other hand, _fk_constraint space
itself remains immutable, so as a temporary workaround lets use flag
indicating pointer to system space passed to OP_OpenRead. It makes
possible to use pointer to space, even if schema has changed.
Closes #3271
---
 src/box/errcode.h            |  2 ++
 src/box/sql/build.c          | 43 
+++++++++++++++++++++++++++++++------------
 src/box/sql/sqliteInt.h      | 10 +++++++---
 src/box/sql/trigger.c        | 24 ++++++++++++++++--------
 src/box/sql/vdbe.c           |  3 ++-
 test/box/misc.result         |  2 ++
 test/sql-tap/alter2.test.lua | 25 ++++++++++++++++++++++++-
 7 files changed, 84 insertions(+), 25 deletions(-)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index c2d3cd035..20ace09e4 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1784,6 +1784,20 @@ vdbe_fkey_code_creation(struct Parse *parse_context, 
const struct fkey_def *fk)
             sqlite3VdbeAddOp2(vdbe, OP_Integer, fk->parent_id,
                               constr_tuple_reg + 2);
     }
+    /*
+     * Lets check that constraint with this name hasn't
+     * been created before.
+     */
+    const char *error_msg =
+            tt_sprintf(tnt_errcode_desc(ER_CONSTRAINT_EXISTS), name_copy);
+    if (vdbe_emit_halt_with_presence_test(parse_context,
+                                          BOX_FK_CONSTRAINT_ID, 0,
+                                          constr_tuple_reg, 2,
+                                          ER_CONSTRAINT_EXISTS, error_msg,
+                                          false, OP_NoConflict) != 0) {
+            free((void *) name_copy);

3. Name_copy is allocated on db.

Fixed:

+++ b/src/box/sql/build.c
@@ -1603,7 +1603,7 @@ vdbe_emit_fkey_create(struct Parse *parse_context, const 
struct fkey_def *fk)
                                              constr_tuple_reg, 2,
                                              ER_CONSTRAINT_EXISTS, error_msg,
                                              false, OP_NoConflict) != 0) {
-               free((void *) name_copy);
+               sqlite3DbFree(parse_context->db, (void *) name_copy);

+            return;
+    }
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index b9723e2e7..0f227e637 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3172,7 +3172,8 @@ case OP_OpenWrite:
      * during runtime.
      */
     if (box_schema_version() != p->schema_ver &&
-        (pOp->p5 & OPFLAG_FRESH_PTR) == 0) {
+        (pOp->p5 & OPFLAG_FRESH_PTR) == 0 &&
+        (pOp->p5 & OPFLAG_SYSTEMSP) == 0) {

4. Why not p5 & (FRESH_PTR | SYSTEMSP) ?

Because I am not used to work with bit flags..
This is definitely proper way to do it. Anyway, FRESH_PTR flag has gone,
so now it is again simple (pOp->p5 & OPFLAG_SYSTEMSP) == 0 check.


Other related posts:

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