From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp43.i.mail.ru (smtp43.i.mail.ru [94.100.177.103]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 914E546970E for ; Tue, 24 Dec 2019 14:35:57 +0300 (MSK) Date: Tue, 24 Dec 2019 14:35:56 +0300 From: Sergey Ostanevich Message-ID: <20191224113556.GC19594@tarantool.org> References: <13cbf2fe407c9cf838d7796496201535bbbbd12b.1576844632.git.korablev@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <13cbf2fe407c9cf838d7796496201535bbbbd12b.1576844632.git.korablev@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v3 02/20] sql: refactor sql_prepare() and sqlPrepare() List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the patch, LGTM. Sergos On 20 Dec 15:47, Nikita Pettik wrote: > - Removed saveSqlFlag as argument from sqlPrepare(). It was used to > indicate that its caller is sql_prepare_v2() not sql_prepare(). > Since in previous commit we've left only one version of this function > let's remove this flag at all. > > - Removed struct db from list of sql_prepare() arguments. There's one > global database handler and it can be obtained by sql_get() call. > Hence, it makes no sense to pass around this argument. > > Needed for #3292 > --- > src/box/execute.c | 3 +-- > src/box/sql/analyze.c | 16 +++++++--------- > src/box/sql/legacy.c | 2 +- > src/box/sql/prepare.c | 10 ++++------ > src/box/sql/sqlInt.h | 3 +-- > src/box/sql/vdbe.h | 2 +- > src/box/sql/vdbeInt.h | 1 - > src/box/sql/vdbeapi.c | 2 +- > src/box/sql/vdbeaux.c | 5 +---- > 9 files changed, 17 insertions(+), 27 deletions(-) > > diff --git a/src/box/execute.c b/src/box/execute.c > index 130a3f675..0b21386b5 100644 > --- a/src/box/execute.c > +++ b/src/box/execute.c > @@ -442,8 +442,7 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind, > struct region *region) > { > struct sql_stmt *stmt; > - struct sql *db = sql_get(); > - if (sql_prepare(db, sql, len, &stmt, NULL) != 0) > + if (sql_prepare(sql, len, &stmt, NULL) != 0) > return -1; > assert(stmt != NULL); > port_sql_create(port, stmt); > diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c > index b9858c8d6..e43011dd0 100644 > --- a/src/box/sql/analyze.c > +++ b/src/box/sql/analyze.c > @@ -1336,14 +1336,13 @@ sample_compare(const void *a, const void *b, void *arg) > * statistics (i.e. arrays lt, dt, dlt and avg_eq). 'load' query > * is needed for > * > - * @param db Database handler. > * @param sql_select_prepare SELECT statement, see above. > * @param sql_select_load SELECT statement, see above. > * @param[out] stats Statistics is saved here. > * @retval 0 on success, -1 otherwise. > */ > static int > -load_stat_from_space(struct sql *db, const char *sql_select_prepare, > +load_stat_from_space(const char *sql_select_prepare, > const char *sql_select_load, struct index_stat *stats) > { > struct index **indexes = NULL; > @@ -1359,7 +1358,7 @@ load_stat_from_space(struct sql *db, const char *sql_select_prepare, > } > } > sql_stmt *stmt = NULL; > - int rc = sql_prepare(db, sql_select_prepare, -1, &stmt, 0); > + int rc = sql_prepare(sql_select_prepare, -1, &stmt, 0); > if (rc) > goto finalize; > uint32_t current_idx_count = 0; > @@ -1427,7 +1426,7 @@ load_stat_from_space(struct sql *db, const char *sql_select_prepare, > rc = sql_finalize(stmt); > if (rc) > goto finalize; > - rc = sql_prepare(db, sql_select_load, -1, &stmt, 0); > + rc = sql_prepare(sql_select_load, -1, &stmt, 0); > if (rc) > goto finalize; > struct index *prev_index = NULL; > @@ -1505,12 +1504,11 @@ load_stat_from_space(struct sql *db, const char *sql_select_prepare, > } > > static int > -load_stat_to_index(struct sql *db, const char *sql_select_load, > - struct index_stat **stats) > +load_stat_to_index(const char *sql_select_load, struct index_stat **stats) > { > assert(stats != NULL && *stats != NULL); > struct sql_stmt *stmt = NULL; > - if (sql_prepare(db, sql_select_load, -1, &stmt, 0) != 0) > + if (sql_prepare(sql_select_load, -1, &stmt, 0) != 0) > return -1; > uint32_t current_idx_count = 0; > while (sql_step(stmt) == SQL_ROW) { > @@ -1696,7 +1694,7 @@ sql_analysis_load(struct sql *db) > const char *load_query = "SELECT \"tbl\",\"idx\",\"neq\",\"nlt\"," > "\"ndlt\",\"sample\" FROM \"_sql_stat4\""; > /* Load the statistics from the _sql_stat4 table. */ > - if (load_stat_from_space(db, init_query, load_query, stats) != 0) > + if (load_stat_from_space(init_query, load_query, stats) != 0) > goto fail; > /* > * Now we have complete statistics for each index > @@ -1739,7 +1737,7 @@ sql_analysis_load(struct sql *db) > */ > const char *order_query = "SELECT \"tbl\",\"idx\" FROM " > "\"_sql_stat4\" GROUP BY \"tbl\",\"idx\""; > - if (load_stat_to_index(db, order_query, heap_stats) == 0) > + if (load_stat_to_index(order_query, heap_stats) == 0) > return box_txn_commit(); > fail: > box_txn_rollback(); > diff --git a/src/box/sql/legacy.c b/src/box/sql/legacy.c > index bfd1e32b9..16507b334 100644 > --- a/src/box/sql/legacy.c > +++ b/src/box/sql/legacy.c > @@ -70,7 +70,7 @@ sql_exec(sql * db, /* The database on which the SQL executes */ > char **azVals = 0; > > pStmt = 0; > - rc = sql_prepare(db, zSql, -1, &pStmt, &zLeftover); > + rc = sql_prepare(zSql, -1, &pStmt, &zLeftover); > assert(rc == 0 || pStmt == NULL); > if (rc != 0) > continue; > diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c > index 35e81212d..520b52d64 100644 > --- a/src/box/sql/prepare.c > +++ b/src/box/sql/prepare.c > @@ -46,7 +46,6 @@ static int > sqlPrepare(sql * db, /* Database handle. */ > const char *zSql, /* UTF-8 encoded SQL statement. */ > int nBytes, /* Length of zSql in bytes. */ > - int saveSqlFlag, /* True to copy SQL text into the sql_stmt */ > Vdbe * pReprepare, /* VM being reprepared */ > sql_stmt ** ppStmt, /* OUT: A pointer to the prepared statement */ > const char **pzTail /* OUT: End of parsed string */ > @@ -156,8 +155,7 @@ sqlPrepare(sql * db, /* Database handle. */ > > if (db->init.busy == 0) { > Vdbe *pVdbe = sParse.pVdbe; > - sqlVdbeSetSql(pVdbe, zSql, (int)(sParse.zTail - zSql), > - saveSqlFlag); > + sqlVdbeSetSql(pVdbe, zSql, (int)(sParse.zTail - zSql)); > } > if (sParse.pVdbe != NULL && (rc != 0 || db->mallocFailed)) { > sqlVdbeFinalize(sParse.pVdbe); > @@ -192,7 +190,7 @@ sqlReprepare(Vdbe * p) > zSql = sql_sql((sql_stmt *) p); > assert(zSql != 0); /* Reprepare only called for prepare_v2() statements */ > db = sqlVdbeDb(p); > - if (sqlPrepare(db, zSql, -1, 0, p, &pNew, 0) != 0) { > + if (sqlPrepare(db, zSql, -1, p, &pNew, 0) != 0) { > assert(pNew == 0); > return -1; > } > @@ -205,10 +203,10 @@ sqlReprepare(Vdbe * p) > } > > int > -sql_prepare(struct sql *db, const char *sql, int length, struct sql_stmt **stmt, > +sql_prepare(const char *sql, int length, struct sql_stmt **stmt, > const char **sql_tail) > { > - int rc = sqlPrepare(db, sql, length, 1, 0, stmt, sql_tail); > + int rc = sqlPrepare(sql_get(), sql, length, 0, stmt, sql_tail); > assert(rc == 0 || stmt == NULL || *stmt == NULL); > return rc; > } > diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h > index 7bd952a17..ac1d8ce42 100644 > --- a/src/box/sql/sqlInt.h > +++ b/src/box/sql/sqlInt.h > @@ -471,14 +471,13 @@ typedef void (*sql_destructor_type) (void *); > /** > * Prepare (compile into VDBE byte-code) statement. > * > - * @param db Database handle. > * @param sql UTF-8 encoded SQL statement. > * @param length Length of @param sql in bytes. > * @param[out] stmt A pointer to the prepared statement. > * @param[out] sql_tail End of parsed string. > */ > int > -sql_prepare(struct sql *db, const char *sql, int length, struct sql_stmt **stmt, > +sql_prepare(const char *sql, int length, struct sql_stmt **stmt, > const char **sql_tail); > > int > diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h > index 582d48a1f..573577355 100644 > --- a/src/box/sql/vdbe.h > +++ b/src/box/sql/vdbe.h > @@ -251,7 +251,7 @@ void sqlVdbeSetNumCols(Vdbe *, int); > int sqlVdbeSetColName(Vdbe *, int, int, const char *, void (*)(void *)); > void sqlVdbeCountChanges(Vdbe *); > sql *sqlVdbeDb(Vdbe *); > -void sqlVdbeSetSql(Vdbe *, const char *z, int n, int); > +void sqlVdbeSetSql(Vdbe *, const char *z, int n); > void sqlVdbeSwap(Vdbe *, Vdbe *); > VdbeOp *sqlVdbeTakeOpArray(Vdbe *, int *, int *); > sql_value *sqlVdbeGetBoundValue(Vdbe *, int, u8); > diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h > index 0f32b4cd6..078ebc34e 100644 > --- a/src/box/sql/vdbeInt.h > +++ b/src/box/sql/vdbeInt.h > @@ -421,7 +421,6 @@ struct Vdbe { > bft explain:2; /* True if EXPLAIN present on SQL command */ > bft changeCntOn:1; /* True to update the change-counter */ > bft runOnlyOnce:1; /* Automatically expire on reset */ > - bft isPrepareV2:1; /* True if prepared with prepare_v2() */ > u32 aCounter[5]; /* Counters used by sql_stmt_status() */ > char *zSql; /* Text of the SQL statement that generated this */ > void *pFree; /* Free this when deleting the vdbe */ > diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c > index 12449d3bc..db7936e78 100644 > --- a/src/box/sql/vdbeapi.c > +++ b/src/box/sql/vdbeapi.c > @@ -451,7 +451,7 @@ sqlStep(Vdbe * p) > if (rc != SQL_ROW) > checkProfileCallback(db, p); > > - if (p->isPrepareV2 && rc != SQL_ROW && rc != SQL_DONE) { > + if (rc != SQL_ROW && rc != SQL_DONE) { > /* If this statement was prepared using sql_prepare(), and an > * error has occurred, then return an error. > */ > diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c > index a1d658648..619105820 100644 > --- a/src/box/sql/vdbeaux.c > +++ b/src/box/sql/vdbeaux.c > @@ -89,14 +89,12 @@ sql_vdbe_prepare(struct Vdbe *vdbe) > * Remember the SQL string for a prepared statement. > */ > void > -sqlVdbeSetSql(Vdbe * p, const char *z, int n, int isPrepareV2) > +sqlVdbeSetSql(Vdbe * p, const char *z, int n) > { > - assert(isPrepareV2 == 1 || isPrepareV2 == 0); > if (p == 0) > return; > assert(p->zSql == 0); > p->zSql = sqlDbStrNDup(p->db, z, n); > - p->isPrepareV2 = (u8) isPrepareV2; > } > > /* > @@ -120,7 +118,6 @@ sqlVdbeSwap(Vdbe * pA, Vdbe * pB) > zTmp = pA->zSql; > pA->zSql = pB->zSql; > pB->zSql = zTmp; > - pB->isPrepareV2 = pA->isPrepareV2; > } > > /* > -- > 2.15.1 >