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 07EE327AF2 for ; Fri, 27 Jul 2018 09:19:45 -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 IjMNJpVQT8ay for ; Fri, 27 Jul 2018 09:19:44 -0400 (EDT) Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (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 8F73627A20 for ; Fri, 27 Jul 2018 09:19:44 -0400 (EDT) Date: Fri, 27 Jul 2018 16:19:42 +0300 From: Kirill Yukhin Subject: [tarantool-patches] Re: [PATCH] sql: remove preupdate hook Message-ID: <20180727131942.6ubuclhzzrmp3yqy@tarantool.org> References: <7D1DA51D-5B70-43C0-BA5D-1E3E497CF327@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <7D1DA51D-5B70-43C0-BA5D-1E3E497CF327@tarantool.org> 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" Cc: tarantool-patches@freelists.org Hello Nikita, On 27 июл 14:40, n.pettik wrote: > I have grepped and found few mentions about preupdate hook > which you didn’t delete. > > Firstly, it seems that you can remove struct PreUpdate. > Secondly, one comment in update.c (line 565): > > * That (regNew==regnewPk+1) is true is also important for the > * pre-update hook. If the caller invokes preupdate_new(), the returned > * value is copied from memory cell (regNewPk+1+iCol), where iCol > * is the column index supplied by the user. > > Remove it as well pls. Fixed. Updated patch below. -- Regards, Kirill Yukhin commit 5d03ba58982a89b8b1eb0f86579e37a213e5b3af Author: Kirill Yukhin Date: Fri Jul 27 14:00:06 2018 +0300 sql: remove preupdate hook SQLITE_ENABLE_PREUPDATE_HOOK is dead macro. Remove it. Part of #2356 diff --git a/src/box/sql/main.c b/src/box/sql/main.c index ded3b5b..4187bb5 100644 --- a/src/box/sql/main.c +++ b/src/box/sql/main.c @@ -1325,26 +1325,6 @@ sqlite3_rollback_hook(sqlite3 * db, /* Attach the hook to this database */ return pRet; } -#ifdef SQLITE_ENABLE_PREUPDATE_HOOK -/* - * Register a callback to be invoked each time a row is updated, - * inserted or deleted using this database connection. - */ -void * -sqlite3_preupdate_hook(sqlite3 * db, /* Attach the hook to this database */ - void (*xCallback) ( /* Callback function */ - void *, sqlite3 *, int, - char const *, sqlite3_int64, sqlite3_int64), - void *pArg) /* First callback argument */ -{ - void *pRet; - pRet = db->pPreUpdateArg; - db->xPreUpdateCallback = xCallback; - db->pPreUpdateArg = pArg; - return pRet; -} -#endif /* SQLITE_ENABLE_PREUPDATE_HOOK */ - /* * Configure an sqlite3_wal_hook() callback to automatically checkpoint * a database after committing a transaction if there are nFrame or diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 5ca91b2..b595984 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -1483,7 +1483,6 @@ typedef struct Lookaside Lookaside; typedef struct LookasideSlot LookasideSlot; typedef struct NameContext NameContext; typedef struct Parse Parse; -typedef struct PreUpdate PreUpdate; typedef struct PrintfArguments PrintfArguments; typedef struct RowSet RowSet; typedef struct Savepoint Savepoint; @@ -1626,14 +1625,6 @@ struct sqlite3 { void *pUpdateArg; void (*xUpdateCallback) (void *, int, const char *, const char *, sqlite_int64); -#ifdef SQLITE_ENABLE_PREUPDATE_HOOK - void *pPreUpdateArg; /* First argument to xPreUpdateCallback */ - void (*xPreUpdateCallback) ( /* Registered using sqlite3_preupdate_hook() */ - void *, sqlite3 *, int, char const *, - char const *, sqlite3_int64, - sqlite3_int64); - PreUpdate *pPreUpdate; /* Context for active pre-update callback */ -#endif /* SQLITE_ENABLE_PREUPDATE_HOOK */ sqlite3_value *pErr; /* Most recent error message */ union { volatile int isInterrupted; /* True if sqlite3_interrupt has been called */ @@ -2956,12 +2947,8 @@ struct Parse { #define OPFLAG_NCHANGE 0x01 /* OP_Insert: Set to update db->nChange */ /* Also used in P2 (not P5) of OP_Delete */ #define OPFLAG_EPHEM 0x01 /* OP_Column: Ephemeral output is ok */ -#define OPFLAG_ISUPDATE 0x04 /* This OP_Insert is an sql UPDATE */ #define OPFLAG_OE_IGNORE 0x200 /* OP_IdxInsert: Ignore flag */ #define OPFLAG_OE_FAIL 0x400 /* OP_IdxInsert: Fail flag */ -#ifdef SQLITE_ENABLE_PREUPDATE_HOOK -#define OPFLAG_ISNOOP 0x40 /* OP_Delete does pre-update-hook only */ -#endif #define OPFLAG_LENGTHARG 0x40 /* OP_Column only used for length() */ #define OPFLAG_TYPEOFARG 0x80 /* OP_Column only used for typeof() */ #define OPFLAG_SEEKEQ 0x02 /* OP_Open** cursor uses EQ seek only */ diff --git a/src/box/sql/update.c b/src/box/sql/update.c index d51a05c..489a456 100644 --- a/src/box/sql/update.c +++ b/src/box/sql/update.c @@ -559,26 +559,12 @@ sqlite3Update(Parse * pParse, /* The parser context */ } /* If changing the PK value, or if there are foreign key constraints - * to process, delete the old record. Otherwise, add a noop OP_Delete - * to invoke the pre-update hook. - * - * That (regNew==regnewPk+1) is true is also important for the - * pre-update hook. If the caller invokes preupdate_new(), the returned - * value is copied from memory cell (regNewPk+1+iCol), where iCol - * is the column index supplied by the user. + * to process, delete the old record. */ assert(regNew == regNewPk + 1); -#ifdef SQLITE_ENABLE_PREUPDATE_HOOK - sqlite3VdbeAddOp3(v, OP_Delete, iDataCur, - OPFLAG_ISUPDATE | - ((hasFK || chngKey - || pPk != 0) ? 0 : OPFLAG_ISNOOP), - regNewRowid); -#else if (hasFK || chngPk || pPk != 0) { sqlite3VdbeAddOp2(v, OP_Delete, iDataCur, 0); } -#endif if (bReplace || chngPk) { sqlite3VdbeJumpHere(v, addr1); } diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index dc22c26..2144d95 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -119,16 +119,6 @@ updateMaxBlobsize(Mem *p) } #endif -/* - * This macro evaluates to true if either the update hook or the preupdate - * hook are enabled for database connect DB. - */ -#ifdef SQLITE_ENABLE_PREUPDATE_HOOK -# define HAS_UPDATE_HOOK(DB) ((DB)->xPreUpdateCallback||(DB)->xUpdateCallback) -#else -# define HAS_UPDATE_HOOK(DB) ((DB)->xUpdateCallback) -#endif - /* * The next global variable is incremented each time the OP_Found opcode * is executed. This is used to test whether or not the foreign key diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h index e5ed94c..a4770a5 100644 --- a/src/box/sql/vdbeInt.h +++ b/src/box/sql/vdbeInt.h @@ -420,24 +420,6 @@ struct Vdbe { #define VDBE_MAGIC_RESET 0x48fa9f76 /* Reset and ready to run again */ #define VDBE_MAGIC_DEAD 0x5606c3c8 /* The VDBE has been deallocated */ -/* - * Structure used to store the context required by the - * sqlite3_preupdate_*() API functions. - */ -struct PreUpdate { - Vdbe *v; - VdbeCursor *pCsr; /* Cursor to read old values from */ - int op; /* One of SQLITE_INSERT, UPDATE, DELETE */ - u8 *aRecord; /* old.* database record */ - UnpackedRecord *pUnpacked; /* Unpacked version of aRecord[] */ - UnpackedRecord *pNewUnpacked; /* Unpacked version of new.* record */ - int iNewReg; /* Register for new.* values */ - i64 iKey1; /* First key value passed to hook */ - i64 iKey2; /* Second key value passed to hook */ - Mem *aNew; /* Array of new.* values */ - Table *pTab; /* Schema object being upated */ -}; - /* * Function prototypes */ diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index f1068d3..d3a91e2 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -1574,26 +1574,6 @@ sqlite3_expanded_sql(sqlite3_stmt * pStmt) #endif } -#ifdef SQLITE_ENABLE_PREUPDATE_HOOK -/* - * This function is designed to be called from within a pre-update callback - * only. It returns zero if the change that caused the callback was made - * immediately by a user SQL statement. Or, if the change was made by a - * trigger program, it returns the number of trigger programs currently - * on the stack (1 for a top-level trigger, 2 for a trigger fired by a - * top-level trigger etc.). - * - * For the purposes of the previous paragraph, a foreign key CASCADE, SET NULL - * or SET DEFAULT action is considered a trigger. - */ -int -sqlite3_preupdate_depth(sqlite3 * db) -{ - PreUpdate *p = db->pPreUpdate; - return (p ? p->v->nFrame : 0); -} -#endif /* SQLITE_ENABLE_PREUPDATE_HOOK */ - #ifdef SQLITE_ENABLE_STMT_SCANSTATUS /* * Return status data for a single loop within query pStmt. diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index c71e0fe..d6ff51c 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -3625,31 +3625,6 @@ sqlite3VdbeSetVarmask(Vdbe * v, int iVar) } } -#ifdef SQLITE_ENABLE_PREUPDATE_HOOK - -/* - * If the second argument is not NULL, release any allocations associated - * with the memory cells in the p->aMem[] array. Also free the UnpackedRecord - * structure itself, using sqlite3DbFree(). - * - * This function is used to free UnpackedRecord structures allocated by - * the vdbeUnpackRecord() function found in vdbeapi.c. - */ -static void -vdbeFreeUnpacked(sqlite3 * db, UnpackedRecord * p) -{ - if (p) { - int i; - for (i = 0; i < p->nField; i++) { - Mem *pMem = &p->aMem[i]; - if (pMem->zMalloc) - sqlite3VdbeMemRelease(pMem); - } - sqlite3DbFree(db, p); - } -} -#endif /* SQLITE_ENABLE_PREUPDATE_HOOK */ - i64 sqlite3VdbeMsgpackRecordLen(Mem * pRec, u32 n) {