From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 804762865C for ; Wed, 10 Oct 2018 10:39:51 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id gLnB7VbBm7Vp for ; Wed, 10 Oct 2018 10:39:51 -0400 (EDT) Received: from smtp63.i.mail.ru (smtp63.i.mail.ru [217.69.128.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 0EEC322310 for ; Wed, 10 Oct 2018 10:39:50 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH 2/2] sql: add test on changes/total_changes References: <7BEF7461-8C36-472F-95D7-6E46CD99E956@tarantool.org> <9C1DDEBD-C37D-4B61-96AC-832BC4DDD12A@tarantool.org> <881CB143-0DC2-468A-90CA-0459E9EE7B15@gmail.com> <2DF5F7A1-C504-4B82-B394-0F16DF025D13@tarantool.org> <75020feb-3d2a-2707-bfdb-fd3fd2229afa@tarantool.org> <02140D0D-8DA6-4B74-B6A0-38572F63C745@tarantool.org> From: Alex Khatskevich Message-ID: <72b1a158-ef2d-50f6-2ad5-5ffcf598b4c1@tarantool.org> Date: Wed, 10 Oct 2018 17:39:48 +0300 MIME-Version: 1.0 In-Reply-To: <02140D0D-8DA6-4B74-B6A0-38572F63C745@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-US Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: "n.pettik" , tarantool-patches@freelists.org >> commit 4b72b445851fb8c14313f2ed15b87b1c6b792b13 >> Author: AKhatskevich >> Date: Thu Sep 27 15:28:52 2018 +0300 >> >> sql: total_changes add test && change behavior > Nit: total_changes and what? Don’t try to fit everything > in title, you can extend it with details in commit message. > I would call it simply like: > > sql: restore total_changes after rollback > > Change of user-observable behaviour implies that tests must > be included in patch. Current message describe commit internals better, I suppose. >> This commit relives changes/total_changes from the Sqlite. > Nit: according to Oxford dictionary ‘relive' is > "Live through (an experience or feeling, especially an unpleasant > one) again in one's imagination or memory.” So I guess “resurrect” > or “repair" would be much more suitable here. > *sorry for being too picky* changed to repair >> Key points: >> * Add tests. >> * Change rollback behavior. In case an operation is not applied, >> the total_changes counter is decreased back. >> >> Closes #2181 > I found that some cases still seem to be invalid in their behaviour: > > CREATE TABLE t1 (id INT PRIMARY KEY); > INSERT OR FAIL VALUES (1), (1), (1); > SELECT changes(); > --- > - - [1] > ... > > > SELECT total_changes(); > --- > - - [4] > ... > > Total changes counted is incremented by 3, but only one insertion > was successful. > *I wouldn’t notice it if you didn’t start to fix IGNORE conflict action* Checked this case. Works well (1 instead of 4) > Next example: > > tarantool> box.sql.execute('start transaction') box.sql.execute('insert into t1 values(3), (4),(5)') box.sql.execute('rollback') > --- > ... > > tarantool> box.sql.execute('select changes()') > --- > - - [3] > ... > > tarantool> box.sql.execute('select * from t1') > --- > - [] > ... > > Why does savepoint rollback set changes to negative value, but regular rollback > doesn’t even nullify it? Total changes counter is also incremented by 3. After the last patch update, zero would be returned instead of 3. > Also, original doc concerning total_changes (https://www.sqlite.org/c3ref/total_changes.html) > says that changes provided by INSTEAD OF triggers are not counted. > Add test on this case pls as well pls. > > What is more, I guess we need way to reset this counter: originally (in SQLite) this > counter is reset when connection is closed. In our implementation total_changes() > turns out to be global and never reset. We can implement it as a pragma for instance. I suggest do that on the first request from a user, not before. >> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h >> index 53188e74d..e3c2041da 100644 >> --- a/src/box/sql/sqliteInt.h >> +++ b/src/box/sql/sqliteInt.h >> @@ -1763,6 +1763,7 @@ struct Savepoint { >> box_txn_savepoint_t *tnt_savepoint; /* Tarantool's savepoint struct */ >> char *zName; /* Savepoint name (nul-terminated) */ >> Savepoint *pNext; /* Parent savepoint (if any) */ >> + int n_change; /* Changes made since "START TRANSACTOIN" */ > Still one of problems with counter is following: if changes come from non-SQL-land, > they won’t be taken into consideration. Personally, I don’t think that it is sort of > ‘incorrect’ behaviour, but it definitely should be mentioned in docs. Hence, create > doc-bot request and briefly describe this feature and how it works. > > I spotted a lot of codestyle violations. I put diff at the end of letter. > >> /* >> @@ -4305,7 +4306,7 @@ Expr *sqlite3ExprAddCollateToken(Parse * pParse, Expr *, const Token *, int); >> Expr *sqlite3ExprAddCollateString(Parse *, Expr *, const char *); >> Expr *sqlite3ExprSkipCollate(Expr *); >> int sqlite3CheckIdentifierName(Parse *, char *); >> -void sqlite3VdbeSetChanges(sqlite3 *, int); >> +void sqlite3VdbeSetChanges(struct Vdbe *); >> int sqlite3AddInt64(i64 *, i64); >> int sqlite3SubInt64(i64 *, i64); >> int sqlite3MulInt64(i64 *, i64); >> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c >> index 00ffb0c5d..30f460216 100644 >> --- a/src/box/sql/vdbe.c >> +++ b/src/box/sql/vdbe.c >> @@ -988,7 +988,7 @@ case OP_Halt: { >> pFrame = p->pFrame; >> p->pFrame = pFrame->pParent; >> p->nFrame--; >> - sqlite3VdbeSetChanges(db, p->nChange); >> + sqlite3VdbeSetChanges(p); >> pcx = sqlite3VdbeFrameRestore(pFrame); >> if (pOp->p2 == ON_CONFLICT_ACTION_IGNORE) { >> /* Instruction pcx is the OP_Program that invoked the sub-program >> @@ -2881,7 +2881,7 @@ case OP_Savepoint: { >> >> if (p1==SAVEPOINT_BEGIN) { >> /* Create a new savepoint structure. */ >> - pNew = sql_savepoint(p, zName); >> + pNew = sql_savepoint(p, zName, psql_txn->n_change); >> /* Link the new savepoint into the database handle's list. */ >> pNew->pNext = psql_txn->pSavepoint; >> psql_txn->pSavepoint = pNew; >> @@ -2930,6 +2930,16 @@ case OP_Savepoint: { >> * have to destroy them >> */ >> } >> + if (p1 == SAVEPOINT_ROLLBACK) { >> + /* >> + * Set nChanges right there, because savepoint >> + * statements do not call halt. >> + */ >> + assert(psql_txn->pSavepoint); >> + p->nChange = psql_txn->pSavepoint->n_change - >> + psql_txn->n_change; >> + sqlite3VdbeSetChanges(p); >> + } >> >> /* If it is a RELEASE, then destroy the savepoint being operated on >> * too. If it is a ROLLBACK TO, then set the number of deferred >> @@ -3019,6 +3029,8 @@ case OP_TransactionRollback: { >> rc = SQL_TARANTOOL_ERROR; >> goto abort_due_to_error; >> } >> + p->nChange = -p->psql_txn->n_change; > I don’t like that changes counter can take negative values - it may be quite > misleading. Lets avoid this behaviour, i.e. make sure that changes always >= 0. Done, by incrementing total_changes only on a commit. >> + sqlite3VdbeSetChanges(p); >> } else { >> sqlite3VdbeError(p, "cannot rollback - no " >> "transaction is active"); >> @@ -3045,7 +3057,9 @@ case OP_TTransaction: { >> goto abort_due_to_error; >> } >> } else { >> - p->anonymous_savepoint = sql_savepoint(p, NULL); >> + assert(p->psql_txn); > Nit: we use explicit != NULL check. I fixed almost all such places > in the diff - see at the end of letter. > >> diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h >> index ce97f4984..72cd45ab4 100644 >> --- a/src/box/sql/vdbeInt.h >> +++ b/src/box/sql/vdbeInt.h >> @@ -442,7 +442,8 @@ int >> sql_txn_begin(Vdbe *p); >> Savepoint * >> sql_savepoint(Vdbe *p, >> - const char *zName); >> + const char *zName, >> + int n_change); > Looks awful. I’ve fixed that. > >> /* >> diff --git a/src/box/txn.h b/src/box/txn.h >> index e74c14d40..137cc6491 100644 >> --- a/src/box/txn.h >> +++ b/src/box/txn.h >> @@ -112,6 +112,8 @@ extern double too_long_threshold; >> struct sql_txn { >> /** List of active SQL savepoints. */ >> struct Savepoint *pSavepoint; >> + /** Changes made in that transaction */ >> + int n_change; > Explain pls, why do you need to store changes in both - transaction structure and > SQL savepoint? AFAIK local changes (i.e. simple changes()) are stored in VDBE (p->nChange); > total changes are stored in db; so, to rollback to previous counter value you need only to > store within save point this value. There could be several vdbes between two savepoints. psql_txn accumulates them. >> diff --git a/test/sql-tap/gh2181-changes-statistics.test.lua b/test/sql-tap/gh2181-changes-statistics.test.lua >> new file mode 100755 >> index 000000000..7fae52cee >> --- /dev/null >> +++ b/test/sql-tap/gh2181-changes-statistics.test.lua >> @@ -0,0 +1,125 @@ >> +#!/usr/bin/env tarantool >> +test = require("sqltester") >> +test:plan(79) >> + >> +test:do_select_tests( >> + "gh2181-changes-statistics", >> + { >> + {"0.1", "SELECT CHANGES()", {0}}, >> + {"0.2", "SELECT TOTAL_CHANGES()", {0}}, >> + {"1.0", "CREATE TABLE T1(A PRIMARY KEY)", {}}, >> + {"1.1", "SELECT CHANGES()", {1}}, >> + {"1.2", "SELECT TOTAL_CHANGES()", {1}}, >> + {"2.0", "INSERT INTO T1 VALUES(1)", {}}, >> + {"2.1", "SELECT CHANGES()", {1}}, >> + {"2.2", "SELECT TOTAL_CHANGES()", {2}}, >> + {"3.0", "INSERT INTO T1 VALUES(2)", {}}, >> + {"3.1", "SELECT CHANGES()", {1}}, >> + {"3.2", "SELECT TOTAL_CHANGES()", {3}}, >> + {"4.0", "CREATE TABLE T2(A PRIMARY KEY)", {}}, >> + {"4.1", "SELECT CHANGES()", {1}}, >> + {"4.2", "SELECT TOTAL_CHANGES()", {4}}, >> + {"5.0", "INSERT INTO T2 SELECT * FROM T1", {}}, >> + {"5.1", "SELECT CHANGES()", {2}}, >> + {"5.2", "SELECT TOTAL_CHANGES()", {6}}, >> + {"6.0", [[ >> + CREATE TRIGGER TRIG1 AFTER INSERT ON T1 FOR EACH ROW BEGIN >> + INSERT INTO T2 VALUES(NEW.A); >> + END; >> + ]],{}}, >> + {"6.1", "SELECT CHANGES()", {1}}, >> + {"6.2", "SELECT TOTAL_CHANGES()", {7}}, >> + {"6.3", "INSERT INTO T1 VALUES(3)", {}}, >> + -- 1 instead of 2; sqlite is the same. > AFAIR we came to agreement to avoid mentions about SQLite in our sources… > Moreover, comments says ‘1 instead of 2’ and local change counter is really 1. > On the other hand, total changes have been incremented by 2: 7 -> 9. > I expect that total changes should be addictive in its measure: > total changes = changes_1 + changes_2 + … > At least you should describe this behaviour in docs. > > My diff is below. It consists of only cosmetic changes, > so I strongly recommend to apply it :) applied Full diff: commit 502b1b09083a300bc79f06ea1d1d64b6c686fe6f Author: AKhatskevich Date:   Thu Sep 27 15:28:52 2018 +0300     sql: total_changes add test && change behavior     This commit repair changes/total_changes.     Key points:     * Add tests.     * total_changes is incremented only on commit.     * Increment changes only in case operation succeeds.     Closes #2181     @TarantoolBot document     Title: sql changes/total_changes behavior     changes/total_changes works similarly to sqlite.     `changes`:     * shows number of changed by the last statement rows     * updated after each statement     * doesn't include changes made by triggers     `total_changes`:     * shows number of changed rows since db was started     * updates on commit (or auto-commit)     * include changes made by triggers     Notes:     * Changes which are counted: delete, update, insert.     * Those statistics are not persisted.     * Changes made outside of sql are not counted.     Usage example:     ```     <>     <>     <>     select changes(); -- returns 2     select total_changes(); -- returns 3     ``` diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 53188e74d..af3ca3c57 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -1763,6 +1763,8 @@ struct Savepoint {      box_txn_savepoint_t *tnt_savepoint; /* Tarantool's savepoint struct */      char *zName;        /* Savepoint name (nul-terminated) */      Savepoint *pNext;    /* Parent savepoint (if any) */ +    /** Changes to db made since last START TRANSACTION. */ +    int change_count;  };  /* @@ -4305,7 +4307,17 @@ Expr *sqlite3ExprAddCollateToken(Parse * pParse, Expr *, const Token *, int);  Expr *sqlite3ExprAddCollateString(Parse *, Expr *, const char *);  Expr *sqlite3ExprSkipCollate(Expr *);  int sqlite3CheckIdentifierName(Parse *, char *); -void sqlite3VdbeSetChanges(sqlite3 *, int); + +/** + * This routine sets the value to be returned by subsequent + * calls to changes() and total_changes(). + * + * @param v Active Vdbe. + * @param transaction_end Indicator for total_changes update. + */ +void +vdbe_changes_update(struct Vdbe *v, bool transaction_end); +  int sqlite3AddInt64(i64 *, i64);  int sqlite3SubInt64(i64 *, i64);  int sqlite3MulInt64(i64 *, i64); diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 00ffb0c5d..56c23e936 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -988,7 +988,7 @@ case OP_Halt: {          pFrame = p->pFrame;          p->pFrame = pFrame->pParent;          p->nFrame--; -        sqlite3VdbeSetChanges(db, p->nChange); +        vdbe_changes_update(p, false);          pcx = sqlite3VdbeFrameRestore(pFrame);          if (pOp->p2 == ON_CONFLICT_ACTION_IGNORE) {              /* Instruction pcx is the OP_Program that invoked the sub-program @@ -2881,7 +2881,7 @@ case OP_Savepoint: {      if (p1==SAVEPOINT_BEGIN) {          /* Create a new savepoint structure. */ -        pNew = sql_savepoint(p, zName); +        pNew = sql_savepoint_create(p, zName, psql_txn->change_count);          /* Link the new savepoint into the database handle's list. */          pNew->pNext = psql_txn->pSavepoint;          psql_txn->pSavepoint = pNew; @@ -2930,6 +2930,16 @@ case OP_Savepoint: {                   * have to destroy them                   */              } +            if (p1 == SAVEPOINT_ROLLBACK) { +                /* +                * Set nChanges right there, +                * because savepoint statements +                * do not call halt. +                 */ +                assert(psql_txn->pSavepoint != NULL); +                psql_txn->change_count = +                    psql_txn->pSavepoint->change_count; +            }              /* If it is a RELEASE, then destroy the savepoint being operated on               * too. If it is a ROLLBACK TO, then set the number of deferred @@ -3000,6 +3010,7 @@ case OP_TransactionCommit: {              rc = SQL_TARANTOOL_ERROR;              goto abort_due_to_error;          } +        vdbe_changes_update(p, true);      } else {          sqlite3VdbeError(p, "cannot commit - no transaction is active");          rc = SQLITE_ERROR; @@ -3019,6 +3030,7 @@ case OP_TransactionRollback: {              rc = SQL_TARANTOOL_ERROR;              goto abort_due_to_error;          } +        vdbe_changes_update(p, false);      } else {          sqlite3VdbeError(p, "cannot rollback - no "                      "transaction is active"); @@ -3045,7 +3057,10 @@ case OP_TTransaction: {              goto abort_due_to_error;          }      } else { -        p->anonymous_savepoint = sql_savepoint(p, NULL); +        assert(p->psql_txn); +        int new_changes = p->psql_txn->change_count; +        p->anonymous_savepoint = sql_savepoint_create(p, NULL, +                                 new_changes);          if (p->anonymous_savepoint == NULL) {              rc = SQL_TARANTOOL_ERROR;              goto abort_due_to_error; @@ -3836,7 +3851,7 @@ case OP_Delete: {   * This is used by trigger programs.   */  case OP_ResetCount: { -    sqlite3VdbeSetChanges(db, p->nChange); +    vdbe_changes_update(p, true);      p->nChange = 0;      p->ignoreRaised = 0;      break; @@ -4277,7 +4292,6 @@ case OP_IdxInsert: {        /* in2 */      assert(isSorter(pC)==(pOp->opcode==OP_SorterInsert));      pIn2 = &aMem[pOp->p2];      assert(pIn2->flags & MEM_Blob); -    if (pOp->p5 & OPFLAG_NCHANGE) p->nChange++;      assert(pC->eCurType==CURTYPE_TARANTOOL || pOp->opcode==OP_SorterInsert);      rc = ExpandBlob(pIn2);      if (rc) goto abort_due_to_error; @@ -4307,6 +4321,9 @@ case OP_IdxInsert: {        /* in2 */      }      if (pOp->p5 & OPFLAG_OE_IGNORE) { +        /* Compensate nChange increment. */ +        if (rc != SQLITE_OK) +            p->nChange--;          /* Ignore any kind of failes and do not raise error message */          rc = SQLITE_OK;          /* If we are in trigger, increment ignore raised counter */ @@ -4319,6 +4336,8 @@ case OP_IdxInsert: {        /* in2 */          p->errorAction = ON_CONFLICT_ACTION_ROLLBACK;      }      if (rc) goto abort_due_to_error; +    if ((pOp->p5 & OPFLAG_NCHANGE) != 0) +        p->nChange++;      break;  } diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h index ce97f4984..cbdb8d584 100644 --- a/src/box/sql/vdbeInt.h +++ b/src/box/sql/vdbeInt.h @@ -440,9 +440,10 @@ int sqlite3VdbeExec(Vdbe *);  int sqlite3VdbeList(Vdbe *);  int  sql_txn_begin(Vdbe *p); -Savepoint * -sql_savepoint(Vdbe *p, -          const char *zName); + +struct Savepoint * +sql_savepoint_create(struct Vdbe *p, const char *name, int change_count); +  int sqlite3VdbeHalt(Vdbe *);  int sqlite3VdbeMemTooBig(Mem *);  int sqlite3VdbeMemCopy(Mem *, const Mem *); diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 54edf1b03..284e1deec 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -85,6 +85,7 @@ sql_alloc_txn()          return NULL;      }      txn->pSavepoint = NULL; +    txn->change_count = 0;      txn->fk_deferred_count = 0;      return txn;  } @@ -2270,30 +2271,28 @@ sql_txn_begin(Vdbe *p)      return 0;  } -Savepoint * -sql_savepoint(Vdbe *p, const char *zName) +struct Savepoint * +sql_savepoint_create(struct Vdbe *p, const char *name, int change_count)  { -    assert(p); -    size_t nName = zName ? strlen(zName) + 1 : 0; -    size_t savepoint_sz = sizeof(Savepoint) + nName; -    Savepoint *pNew; - -    pNew = (Savepoint *)region_aligned_alloc(&fiber()->gc, -                         savepoint_sz, -                         alignof(Savepoint)); -    if (pNew == NULL) { -        diag_set(OutOfMemory, savepoint_sz, "region", -             "savepoint"); +    assert(p != NULL); +    size_t name_len = name != NULL ? strlen(name) + 1 : 0; +    size_t savepoint_sz = sizeof(struct Savepoint) + name_len; +    struct Savepoint *savepoint = +        (struct Savepoint *) region_alloc(&fiber()->gc, savepoint_sz); +    if (savepoint == NULL) { +        diag_set(OutOfMemory, savepoint_sz, "region", "savepoint");          return NULL;      } -    pNew->tnt_savepoint = box_txn_savepoint(); -    if (!pNew->tnt_savepoint) +    savepoint->tnt_savepoint = box_txn_savepoint(); +    if (savepoint->tnt_savepoint == NULL)          return NULL; -    if (zName) { -        pNew->zName = (char *)&pNew[1]; -        memcpy(pNew->zName, zName, nName); -    }; -    return pNew; +    savepoint->change_count = change_count; +    if (name != NULL) { +        /* Name is placed right after structure itself. */ +        savepoint->zName = (char *) &savepoint[1]; +        memcpy(savepoint->zName, name, name_len); +    } +    return savepoint;  }  /* @@ -2446,7 +2445,8 @@ sqlite3VdbeHalt(Vdbe * p)                  closeCursorsAndFree(p);                  sqlite3RollbackAll(p);                  sqlite3CloseSavepoints(p); -                p->nChange = 0; +                assert(p->psql_txn); +                p->nChange = -p->psql_txn->change_count;              }          } @@ -2477,12 +2477,11 @@ sqlite3VdbeHalt(Vdbe * p)           * has been rolled back, update the database connection change-counter.           */          if (p->changeCntOn) { -            if (eStatementOp != SAVEPOINT_ROLLBACK) { -                sqlite3VdbeSetChanges(db, p->nChange); -            } else { -                sqlite3VdbeSetChanges(db, 0); +            if (eStatementOp == SAVEPOINT_ROLLBACK) { +                p->nChange = 0; +                p->psql_txn->change_count = 0;              } -            p->nChange = 0; +            vdbe_changes_update(p, p->auto_commit);          }      } @@ -3439,15 +3438,23 @@ sqlite3MemCompare(const Mem * pMem1, const Mem * pMem2, const struct coll * pCol      return sqlite3BlobCompare(pMem1, pMem2);  } -/* - * This routine sets the value to be returned by subsequent calls to - * sqlite3_changes() on the database handle 'db'. - */  void -sqlite3VdbeSetChanges(sqlite3 * db, int nChange) +vdbe_changes_update(struct Vdbe *p, bool transaction_end)  { -    db->nChange = nChange; -    db->nTotalChange += nChange; +    struct sqlite3 *db = p->db; +    struct sql_txn *psql_txn = p->psql_txn; +    db->nChange = p->nChange; +    if (transaction_end) { +        db->nTotalChange += p->nChange; +        if (psql_txn != NULL) { +            db->nTotalChange += psql_txn->change_count; +            psql_txn->change_count = 0; +        } +    } else { +        if (psql_txn != NULL) +            psql_txn->change_count += p->nChange; +    } +    p->nChange = 0;  }  /* diff --git a/src/box/txn.h b/src/box/txn.h index e74c14d40..4f4c86347 100644 --- a/src/box/txn.h +++ b/src/box/txn.h @@ -112,6 +112,12 @@ extern double too_long_threshold;  struct sql_txn {      /** List of active SQL savepoints. */      struct Savepoint *pSavepoint; +    /** +    * Changes have been made in current transaction. +    * It is needed to handle outer SQL function +    * total_changes(). +    */ +    int change_count;      /**       * This variables transfer deferred constraints from one       * VDBE to the next in the same transaction. diff --git a/test/sql-tap/gh2181-changes-statistics.test.lua b/test/sql-tap/gh2181-changes-statistics.test.lua new file mode 100755 index 000000000..14f0999ae --- /dev/null +++ b/test/sql-tap/gh2181-changes-statistics.test.lua @@ -0,0 +1,125 @@ +#!/usr/bin/env tarantool +test = require("sqltester") +test:plan(79) + +test:do_select_tests( +    "gh2181-changes-statistics", +    { +        {"0.1", "SELECT CHANGES()", {0}}, +        {"0.2", "SELECT TOTAL_CHANGES()", {0}}, +        {"1.0", "CREATE TABLE T1(A PRIMARY KEY)", {}}, +        {"1.1", "SELECT CHANGES()", {1}}, +        {"1.2", "SELECT TOTAL_CHANGES()", {1}}, +        {"2.0", "INSERT INTO T1 VALUES(1)", {}}, +        {"2.1", "SELECT CHANGES()", {1}}, +        {"2.2", "SELECT TOTAL_CHANGES()", {2}}, +        {"3.0", "INSERT INTO T1 VALUES(2)", {}}, +        {"3.1", "SELECT CHANGES()", {1}}, +        {"3.2", "SELECT TOTAL_CHANGES()", {3}}, +        {"4.0", "CREATE TABLE T2(A PRIMARY KEY)", {}}, +        {"4.1", "SELECT CHANGES()", {1}}, +        {"4.2", "SELECT TOTAL_CHANGES()", {4}}, +        {"5.0", "INSERT INTO T2 SELECT * FROM T1", {}}, +        {"5.1", "SELECT CHANGES()", {2}}, +        {"5.2", "SELECT TOTAL_CHANGES()", {6}}, +        {"6.0", [[ +            CREATE TRIGGER TRIG1 AFTER INSERT ON T1 FOR EACH ROW BEGIN +                INSERT INTO T2 VALUES(NEW.A); +            END; +            ]],{}}, +        {"6.1", "SELECT CHANGES()", {1}}, +        {"6.2", "SELECT TOTAL_CHANGES()", {7}}, +        {"6.3", "INSERT INTO T1 VALUES(3)", {}}, +        -- 1 instead of 2; sqlite is the same. +        {"6.4", "SELECT CHANGES()", {1}}, +        {"6.5", "SELECT TOTAL_CHANGES()", {9}}, +        {"6.6", "DROP TRIGGER TRIG1"}, +        {"6.7", "SELECT CHANGES()", {1}}, +        {"6.8", "SELECT TOTAL_CHANGES()", {10}}, +        {"7.0", "DELETE FROM T1 WHERE A = 1", {}}, +        {"7.1", "SELECT CHANGES()", {1}}, +        {"7.2", "SELECT TOTAL_CHANGES()", {11}}, +        {"8.0", "UPDATE T1 SET A = 1 where A = 2", {}}, +        {"8.1", "SELECT CHANGES()", {1}}, +        {"8.2", "SELECT TOTAL_CHANGES()", {12}}, +        {"9.0", "SELECT COUNT(*) FROM T2", {3}}, +        {"9.1", "UPDATE T2 SET A = A + 3", {}}, +        -- Inserts to an ephemeral space are not counted. +        {"9.2", "SELECT CHANGES()", {3}}, +        {"9.3", "SELECT TOTAL_CHANGES()", {15}}, +        {"11.0", "DELETE FROM T1", {}}, +        {"11.1", "SELECT CHANGES()", {0}}, +        {"11.2", "SELECT TOTAL_CHANGES()", {15}}, +        {"12.0", "DELETE FROM T2 WHERE A < 100", {}}, +        {"12.1", "SELECT CHANGES()", {3}}, +        {"12.2", "SELECT TOTAL_CHANGES()", {18}}, +        -- Transactions/savepoints. +        {"13.0", "START TRANSACTION", {}}, +        {"13.1", "INSERT INTO T1 VALUES(11)", {}}, +        {"13.2", "SELECT CHANGES()", {1}}, +        {"13.3", "SELECT TOTAL_CHANGES()", {18}}, +        {"13.4", "SAVEPOINT S1", {}}, +        {"13.5", "INSERT INTO T1 VALUES(12)", {}}, +        {"13.6", "SELECT CHANGES()", {1}}, +        {"13.7", "SELECT TOTAL_CHANGES()", {18}}, +        {"13.8", "ROLLBACK TO SAVEPOINT S1", {}}, +        {"13.9", "SELECT CHANGES()", {1}}, +        {"13.10", "SELECT TOTAL_CHANGES()", {18}}, +        {"13.11", "COMMIT", {}}, +        {"13.12", "SELECT CHANGES()", {0}}, +        {"13.13", "SELECT TOTAL_CHANGES()", {19}}, +        {"14.0", "START TRANSACTION", {}}, +        {"14.1", "INSERT INTO T1 VALUES(15)", {}}, +        {"14.2", "SELECT CHANGES()", {1}}, +        {"14.3", "SELECT TOTAL_CHANGES()", {19}}, +        {"14.4", "ROLLBACK", {}}, +        {"14.5", "SELECT CHANGES()", {0}}, +        {"14.6", "SELECT TOTAL_CHANGES()", {19}}, +        {"15.0.1", "INSERT INTO T1 VALUES(1)", {}}, +        {"15.0.2", "SELECT TOTAL_CHANGES()", {20}}, +    }) + +test:do_catchsql_test( +    "gh2181-changes-statistics-15.1", +    -- Rollback during autocommit. +    "INSERT INTO T1 VALUES(0),(1)", +    {1, "Duplicate key exists in unique index 'pk_unnamed_T1_1' in space 'T1'"}) + +test:do_select_tests( +    "gh2181-changes-statistics", +    { +        {"15.2", "SELECT CHANGES()", {0}}, +        {"15.3", "SELECT TOTAL_CHANGES()", {20}}, +        {"16.0", "START TRANSACTION", {}}, +        {"16.1", "INSERT INTO T1 VALUES(21)", {}}, +    }) + +test:do_catchsql_test( +    "gh2181-changes-statistics-16.2", +    -- Rollback during autocommit. +    "INSERT OR ROLLBACK INTO T1 VALUES(21)", +    {1, "Duplicate key exists in unique index 'pk_unnamed_T1_1' in space 'T1'"}) + +test:do_select_tests( +    "gh2181-changes-statistics", +    { +        {"16.3", "SELECT CHANGES()", {-1}}, +        {"16.4", "SELECT TOTAL_CHANGES()", {20}}, +    }) + +test:do_catchsql_test( +    "gh2181-changes-statistics-17.1", +    "INSERT OR FAIL INTO T1 VALUES(31), (31)", +    {1, "Duplicate key exists in unique index 'pk_unnamed_T1_1' in space 'T1'"}) + +test:do_select_tests( +    "gh2181-changes-statistics", +    { +        {"17.2", "SELECT CHANGES()", {1}}, +        {"17.3", "SELECT TOTAL_CHANGES()", {21}}, +        {"18.1", "INSERT OR IGNORE INTO T1 VALUES(32), (32)", {}}, +        {"18.2", "SELECT CHANGES()", {1}}, +        {"18.3", "SELECT TOTAL_CHANGES()", {22}}, +    }) + +test:finish_test()