* [tarantool-patches] [PATCH v2 1/4] sql: get rid off sqlite3NestedParse in clean stats
2018-07-10 17:08 [tarantool-patches] [PATCH v2 0/4] sql: get rid off sqlite3NestedParse Kirill Shcherbatov
@ 2018-07-10 17:08 ` Kirill Shcherbatov
2018-07-10 17:52 ` [tarantool-patches] " n.pettik
2018-07-10 17:08 ` [tarantool-patches] [PATCH v2 2/4] sql: remove usless sqlite3NestedParse function Kirill Shcherbatov
` (3 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Kirill Shcherbatov @ 2018-07-10 17:08 UTC (permalink / raw)
To: tarantool-patches; +Cc: korablev, Kirill Shcherbatov
Now we manually generate AST structures to drop outdated
stats from _sql_stat1 and _sql_stat4 spaces instead of
starting sqlite3NestedParse. This function become totally
useless and could be removed.
Part of #3496.
---
src/box/sql/analyze.c | 102 ++++++++++++++++++++----------------------------
src/box/sql/build.c | 79 ++++++++++++++++++++++++++-----------
src/box/sql/sqliteInt.h | 13 ++++++
3 files changed, 113 insertions(+), 81 deletions(-)
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 5f73f02..336d146 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -116,71 +116,53 @@
#include "tarantoolInt.h"
#include "vdbeInt.h"
-/*
- * This routine generates code that opens the sql_statN tables.
- * The _sql_stat1 table is always relevant. _sql_stat4 is only opened when
- * appropriate compile-time options are provided.
- *
- * If the sql_statN tables do not previously exist, it is created.
+/**
+ * This routine generates code that opens the sql_stat1/4 tables.
+ * If the sql_statN tables do not previously exist, they are
+ * created.
*
- * Argument zWhere may be a pointer to a buffer containing a table name,
- * or it may be a NULL pointer. If it is not NULL, then all entries in
- * the sql_statN tables associated with the named table are deleted.
- * If zWhere==0, then code is generated to delete all stat table entries.
+ * @param parse Parsing context.
+ * @param stat_cursor Open the _sql_stat1 table on this cursor.
+ * @param index_name Delete records of this table if specified.
+ * @param table_name Delete records of this index if specified.
*/
static void
-openStatTable(Parse * pParse, /* Parsing context */
- int iStatCur, /* Open the _sql_stat1 table on this cursor */
- const char *zWhere, /* Delete entries for this table or index */
- const char *zWhereType /* Either "tbl" or "idx" */
- )
+vdbe_emit_stat_space_open(struct Parse *parse, int stat_cursor,
+ const char *index_name, const char *table_name)
{
- const char *aTable[] = {
- "_sql_stat1",
- "_sql_stat4",
- NULL};
- int i;
- sqlite3 *db = pParse->db;
- Vdbe *v = sqlite3GetVdbe(pParse);
- int aRoot[ArraySize(aTable)];
- u8 aCreateTbl[ArraySize(aTable)];
+ const char *space_names[] = {"_sql_stat1", "_sql_stat4"};
+ const uint32_t space_ids[] = {BOX_SQL_STAT1_ID, BOX_SQL_STAT4_ID};
+ struct Vdbe *v = sqlite3GetVdbe(parse);
+ assert(v != NULL);
+ assert(sqlite3VdbeDb(v) == parse->db);
- if (v == 0)
- return;
- assert(sqlite3VdbeDb(v) == db);
-
- /* Create new statistic tables if they do not exist, or clear them
- * if they do already exist.
+ /*
+ * Create new statistic tables if they do not exist, or
+ * clear them if they do already exist.
*/
- for (i = 0; aTable[i]; i++) {
- const char *zTab = aTable[i];
- Table *pStat;
- /* The table already exists, because it is a system space */
- pStat = sqlite3HashFind(&db->pSchema->tblHash, zTab);
- assert(pStat != NULL);
- aRoot[i] = pStat->tnum;
- aCreateTbl[i] = 0;
- if (zWhere) {
- sqlite3NestedParse(pParse,
- "DELETE FROM \"%s\" WHERE \"%s\"=%Q",
- zTab, zWhereType, zWhere);
+ for (uint i = 0; i < lengthof(space_names); ++i) {
+ const char *space_name = space_names[i];
+ /*
+ * The table already exists, because it is a
+ * system space.
+ */
+ assert(sqlite3HashFind(&parse->db->pSchema->tblHash,
+ space_name) != NULL);
+ if (table_name != NULL || index_name != NULL) {
+ vdbe_emit_stat_space_clear(parse, space_name,
+ index_name, table_name);
} else {
- /*
- * The sql_stat[134] table already exists.
- * Delete all rows.
- */
- sqlite3VdbeAddOp2(v, OP_Clear,
- SQLITE_PAGENO_TO_SPACEID(aRoot[i]), 0);
+ sqlite3VdbeAddOp2(v, OP_Clear, space_ids[i], 0);
}
}
- /* Open the sql_stat[134] tables for writing. */
- for (i = 0; aTable[i]; i++) {
- struct space *space =
- space_by_id(SQLITE_PAGENO_TO_SPACEID(aRoot[i]));
- vdbe_emit_open_cursor(pParse, iStatCur + i, aRoot[i], space);
- sqlite3VdbeChangeP5(v, aCreateTbl[i]);
- VdbeComment((v, aTable[i]));
+ /* Open the sql_stat tables for writing. */
+ for (uint i = 0; i < lengthof(space_names); ++i) {
+ uint32_t id = space_ids[i];
+ int tnum = SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(id, 0);
+ vdbe_emit_open_cursor(parse, stat_cursor + i, tnum,
+ space_by_id(id));
+ VdbeComment((v, space_names[i]));
}
}
@@ -1117,7 +1099,7 @@ sql_analyze_database(Parse *parser)
sql_set_multi_write(parser, false);
int stat_cursor = parser->nTab;
parser->nTab += 3;
- openStatTable(parser, stat_cursor, NULL, NULL);
+ vdbe_emit_stat_space_open(parser, stat_cursor, NULL, NULL);
int reg = parser->nMem + 1;
int tab_cursor = parser->nTab;
for (struct HashElem *k = sqliteHashFirst(&schema->tblHash); k != NULL;
@@ -1145,10 +1127,12 @@ analyzeTable(Parse * pParse, Table * pTab, Index * pOnlyIdx)
sql_set_multi_write(pParse, false);
iStatCur = pParse->nTab;
pParse->nTab += 3;
- if (pOnlyIdx) {
- openStatTable(pParse, iStatCur, pOnlyIdx->zName, "idx");
+ if (pOnlyIdx != NULL) {
+ vdbe_emit_stat_space_open(pParse, iStatCur, pOnlyIdx->zName,
+ NULL);
} else {
- openStatTable(pParse, iStatCur, pTab->def->name, "tbl");
+ vdbe_emit_stat_space_open(pParse, iStatCur, NULL,
+ pTab->def->name);
}
analyzeOneTable(pParse, pTab, pOnlyIdx, iStatCur, pParse->nMem + 1,
pParse->nTab);
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 0072f84..21791a4 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2050,6 +2050,60 @@ sql_store_select(struct Parse *parse_context, struct Select *select)
}
/**
+ * Create expression record "@col_name = '@col_value'".
+ *
+ * @param parse The parsing context.
+ * @param col_name Name of column.
+ * @param col_value Name of row.
+ * @retval not NULL on success.
+ * @retval NULL on failure.
+ */
+static struct Expr *
+sql_id_eq_str_expr(struct Parse *parse, const char *col_name,
+ const char *col_value)
+{
+ struct sqlite3 *db = parse->db;
+
+ struct Expr *col_name_expr = sqlite3Expr(db, TK_ID, col_name);
+ if (col_name_expr == NULL)
+ return NULL;
+ struct Expr *col_value_expr = sqlite3Expr(db, TK_STRING, col_value);
+ if (col_value_expr == NULL) {
+ sql_expr_delete(db, col_name_expr, false);
+ return NULL;
+ }
+ return sqlite3PExpr(parse, TK_EQ, col_name_expr, col_value_expr);
+}
+
+void
+vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name,
+ const char *idx_name, const char *table_name)
+{
+ assert(idx_name != NULL || table_name != NULL);
+ struct sqlite3 *db = parse->db;
+ assert(!db->mallocFailed);
+ struct SrcList *src_list = sql_alloc_src_list(db);
+ if (src_list != NULL)
+ src_list->a[0].zName = sqlite3DbStrDup(db, stat_table_name);
+ struct Expr *where = NULL;
+ if (idx_name != NULL) {
+ struct Expr *expr = sql_id_eq_str_expr(parse, "idx", idx_name);
+ if (expr != NULL)
+ where = sqlite3ExprAnd(db, expr, where);
+ }
+ if (table_name != NULL) {
+ struct Expr *expr = sql_id_eq_str_expr(parse, "tbl", table_name);
+ if (expr != NULL)
+ where = sqlite3ExprAnd(db, expr, where);
+ }
+ /**
+ * On memory allocation error sql_table delete_from
+ * releases memory for its own.
+ */
+ sql_table_delete_from(parse, src_list, where);
+}
+
+/**
* Remove entries from the _sql_stat1 and _sql_stat4
* system spaces after a DROP INDEX or DROP TABLE command.
*
@@ -2059,30 +2113,11 @@ sql_store_select(struct Parse *parse_context, struct Select *select)
* @param idx_name Index to be dropped.
*/
static void
-sql_clear_stat_spaces(Parse *parse, const char *table_name,
+sql_clear_stat_spaces(struct Parse *parse, const char *table_name,
const char *idx_name)
{
- if (idx_name != NULL) {
- sqlite3NestedParse(parse,
- "DELETE FROM \"_sql_stat1\" "
- "WHERE (\"idx\"=%Q AND "
- "\"tbl\"=%Q)",
- idx_name, table_name);
- sqlite3NestedParse(parse,
- "DELETE FROM \"_sql_stat4\" "
- "WHERE (\"idx\"=%Q AND "
- "\"tbl\"=%Q)",
- idx_name, table_name);
- } else {
- sqlite3NestedParse(parse,
- "DELETE FROM \"_sql_stat1\" "
- "WHERE \"tbl\"=%Q",
- table_name);
- sqlite3NestedParse(parse,
- "DELETE FROM \"_sql_stat4\" "
- "WHERE \"tbl\"=%Q",
- table_name);
- }
+ vdbe_emit_stat_space_clear(parse, "_sql_stat4", idx_name, table_name);
+ vdbe_emit_stat_space_clear(parse, "_sql_stat1", idx_name, table_name);
}
/**
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 8b75ae8..d76d173 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -4847,4 +4847,17 @@ vdbe_emit_halt_with_presence_test(struct Parse *parser, int space_id,
const char *error_src, bool no_error,
int cond_opcode);
+/**
+ * Generate VDBE code to delete records from system _sql_stat1 or
+ * _sql_stat4 table.
+ *
+ * @param parse The parsing context.
+ * @param stat_table_name System stat table name.
+ * @param idx_name Index name.
+ * @param table_name Table name.
+ */
+void
+vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name,
+ const char *idx_name, const char *table_name);
+
#endif /* SQLITEINT_H */
--
2.7.4
^ permalink raw reply [flat|nested] 18+ messages in thread* [tarantool-patches] Re: [PATCH v2 1/4] sql: get rid off sqlite3NestedParse in clean stats
2018-07-10 17:08 ` [tarantool-patches] [PATCH v2 1/4] sql: get rid off sqlite3NestedParse in clean stats Kirill Shcherbatov
@ 2018-07-10 17:52 ` n.pettik
2018-07-11 7:22 ` Kirill Shcherbatov
0 siblings, 1 reply; 18+ messages in thread
From: n.pettik @ 2018-07-10 17:52 UTC (permalink / raw)
To: tarantool-patches; +Cc: Kirill Shcherbatov
> On 10 Jul 2018, at 20:08, Kirill Shcherbatov <kshcherbatov@tarantool.org> wrote:
>
> Now we manually generate AST structures to drop outdated
> stats from _sql_stat1 and _sql_stat4 spaces instead of
> starting sqlite3NestedParse. This function become totally
Typo: becomes.
>
> --- a/src/box/sql/analyze.c
> +++ b/src/box/sql/analyze.c
> @@ -116,71 +116,53 @@
> +vdbe_emit_stat_space_open(struct Parse *parse, int stat_cursor,
> + const char *index_name, const char *table_name)
> {
> - const char *aTable[] = {
> - "_sql_stat1",
> - "_sql_stat4",
> - NULL};
> - int i;
> - sqlite3 *db = pParse->db;
> - Vdbe *v = sqlite3GetVdbe(pParse);
> - int aRoot[ArraySize(aTable)];
> - u8 aCreateTbl[ArraySize(aTable)];
> + const char *space_names[] = {"_sql_stat1", "_sql_stat4"};
You already have argument ’table_name’, so it looks confusing.
Lets name it like ‘stat_names’. The same for space_ids.
> + const uint32_t space_ids[] = {BOX_SQL_STAT1_ID, BOX_SQL_STAT4_ID};
> + struct Vdbe *v = sqlite3GetVdbe(parse);
> + assert(v != NULL);
> + assert(sqlite3VdbeDb(v) == parse->db);
>
> - if (v == 0)
> - return;
> - assert(sqlite3VdbeDb(v) == db);
> -
> - /* Create new statistic tables if they do not exist, or clear them
> - * if they do already exist.
> + /*
> + * Create new statistic tables if they do not exist, or
But comment below says that ’tables already exist since they are system’.
> + * clear them if they do already exist.
> */
> - for (i = 0; aTable[i]; i++) {
> - const char *zTab = aTable[i];
> - Table *pStat;
> - /* The table already exists, because it is a system space */
> - pStat = sqlite3HashFind(&db->pSchema->tblHash, zTab);
> - assert(pStat != NULL);
> - aRoot[i] = pStat->tnum;
> - aCreateTbl[i] = 0;
> - if (zWhere) {
> - sqlite3NestedParse(pParse,
> - "DELETE FROM \"%s\" WHERE \"%s\"=%Q",
> - zTab, zWhereType, zWhere);
> + for (uint i = 0; i < lengthof(space_names); ++i) {
> + const char *space_name = space_names[i];
> + /*
> + * The table already exists, because it is a
> + * system space.
> + */
> + assert(sqlite3HashFind(&parse->db->pSchema->tblHash,
> + space_name) != NULL);
> + if (table_name != NULL || index_name != NULL) {
I don’t understand situation when index_name != NULL but table_name == NULL.
In our SQL index name is local to table, (i.e. indexes with the same name could
exist within different tables). I see that vdbe_emit_stat_space_open() is called twice
in analyzeTable() and analyzeTable() in turn always called with pOnlyIdx == NULL.
Hence, in vdbe_emit_stat_space_open() index_name is always == NULL.
Lets remove this dead code and excess arg.
> + vdbe_emit_stat_space_clear(parse, space_name,
> + index_name, table_name);
> } else {
> - /*
> - * The sql_stat[134] table already exists.
> - * Delete all rows.
> - */
> - sqlite3VdbeAddOp2(v, OP_Clear,
> - SQLITE_PAGENO_TO_SPACEID(aRoot[i]), 0);
> + sqlite3VdbeAddOp2(v, OP_Clear, space_ids[i], 0);
OP_Clear uses only first operand, so you can use sqlite3VdbeAddOp1();
^ permalink raw reply [flat|nested] 18+ messages in thread* [tarantool-patches] Re: [PATCH v2 1/4] sql: get rid off sqlite3NestedParse in clean stats
2018-07-10 17:52 ` [tarantool-patches] " n.pettik
@ 2018-07-11 7:22 ` Kirill Shcherbatov
2018-07-11 12:19 ` n.pettik
0 siblings, 1 reply; 18+ messages in thread
From: Kirill Shcherbatov @ 2018-07-11 7:22 UTC (permalink / raw)
To: tarantool-patches, Nikita Pettik
Thank you for review.
> Typo: becomes.
Ok, tnx.
> You already have argument ’table_name’, so it looks confusing.
> Lets name it like ‘stat_names’. The same for space_ids.
- const char *space_names[] = {"_sql_stat1", "_sql_stat4"};
- const uint32_t space_ids[] = {BOX_SQL_STAT1_ID, BOX_SQL_STAT4_ID};
+ const char *stat_names[] = {"_sql_stat1", "_sql_stat4"};
+ const uint32_t stat_ids[] = {BOX_SQL_STAT1_ID, BOX_SQL_STAT4_ID};
> But comment below says that ’tables already exist since they are system’.
Hmm, loks like some legacy.
-
- /*
- * Create new statistic tables if they do not exist, or
- * clear them if they do already exist.
- */
>
>> + * clear them if they do already exist.
>> */
>> - for (i = 0; aTable[i]; i++) {
>> - const char *zTab = aTable[i];
>> - Table *pStat;
>> - /* The table already exists, because it is a system space */
>> - pStat = sqlite3HashFind(&db->pSchema->tblHash, zTab);
>> - assert(pStat != NULL);
>> - aRoot[i] = pStat->tnum;
>> - aCreateTbl[i] = 0;
>> - if (zWhere) {
>> - sqlite3NestedParse(pParse,
>> - "DELETE FROM \"%s\" WHERE \"%s\"=%Q",
>> - zTab, zWhereType, zWhere);
>> + for (uint i = 0; i < lengthof(space_names); ++i) {
>> + const char *space_name = space_names[i];
>> + /*
>> + * The table already exists, because it is a
>> + * system space.
>> + */
>> + assert(sqlite3HashFind(&parse->db->pSchema->tblHash,
>> + space_name) != NULL);
>> + if (table_name != NULL || index_name != NULL) {
>
> I don’t understand situation when index_name != NULL but table_name == NULL.
> In our SQL index name is local to table, (i.e. indexes with the same name could
> exist within different tables). I see that vdbe_emit_stat_space_open() is called twice
> in analyzeTable() and analyzeTable() in turn always called with pOnlyIdx == NULL.
> Hence, in vdbe_emit_stat_space_open() index_name is always == NULL.
> Lets remove this dead code and excess arg.
- if (table_name != NULL || index_name != NULL) {
+ if (table_name != NULL) {
/**
* Generate code that will do an analysis of a single table in
* a database.
*
* @param parse Parser context.
* @param table Target table to analyze.
*/
static void
vdbe_emit_analyze_table(struct Parse *parse, struct Table *table)
{
assert(table != NULL);
sql_set_multi_write(parse, false);
int stat_cursor = parse->nTab;
parse->nTab += 3;
vdbe_emit_stat_space_open(parse, stat_cursor, NULL, table->def->name);
analyzeOneTable(parse, table, NULL, stat_cursor, parse->nMem + 1,
parse->nTab);
loadAnalysis(parse);
}
> OP_Clear uses only first operand, so you can use sqlite3VdbeAddOp1();
- sqlite3VdbeAddOp2(v, OP_Clear, stat_ids[i], 0);
+ sqlite3VdbeAddOp1(v, OP_Clear, stat_ids[i]);
^ permalink raw reply [flat|nested] 18+ messages in thread* [tarantool-patches] Re: [PATCH v2 1/4] sql: get rid off sqlite3NestedParse in clean stats
2018-07-11 7:22 ` Kirill Shcherbatov
@ 2018-07-11 12:19 ` n.pettik
2018-07-11 12:23 ` Kirill Shcherbatov
0 siblings, 1 reply; 18+ messages in thread
From: n.pettik @ 2018-07-11 12:19 UTC (permalink / raw)
To: tarantool-patches; +Cc: Kirill Shcherbatov
> * Generate code that will do an analysis of a single table in
> * a database.
> *
> * @param parse Parser context.
> * @param table Target table to analyze.
> */
> static void
> vdbe_emit_analyze_table(struct Parse *parse, struct Table *table)
> {
> assert(table != NULL);
> sql_set_multi_write(parse, false);
> int stat_cursor = parse->nTab;
> parse->nTab += 3;
> vdbe_emit_stat_space_open(parse, stat_cursor, NULL, table->def->name);
Look, vdbe_emit_stat_space_open() still always takes NULL as index_name.
Lets remove it.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [tarantool-patches] [PATCH v2 2/4] sql: remove usless sqlite3NestedParse function
2018-07-10 17:08 [tarantool-patches] [PATCH v2 0/4] sql: get rid off sqlite3NestedParse Kirill Shcherbatov
2018-07-10 17:08 ` [tarantool-patches] [PATCH v2 1/4] sql: get rid off sqlite3NestedParse in clean stats Kirill Shcherbatov
@ 2018-07-10 17:08 ` Kirill Shcherbatov
2018-07-10 18:22 ` [tarantool-patches] " n.pettik
2018-07-10 17:08 ` [tarantool-patches] [PATCH v2 3/4] sql: refactor vdbe_emit_open_cursor calls Kirill Shcherbatov
` (2 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Kirill Shcherbatov @ 2018-07-10 17:08 UTC (permalink / raw)
To: tarantool-patches; +Cc: korablev, Kirill Shcherbatov
As sqlite3NestedParse become totaly useless, let's
remove unreacheble code and all mentioning.
Resolves #3496.
---
src/box/sql/build.c | 100 +++++-------------------------------------------
src/box/sql/delete.c | 13 ++-----
src/box/sql/insert.c | 18 +++------
src/box/sql/parse.y | 12 +-----
src/box/sql/sqliteInt.h | 15 ++------
src/box/sql/tokenize.c | 2 +-
src/box/sql/trigger.c | 45 +++++++---------------
src/box/sql/update.c | 15 +++-----
8 files changed, 45 insertions(+), 175 deletions(-)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 21791a4..1c00842 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -57,8 +57,6 @@
void
sql_finish_coding(struct Parse *parse_context)
{
- if (parse_context->nested)
- return;
assert(parse_context->pToplevel == NULL);
struct sqlite3 *db = parse_context->db;
struct Vdbe *v = sqlite3GetVdbe(parse_context);
@@ -116,46 +114,6 @@ sql_finish_coding(struct Parse *parse_context)
}
}
-/*
- * Run the parser and code generator recursively in order to generate
- * code for the SQL statement given onto the end of the pParse context
- * currently under construction. When the parser is run recursively
- * this way, the final OP_Halt is not appended and other initialization
- * and finalization steps are omitted because those are handling by the
- * outermost parser.
- *
- * Not everything is nestable. This facility is designed to perform
- * basic DDL operations. Use care if you decide to try to use this routine
- * for some other purposes.
- */
-void
-sqlite3NestedParse(Parse * pParse, const char *zFormat, ...)
-{
- va_list ap;
- char *zSql;
- char *zErrMsg = 0;
- sqlite3 *db = pParse->db;
- char saveBuf[PARSE_TAIL_SZ];
-
- if (pParse->nErr)
- return;
- assert(pParse->nested < 10); /* Nesting should only be of limited depth */
- va_start(ap, zFormat);
- zSql = sqlite3VMPrintf(db, zFormat, ap);
- va_end(ap);
- if (zSql == 0) {
- return; /* A malloc must have failed */
- }
- pParse->nested++;
- memcpy(saveBuf, PARSE_TAIL(pParse), PARSE_TAIL_SZ);
- memset(PARSE_TAIL(pParse), 0, PARSE_TAIL_SZ);
- sqlite3RunParser(pParse, zSql, &zErrMsg);
- sqlite3DbFree(db, zErrMsg);
- sqlite3DbFree(db, zSql);
- memcpy(PARSE_TAIL(pParse), saveBuf, PARSE_TAIL_SZ);
- pParse->nested--;
-}
-
/**
* This is a function which should be called during execution
* of sqlite3EndTable. It ensures that only PRIMARY KEY
@@ -532,17 +490,10 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
Table *pTable;
char *zName = 0; /* The name of the new table */
sqlite3 *db = pParse->db;
- Vdbe *v;
-
- /* Do not account nested operations: the count of such
- * operations depends on Tarantool data dictionary internals,
- * such as data layout in system spaces.
- */
- if (!pParse->nested) {
- if ((v = sqlite3GetVdbe(pParse)) == NULL)
- goto cleanup;
- sqlite3VdbeCountChanges(v);
- }
+ struct Vdbe *v = sqlite3GetVdbe(pParse);
+ if (v == NULL)
+ goto cleanup;
+ sqlite3VdbeCountChanges(v);
zName = sqlite3NameFromToken(db, pName);
@@ -1496,13 +1447,7 @@ createIndex(Parse * pParse, Index * pIndex, int iSpaceId, int iIndexId,
SQL_SUBTYPE_MSGPACK,zParts, P4_STATIC);
sqlite3VdbeAddOp3(v, OP_MakeRecord, iFirstCol, 6, iRecord);
sqlite3VdbeAddOp2(v, OP_SInsert, BOX_INDEX_ID, iRecord);
- /* Do not account nested operations: the count of such
- * operations depends on Tarantool data dictionary internals,
- * such as data layout in system spaces. Also do not account
- * autoindexes - they had been accounted as a part of
- * CREATE TABLE already.
- */
- if (!pParse->nested && pIndex->idxType == SQLITE_IDXTYPE_APPDEF)
+ if (pIndex->idxType == SQLITE_IDXTYPE_APPDEF)
sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
}
@@ -1601,12 +1546,7 @@ createSpace(Parse * pParse, int iSpaceId, char *zStmt)
SQL_SUBTYPE_MSGPACK, zFormat, P4_STATIC);
sqlite3VdbeAddOp3(v, OP_MakeRecord, iFirstCol, 7, iRecord);
sqlite3VdbeAddOp2(v, OP_SInsert, BOX_SPACE_ID, iRecord);
- /* Do not account nested operations: the count of such
- * operations depends on Tarantool data dictionary internals,
- * such as data layout in system spaces.
- */
- if (!pParse->nested)
- sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
+ sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
}
/*
@@ -2144,13 +2084,11 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
* Do not account triggers deletion - they will be
* accounted in DELETE from _space below.
*/
- parse_context->nested++;
struct sql_trigger *trigger = space->sql_triggers;
while (trigger != NULL) {
- vdbe_code_drop_trigger(parse_context, trigger->zName);
+ vdbe_code_drop_trigger(parse_context, trigger->zName, false);
trigger = trigger->next;
}
- parse_context->nested--;
/*
* Remove any entries of the _sequence and _space_sequence
* space associated with the table being dropped. This is
@@ -2238,13 +2176,7 @@ sql_drop_table(struct Parse *parse_context, struct SrcList *table_name_list,
if (v == NULL || db->mallocFailed) {
goto exit_drop_table;
}
- /*
- * Activate changes counting here to account
- * DROP TABLE IF NOT EXISTS, if the table really does not
- * exist.
- */
- if (!parse_context->nested)
- sqlite3VdbeCountChanges(v);
+ sqlite3VdbeCountChanges(v);
assert(parse_context->nErr == 0);
assert(table_name_list->nSrc == 1);
assert(is_view == 0 || is_view == LOCATE_VIEW);
@@ -2705,13 +2637,7 @@ sql_create_index(struct Parse *parse, struct Token *token,
if (db->mallocFailed || parse->nErr > 0) {
goto exit_create_index;
}
- /* Do not account nested operations: the count of such
- * operations depends on Tarantool data dictionary internals,
- * such as data layout in system spaces. Also do not account
- * PRIMARY KEY and UNIQUE constraint - they had been accounted
- * in CREATE TABLE already.
- */
- if (!parse->nested && idx_type == SQLITE_IDXTYPE_APPDEF) {
+ if (idx_type == SQLITE_IDXTYPE_APPDEF) {
Vdbe *v = sqlite3GetVdbe(parse);
if (v == NULL)
goto exit_create_index;
@@ -3158,13 +3084,7 @@ sql_drop_index(struct Parse *parse_context, struct SrcList *index_name_list,
if (db->mallocFailed) {
goto exit_drop_index;
}
- /*
- * Do not account nested operations: the count of such
- * operations depends on Tarantool data dictionary internals,
- * such as data layout in system spaces.
- */
- if (!parse_context->nested)
- sqlite3VdbeCountChanges(v);
+ sqlite3VdbeCountChanges(v);
assert(index_name_list->nSrc == 1);
assert(table_token->n > 0);
uint32_t space_id = box_space_id_by_name(table_name,
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 5a79971..66dc0fc 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -156,8 +156,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
if (v == NULL)
goto delete_from_cleanup;
- if (parse->nested == 0)
- sqlite3VdbeCountChanges(v);
+ sqlite3VdbeCountChanges(v);
sql_set_multi_write(parse, true);
/* If we are trying to delete from a view, realize that
@@ -387,7 +386,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
idx_noseek = one_pass_cur[1];
sql_generate_row_delete(parse, table, trigger_list, tab_cursor,
- reg_key, key_len, parse->nested == 0,
+ reg_key, key_len, true,
ON_CONFLICT_ACTION_DEFAULT, one_pass,
idx_noseek);
@@ -403,13 +402,9 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
}
}
- /* Return the number of rows that were deleted. If this
- * routine is generating code because of a call to
- * sqlite3NestedParse(), do not invoke the callback
- * function.
- */
+ /* Return the number of rows that were deleted. */
if ((user_session->sql_flags & SQLITE_CountRows) != 0 &&
- parse->nested == 0 && parse->pTriggerTab != NULL) {
+ parse->pTriggerTab != NULL) {
sqlite3VdbeAddOp2(v, OP_ResultRow, reg_count, 1);
sqlite3VdbeSetNumCols(v, 1);
sqlite3VdbeSetColName(v, 0, COLNAME_NAME, "rows deleted",
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index c12043b..58e159c 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -413,13 +413,11 @@ sqlite3Insert(Parse * pParse, /* Parser context */
goto insert_cleanup;
}
- /* Allocate a VDBE
- */
+ /* Allocate a VDBE. */
v = sqlite3GetVdbe(pParse);
- if (v == 0)
+ if (v == NULL)
goto insert_cleanup;
- if (pParse->nested == 0)
- sqlite3VdbeCountChanges(v);
+ sqlite3VdbeCountChanges(v);
sql_set_multi_write(pParse, pSelect != NULL || trigger != NULL);
#ifndef SQLITE_OMIT_XFER_OPT
@@ -904,13 +902,9 @@ sqlite3Insert(Parse * pParse, /* Parser context */
insert_end:
- /*
- * Return the number of rows inserted. If this routine is
- * generating code because of a call to sqlite3NestedParse(), do not
- * invoke the callback function.
- */
- if ((user_session->sql_flags & SQLITE_CountRows) && !pParse->nested
- && !pParse->pTriggerTab) {
+ /* Return the number of rows inserted. */
+ if ((user_session->sql_flags & SQLITE_CountRows) != 0 &&
+ pParse->pTriggerTab == NULL) {
sqlite3VdbeAddOp2(v, OP_ResultRow, regRowCount, 1);
sqlite3VdbeSetNumCols(v, 1);
sqlite3VdbeSetColName(v, 0, COLNAME_NAME, "rows inserted",
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index b2940b7..ac935fd 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -877,18 +877,10 @@ expr(A) ::= VARIABLE(X). {
else
sqlite3ExprAssignVarNumber(pParse, A.pExpr, n);
}else{
- /* When doing a nested parse, one can include terms in an expression
- ** that look like this: #1 #2 ... These terms refer to registers
- ** in the virtual machine. #N is the N-th register. */
assert( t.n>=2 );
spanSet(&A, &t, &t);
- if( pParse->nested==0 ){
- sqlite3ErrorMsg(pParse, "near \"%T\": syntax error", &t);
- A.pExpr = 0;
- }else{
- A.pExpr = sqlite3PExpr(pParse, TK_REGISTER, 0, 0);
- if( A.pExpr ) sqlite3GetInt32(&t.z[1], &A.pExpr->iTable);
- }
+ sqlite3ErrorMsg(pParse, "near \"%T\": syntax error", &t);
+ A.pExpr = NULL;
}
}
expr(A) ::= expr(A) COLLATE id(C). {
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index d76d173..380c9c6 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2880,7 +2880,6 @@ struct Parse {
int rc; /* Return code from execution */
u8 colNamesSet; /* TRUE after OP_ColumnName has been issued to pVdbe */
u8 checkSchema; /* Causes schema cookie check after an error */
- u8 nested; /* Number of nested calls to the parser/code generator */
u8 nTempReg; /* Number of temporary registers in aTempReg[] */
u8 isMultiWrite; /* True if statement may modify/insert multiple rows */
u8 mayAbort; /* True if statement may throw an ABORT exception */
@@ -2976,14 +2975,6 @@ struct Parse {
};
/*
- * Sizes and pointers of various parts of the Parse object.
- */
-#define PARSE_HDR_SZ offsetof(Parse,aColCache) /* Recursive part w/o aColCache */
-#define PARSE_RECURSE_SZ offsetof(Parse,sLastToken) /* Recursive part */
-#define PARSE_TAIL_SZ (sizeof(Parse)-PARSE_RECURSE_SZ) /* Non-recursive part */
-#define PARSE_TAIL(X) (((char*)(X))+PARSE_RECURSE_SZ) /* Pointer to tail */
-
-/*
* Bitfield flags for P5 value in various opcodes.
*
* Value constraints (enforced via assert()):
@@ -4129,9 +4120,12 @@ sql_drop_trigger(struct Parse *parser, struct SrcList *name, bool no_err);
*
* @param parser Parser context.
* @param trigger_name The name of trigger to drop.
+ * @param account_changes Increase number of db changes made since
+ * last reset.
*/
void
-vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name);
+vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name,
+ bool account_changes);
/**
* Return a list of all triggers on table pTab if there exists at
@@ -4449,7 +4443,6 @@ void sqlite3AlterRenameTable(Parse *, SrcList *, Token *);
int
sql_token(const char *z, int *type, bool *is_reserved);
-void sqlite3NestedParse(Parse *, const char *, ...);
void sqlite3ExpirePreparedStatements(sqlite3 *);
int sqlite3CodeSubselect(Parse *, Expr *, int);
void sqlite3SelectPrep(Parse *, Select *, NameContext *);
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 7c3dabe..ce9ed84 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -526,7 +526,7 @@ sqlite3RunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
pParse->zErrMsg = 0;
nErr++;
}
- if (pParse->pVdbe && pParse->nErr > 0 && pParse->nested == 0) {
+ if (pParse->pVdbe != NULL && pParse->nErr > 0) {
sqlite3VdbeDelete(pParse->pVdbe);
pParse->pVdbe = 0;
}
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 801013b..ec0bc98 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -75,16 +75,10 @@ sql_trigger_begin(struct Parse *parse, struct Token *name, int tr_tm,
/* The name of the Trigger. */
char *trigger_name = NULL;
- /*
- * Do not account nested operations: the count of such
- * operations depends on Tarantool data dictionary
- * internals, such as data layout in system spaces.
- */
- if (!parse->nested) {
- struct Vdbe *v = sqlite3GetVdbe(parse);
- if (v != NULL)
- sqlite3VdbeCountChanges(v);
- }
+ struct Vdbe *v = sqlite3GetVdbe(parse);
+ if (v != NULL)
+ sqlite3VdbeCountChanges(v);
+
/* pName->z might be NULL, but not pName itself. */
assert(name != NULL);
assert(op == TK_INSERT || op == TK_UPDATE || op == TK_DELETE);
@@ -261,14 +255,8 @@ sql_trigger_finish(struct Parse *parse, struct TriggerStep *step_list,
SQL_SUBTYPE_MSGPACK, opts_buff, P4_DYNAMIC);
sqlite3VdbeAddOp3(v, OP_MakeRecord, first_col, 3, record);
sqlite3VdbeAddOp2(v, OP_IdxInsert, cursor, record);
- /*
- * Do not account nested operations: the count of
- * such operations depends on Tarantool data
- * dictionary internals, such as data layout in
- * system spaces.
- */
- if (!parse->nested)
- sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
+
+ sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
sqlite3VdbeAddOp1(v, OP_Close, cursor);
sql_set_multi_write(parse, false);
@@ -440,7 +428,8 @@ sql_trigger_delete(struct sqlite3 *db, struct sql_trigger *trigger)
}
void
-vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name)
+vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name,
+ bool account_changes)
{
sqlite3 *db = parser->db;
assert(db->pSchema != NULL);
@@ -459,7 +448,7 @@ vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name)
record_to_delete);
sqlite3VdbeAddOp2(v, OP_SDelete, BOX_TRIGGER_ID,
record_to_delete);
- if (parser->nested == 0)
+ if (account_changes)
sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
sqlite3ChangeCookie(parser);
}
@@ -473,17 +462,9 @@ sql_drop_trigger(struct Parse *parser, struct SrcList *name, bool no_err)
goto drop_trigger_cleanup;
assert(db->pSchema != NULL);
- /* Do not account nested operations: the count of such
- * operations depends on Tarantool data dictionary internals,
- * such as data layout in system spaces. Activate the counter
- * here to account DROP TRIGGER IF EXISTS case if the trigger
- * actually does not exist.
- */
- if (!parser->nested) {
- Vdbe *v = sqlite3GetVdbe(parser);
- if (v != NULL)
- sqlite3VdbeCountChanges(v);
- }
+ struct Vdbe *v = sqlite3GetVdbe(parser);
+ if (v != NULL)
+ sqlite3VdbeCountChanges(v);
assert(name->nSrc == 1);
const char *trigger_name = name->a[0].zName;
@@ -495,7 +476,7 @@ sql_drop_trigger(struct Parse *parser, struct SrcList *name, bool no_err)
error_msg, no_err, OP_Found) != 0)
goto drop_trigger_cleanup;
- vdbe_code_drop_trigger(parser, trigger_name);
+ vdbe_code_drop_trigger(parser, trigger_name, true);
drop_trigger_cleanup:
sqlite3SrcListDelete(db, name);
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 212adbc..cbcca34 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -261,10 +261,9 @@ sqlite3Update(Parse * pParse, /* The parser context */
/* Begin generating code. */
v = sqlite3GetVdbe(pParse);
- if (v == 0)
+ if (v == NULL)
goto update_cleanup;
- if (pParse->nested == 0)
- sqlite3VdbeCountChanges(v);
+ sqlite3VdbeCountChanges(v);
sql_set_multi_write(pParse, true);
/* Allocate required registers. */
@@ -625,13 +624,9 @@ sqlite3Update(Parse * pParse, /* The parser context */
}
sqlite3VdbeResolveLabel(v, labelBreak);
- /*
- * Return the number of rows that were changed. If this routine is
- * generating code because of a call to sqlite3NestedParse(), do not
- * invoke the callback function.
- */
- if ((user_session->sql_flags & SQLITE_CountRows) &&
- !pParse->pTriggerTab && !pParse->nested) {
+ /* Return the number of rows that were changed. */
+ if (user_session->sql_flags & SQLITE_CountRows &&
+ pParse->pTriggerTab == NULL) {
sqlite3VdbeAddOp2(v, OP_ResultRow, regRowCount, 1);
sqlite3VdbeSetNumCols(v, 1);
sqlite3VdbeSetColName(v, 0, COLNAME_NAME, "rows updated",
--
2.7.4
^ permalink raw reply [flat|nested] 18+ messages in thread* [tarantool-patches] [PATCH v2 3/4] sql: refactor vdbe_emit_open_cursor calls
2018-07-10 17:08 [tarantool-patches] [PATCH v2 0/4] sql: get rid off sqlite3NestedParse Kirill Shcherbatov
2018-07-10 17:08 ` [tarantool-patches] [PATCH v2 1/4] sql: get rid off sqlite3NestedParse in clean stats Kirill Shcherbatov
2018-07-10 17:08 ` [tarantool-patches] [PATCH v2 2/4] sql: remove usless sqlite3NestedParse function Kirill Shcherbatov
@ 2018-07-10 17:08 ` Kirill Shcherbatov
2018-07-10 18:22 ` [tarantool-patches] " n.pettik
2018-07-10 17:08 ` [tarantool-patches] [PATCH v2 4/4] sql: remove OP_LoadPtr Kirill Shcherbatov
2018-07-11 13:45 ` [tarantool-patches] Re: [PATCH v2 0/4] sql: get rid off sqlite3NestedParse Kirill Yukhin
4 siblings, 1 reply; 18+ messages in thread
From: Kirill Shcherbatov @ 2018-07-10 17:08 UTC (permalink / raw)
To: tarantool-patches; +Cc: korablev, Kirill Shcherbatov
Made vdbe_emit_open_cursor calls consistent:
now it uses index id everywhere.
This required to change a way to detect that
VDBE has openned Read cursor to specified table
in vdbe_has_table_read to write result of insert
in temp table if required.
---
src/box/sql/analyze.c | 6 +--
src/box/sql/build.c | 3 +-
src/box/sql/delete.c | 8 +---
src/box/sql/expr.c | 5 ++-
src/box/sql/fkey.c | 3 +-
src/box/sql/insert.c | 100 +++++++++++++++++++++++++++++-------------------
src/box/sql/pragma.c | 7 +++-
src/box/sql/select.c | 4 +-
src/box/sql/sqliteInt.h | 6 +--
src/box/sql/vdbe.c | 5 +--
src/box/sql/where.c | 10 ++++-
11 files changed, 92 insertions(+), 65 deletions(-)
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 336d146..067a86e 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -159,8 +159,7 @@ vdbe_emit_stat_space_open(struct Parse *parse, int stat_cursor,
/* Open the sql_stat tables for writing. */
for (uint i = 0; i < lengthof(space_names); ++i) {
uint32_t id = space_ids[i];
- int tnum = SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(id, 0);
- vdbe_emit_open_cursor(parse, stat_cursor + i, tnum,
+ vdbe_emit_open_cursor(parse, stat_cursor + i, 0,
space_by_id(id));
VdbeComment((v, space_names[i]));
}
@@ -893,10 +892,11 @@ analyzeOneTable(Parse * pParse, /* Parser context */
/* Open a read-only cursor on the index being analyzed. */
struct space *space =
space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
+ int idx_id = SQLITE_PAGENO_TO_INDEXID(pIdx->tnum);
assert(space != NULL);
sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0,
(void*)space, P4_SPACEPTR);
- sqlite3VdbeAddOp3(v, OP_OpenRead, iIdxCur, pIdx->tnum,
+ sqlite3VdbeAddOp3(v, OP_OpenRead, iIdxCur, idx_id,
space_ptr_reg);
sql_vdbe_set_p4_key_def(pParse, pIdx);
VdbeComment((v, "%s", pIdx->zName));
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 1c00842..5f7a35a 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2448,7 +2448,8 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int memRootPage)
sqlite3VdbeAddOp2(v, OP_Clear, SQLITE_PAGENO_TO_SPACEID(tnum),
0);
struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(tnum));
- vdbe_emit_open_cursor(pParse, iIdx, tnum, space);
+ vdbe_emit_open_cursor(pParse, iIdx, SQLITE_PAGENO_TO_INDEXID(tnum),
+ space);
sqlite3VdbeChangeP5(v, memRootPage >= 0 ? OPFLAG_P2ISREG : 0);
addr1 = sqlite3VdbeAddOp2(v, OP_SorterSort, iSorter, 0);
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 66dc0fc..d1c5935 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -340,18 +340,14 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0,
(void *)space, P4_SPACEPTR);
- int tnum =
- SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(space_id,
- 0);
- sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor,
- tnum, space_ptr_reg);
+ sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor, 0,
+ space_ptr_reg);
struct key_def *def = key_def_dup(pk_def);
if (def == NULL) {
sqlite3OomFault(parse->db);
goto delete_from_cleanup;
}
sqlite3VdbeAppendP4(v, def, P4_KEYDEF);
-
VdbeComment((v, "%s", space->index[0]->def->name));
if (one_pass == ONEPASS_MULTI)
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 3183e3d..b1650cf 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2470,8 +2470,11 @@ sqlite3FindInIndex(Parse * pParse, /* Parsing context */
P4_DYNAMIC);
struct space *space =
space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
+ uint32_t idx_id =
+ SQLITE_PAGENO_TO_INDEXID(pIdx->
+ tnum);
vdbe_emit_open_cursor(pParse, iTab,
- pIdx->tnum, space);
+ idx_id, space);
VdbeComment((v, "%s", pIdx->zName));
assert(IN_INDEX_INDEX_DESC ==
IN_INDEX_INDEX_ASC + 1);
diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
index 6c75c47..face9cb 100644
--- a/src/box/sql/fkey.c
+++ b/src/box/sql/fkey.c
@@ -441,7 +441,8 @@ fkLookupParent(Parse * pParse, /* Parse context */
int regRec = sqlite3GetTempReg(pParse);
struct space *space =
space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
- vdbe_emit_open_cursor(pParse, iCur, pIdx->tnum, space);
+ uint32_t idx_id = SQLITE_PAGENO_TO_INDEXID(pIdx->tnum);
+ vdbe_emit_open_cursor(pParse, iCur, idx_id, space);
for (i = 0; i < nCol; i++) {
sqlite3VdbeAddOp2(v, OP_Copy,
aiCol[i] + 1 + regData,
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 58e159c..62b85d5 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -179,41 +179,51 @@ sqlite3TableAffinity(Vdbe * v, Table * pTab, int iReg)
}
}
-/*
- * Return non-zero if the table pTab in database or any of its indices
- * have been opened at any point in the VDBE program. This is used to see if
- * a statement of the form "INSERT INTO <pTab> SELECT ..." can
- * run for the results of the SELECT.
+/**
+ * This routine is used to see if a statement of the form
+ * "INSERT INTO <table> SELECT ..." can run for the results of the
+ * SELECT.
+ *
+ * @param parser Parse context.
+ * @param table Table AST object.
+ * @retval true if the table table in database or any of its
+ * indices have been opened at any point in the VDBE
+ * program.
+ * @retval false else.
*/
-static int
-readsTable(Parse * p, Table * pTab)
+static bool
+vdbe_has_table_read(struct Parse *parser, const struct Table *table)
{
- Vdbe *v = sqlite3GetVdbe(p);
- int i;
- int iEnd = sqlite3VdbeCurrentAddr(v);
-
- for (i = 1; i < iEnd; i++) {
- VdbeOp *pOp = sqlite3VdbeGetOp(v, i);
- assert(pOp != 0);
- /* Currently, there is no difference between
- * Read and Write cursors.
+ struct Vdbe *v = sqlite3GetVdbe(parser);
+ int last_instr = sqlite3VdbeCurrentAddr(v);
+ for (int i = 1; i < last_instr; i++) {
+ struct VdbeOp *op = sqlite3VdbeGetOp(v, i);
+ assert(op != NULL);
+ /*
+ * Currently, there is no difference between Read
+ * and Write cursors.
*/
- if (pOp->opcode == OP_OpenRead ||
- pOp->opcode == OP_OpenWrite) {
- Index *pIndex;
- int tnum = pOp->p2;
- if (tnum == pTab->tnum) {
- return 1;
- }
- for (pIndex = pTab->pIndex; pIndex;
- pIndex = pIndex->pNext) {
- if (tnum == pIndex->tnum) {
- return 1;
- }
+ if (op->opcode == OP_OpenRead || op->opcode == OP_OpenWrite) {
+ assert(i > 1);
+ struct VdbeOp *space_var_op =
+ sqlite3VdbeGetOp(v, i - 1);
+ assert(space_var_op != NULL);
+ assert(space_var_op->opcode == OP_LoadPtr);
+ struct space *space = space_var_op->p4.space;
+
+ if (space->def->id == table->def->id)
+ return true;
+
+ int idx_id = op->p2;
+ for (struct Index *pIndex = table->pIndex;
+ pIndex != NULL; pIndex = pIndex->pNext) {
+ if (idx_id ==
+ SQLITE_PAGENO_TO_INDEXID(pIndex->tnum))
+ return true;
}
}
}
- return 0;
+ return false;
}
@@ -526,16 +536,20 @@ sqlite3Insert(Parse * pParse, /* Parser context */
assert(pSelect->pEList);
nColumn = pSelect->pEList->nExpr;
- /* Set useTempTable to TRUE if the result of the SELECT statement
- * should be written into a temporary table (template 4). Set to
- * FALSE if each output row of the SELECT can be written directly into
- * the destination table (template 3).
+ /*
+ * Set useTempTable to TRUE if the result of the
+ * SELECT statement should be written into a
+ * temporary table (template 4). Set to FALSE if
+ * each output row of the SELECT can be written
+ * directly into the destination table
+ * (template 3).
*
- * A temp table must be used if the table being updated is also one
- * of the tables being read by the SELECT statement. Also use a
- * temp table in the case of row triggers.
+ * A temp table must be used if the table being
+ * updated is also one of the tables being read by
+ * the SELECT statement. Also use a temp table in
+ * the case of row triggers.
*/
- if (trigger != NULL || readsTable(pParse, pTab))
+ if (trigger != NULL || vdbe_has_table_read(pParse, pTab))
useTempTable = 1;
if (useTempTable) {
@@ -1617,7 +1631,9 @@ sqlite3OpenTableAndIndices(Parse * pParse, /* Parsing context */
p5 = 0;
}
if (aToOpen == 0 || aToOpen[i + 1]) {
- sqlite3VdbeAddOp3(v, op, iIdxCur, pIdx->tnum,
+ int idx_id =
+ SQLITE_PAGENO_TO_INDEXID(pIdx->tnum);
+ sqlite3VdbeAddOp3(v, op, iIdxCur, idx_id,
space_ptr_reg);
sql_vdbe_set_p4_key_def(pParse, pIdx);
sqlite3VdbeChangeP5(v, p5);
@@ -1934,11 +1950,15 @@ xferOptimization(Parse * pParse, /* Parser context */
assert(pSrcIdx);
struct space *src_space =
space_by_id(SQLITE_PAGENO_TO_SPACEID(pSrcIdx->tnum));
- vdbe_emit_open_cursor(pParse, iSrc, pSrcIdx->tnum, src_space);
+ vdbe_emit_open_cursor(pParse, iSrc,
+ SQLITE_PAGENO_TO_INDEXID(pSrcIdx->tnum),
+ src_space);
VdbeComment((v, "%s", pSrcIdx->zName));
struct space *dest_space =
space_by_id(SQLITE_PAGENO_TO_SPACEID(pDestIdx->tnum));
- vdbe_emit_open_cursor(pParse, iDest, pDestIdx->tnum, dest_space);
+ vdbe_emit_open_cursor(pParse, iDest,
+ SQLITE_PAGENO_TO_INDEXID(pDestIdx->tnum),
+ dest_space);
VdbeComment((v, "%s", pDestIdx->zName));
addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iSrc, 0);
VdbeCoverage(v);
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 31581b1..ca119e0 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -685,11 +685,14 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
pParent,
OP_OpenRead);
} else {
+ int idx_id =
+ SQLITE_PAGENO_TO_INDEXID(
+ pIdx->
+ tnum);
sqlite3VdbeAddOp3(v,
OP_OpenRead,
i,
- pIdx->
- tnum,
+ idx_id,
0);
sql_vdbe_set_p4_key_def(pParse,
pIdx);
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 52b3fdd..ceb7e34 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -6279,9 +6279,7 @@ sqlite3Select(Parse * pParse, /* The parser context */
* Open the cursor, execute the OP_Count,
* close the cursor.
*/
- vdbe_emit_open_cursor(pParse, cursor,
- space->def->id << 10,
- space);
+ vdbe_emit_open_cursor(pParse, cursor, 0, space);
sqlite3VdbeAddOp2(v, OP_Count, cursor,
sAggInfo.aFunc[0].iMem);
sqlite3VdbeAddOp1(v, OP_Close, cursor);
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 380c9c6..4a1eef0 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3583,9 +3583,9 @@ void sqlite3EndTable(Parse *, Token *, Token *, Select *);
*
* @param parse_context Parse context.
* @param cursor Number of cursor to be created.
- * @param index_id Encoded index id (encoding is void actually, so
- * pas it as is). In future will be replaced with pointer
- * to struct index.
+ * @param index_id index id. In future will be replaced with
+ * pointer to struct index.
+ * @param space Pointer to space object.
* @retval address of last opcode.
*/
int
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 7a4d376..a0eb41b 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3185,9 +3185,8 @@ case OP_ReopenIdx: {
pIn3 = &aMem[pOp->p3];
assert(pIn3->flags & MEM_Ptr);
if (pCur && pCur->uc.pCursor->space == (struct space *) pIn3->u.p &&
- pCur->uc.pCursor->index->def->iid == SQLITE_PAGENO_TO_INDEXID(p2)) {
+ pCur->uc.pCursor->index->def->iid == (uint32_t)p2)
goto open_cursor_set_hints;
- }
/* If the cursor is not currently open or is open on a different
* index, then fall through into OP_OpenRead to force a reopen
*/
@@ -3213,7 +3212,7 @@ case OP_OpenWrite:
assert(pIn3->flags & MEM_Ptr);
struct space *space = ((struct space *) pIn3->u.p);
assert(space != NULL);
- struct index *index = space_index(space, SQLITE_PAGENO_TO_INDEXID(p2));
+ struct index *index = space_index(space, p2);
assert(index != NULL);
/*
* Since Tarantool iterator provides the full tuple,
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 85143ed..6c07fec 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -4759,10 +4759,16 @@ sqlite3WhereBegin(Parse * pParse, /* The parser context */
assert(iIndexCur >= 0);
if (op) {
if (pIx != NULL) {
+ uint32_t space_id =
+ SQLITE_PAGENO_TO_SPACEID(pIx->
+ tnum);
struct space *space =
- space_by_id(SQLITE_PAGENO_TO_SPACEID(pIx->tnum));
+ space_by_id(space_id);
+ uint32_t idx_id =
+ SQLITE_PAGENO_TO_INDEXID(pIx->
+ tnum);
vdbe_emit_open_cursor(pParse, iIndexCur,
- pIx->tnum, space);
+ idx_id, space);
} else {
vdbe_emit_open_cursor(pParse, iIndexCur,
idx_def->iid,
--
2.7.4
^ permalink raw reply [flat|nested] 18+ messages in thread* [tarantool-patches] Re: [PATCH v2 3/4] sql: refactor vdbe_emit_open_cursor calls
2018-07-10 17:08 ` [tarantool-patches] [PATCH v2 3/4] sql: refactor vdbe_emit_open_cursor calls Kirill Shcherbatov
@ 2018-07-10 18:22 ` n.pettik
2018-07-11 7:22 ` Kirill Shcherbatov
0 siblings, 1 reply; 18+ messages in thread
From: n.pettik @ 2018-07-10 18:22 UTC (permalink / raw)
To: tarantool-patches; +Cc: Kirill Shcherbatov
> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> index 58e159c..62b85d5 100644
> --- a/src/box/sql/insert.c
> +++ b/src/box/sql/insert.c
> @@ -179,41 +179,51 @@ sqlite3TableAffinity(Vdbe * v, Table * pTab, int iReg)
> -static int
> -readsTable(Parse * p, Table * pTab)
> +static bool
> +vdbe_has_table_read(struct Parse *parser, const struct Table *table)
> {
> + if (op->opcode == OP_OpenRead || op->opcode == OP_OpenWrite) {
> + assert(i > 1);
> + struct VdbeOp *space_var_op =
> + sqlite3VdbeGetOp(v, i - 1);
> + assert(space_var_op != NULL);
> + assert(space_var_op->opcode == OP_LoadPtr);
> + struct space *space = space_var_op->p4.space;
> +
> + if (space->def->id == table->def->id)
> + return true;
> +
> + int idx_id = op->p2;
> + for (struct Index *pIndex = table->pIndex;
> + pIndex != NULL; pIndex = pIndex->pNext) {
> + if (idx_id ==
> + SQLITE_PAGENO_TO_INDEXID(pIndex->tnum))
> + return true;
Why do you need these index iterations? If spaces don’t match, what is the
point to check index ids? Btw, I deleted this code and all tests seem to pass.
The rest is OK in this patch.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [tarantool-patches] [PATCH v2 4/4] sql: remove OP_LoadPtr
2018-07-10 17:08 [tarantool-patches] [PATCH v2 0/4] sql: get rid off sqlite3NestedParse Kirill Shcherbatov
` (2 preceding siblings ...)
2018-07-10 17:08 ` [tarantool-patches] [PATCH v2 3/4] sql: refactor vdbe_emit_open_cursor calls Kirill Shcherbatov
@ 2018-07-10 17:08 ` Kirill Shcherbatov
2018-07-10 18:34 ` [tarantool-patches] " n.pettik
2018-07-11 13:45 ` [tarantool-patches] Re: [PATCH v2 0/4] sql: get rid off sqlite3NestedParse Kirill Yukhin
4 siblings, 1 reply; 18+ messages in thread
From: Kirill Shcherbatov @ 2018-07-10 17:08 UTC (permalink / raw)
To: tarantool-patches; +Cc: korablev, Vladislav Shpilevoy
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
This opcode was used when Vdbe had to store key_def in P4 for
OP_OpenRead/Write, so P4 was occupied and OP_LoadPtr was used to
store space pointer in another opcode. But now P4 is free for
OpenRead/Write and space pointer can be stored here.
Alongside, some useless key_def duplications are removed.
---
src/box/sql/analyze.c | 8 ++----
src/box/sql/build.c | 20 +++++----------
src/box/sql/delete.c | 15 ++---------
src/box/sql/insert.c | 18 +++----------
src/box/sql/pragma.c | 39 ++++++++++++++--------------
src/box/sql/sqliteInt.h | 5 ----
src/box/sql/vdbe.c | 68 +++++--------------------------------------------
7 files changed, 40 insertions(+), 133 deletions(-)
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 067a86e..699a4e8 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -794,7 +794,6 @@ analyzeOneTable(Parse * pParse, /* Parser context */
int iTabCur; /* Table cursor */
Vdbe *v; /* The virtual machine being built up */
int i; /* Loop counter */
- int space_ptr_reg = iMem++;
int regStat4 = iMem++; /* Register to hold Stat4Accum object */
int regChng = iMem++; /* Index of changed index field */
int regKey = iMem++; /* Key argument passed to stat_push() */
@@ -894,11 +893,8 @@ analyzeOneTable(Parse * pParse, /* Parser context */
space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
int idx_id = SQLITE_PAGENO_TO_INDEXID(pIdx->tnum);
assert(space != NULL);
- sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0,
- (void*)space, P4_SPACEPTR);
- sqlite3VdbeAddOp3(v, OP_OpenRead, iIdxCur, idx_id,
- space_ptr_reg);
- sql_vdbe_set_p4_key_def(pParse, pIdx);
+ sqlite3VdbeAddOp4(v, OP_OpenRead, iIdxCur, idx_id, 0,
+ (void *) space, P4_SPACEPTR);
VdbeComment((v, "%s", pIdx->zName));
/* Invoke the stat_init() function. The arguments are:
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 5f7a35a..26af0fe 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1108,12 +1108,8 @@ vdbe_emit_open_cursor(struct Parse *parse_context, int cursor, int index_id,
struct space *space)
{
assert(space != NULL);
- struct Vdbe *vdbe = parse_context->pVdbe;
- int space_ptr_reg = ++parse_context->nMem;
- sqlite3VdbeAddOp4(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space,
- P4_SPACEPTR);
- return sqlite3VdbeAddOp3(vdbe, OP_OpenWrite, cursor, index_id,
- space_ptr_reg);
+ return sqlite3VdbeAddOp4(parse_context->pVdbe, OP_OpenWrite, cursor,
+ index_id, 0, (void *) space, P4_SPACEPTR);
}
/*
* Generate code that will increment the schema cookie.
@@ -2961,7 +2957,6 @@ sql_create_index(struct Parse *parse, struct Token *token,
Vdbe *v;
char *zStmt;
int iCursor = parse->nTab++;
- int index_space_ptr_reg = parse->nTab++;
int iSpaceId, iIndexId, iFirstSchemaCol;
v = sqlite3GetVdbe(parse);
@@ -2969,12 +2964,9 @@ sql_create_index(struct Parse *parse, struct Token *token,
goto exit_create_index;
sql_set_multi_write(parse, true);
-
-
- sqlite3VdbeAddOp2(v, OP_SIDtoPtr, BOX_INDEX_ID,
- index_space_ptr_reg);
- sqlite3VdbeAddOp4Int(v, OP_OpenWrite, iCursor, 0,
- index_space_ptr_reg, 6);
+ sqlite3VdbeAddOp4(v, OP_OpenWrite, iCursor, 0, 0,
+ (void *)space_by_id(BOX_INDEX_ID),
+ P4_SPACEPTR);
sqlite3VdbeChangeP5(v, OPFLAG_SEEKEQ);
/*
@@ -3943,7 +3935,7 @@ vdbe_emit_halt_with_presence_test(struct Parse *parser, int space_id,
int cursor = parser->nTab++;
vdbe_emit_open_cursor(parser, cursor, index_id, space_by_id(space_id));
- int name_reg = parser->nMem++;
+ int name_reg = ++parser->nMem;
int label = sqlite3VdbeAddOp4(v, OP_String8, 0, name_reg, 0, name,
P4_DYNAMIC);
sqlite3VdbeAddOp4Int(v, cond_opcode, cursor, label + 3, name_reg, 1);
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index d1c5935..ca1e77d 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -335,19 +335,8 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
iAddrOnce = sqlite3VdbeAddOp0(v, OP_Once);
VdbeCoverage(v);
}
-
- int space_ptr_reg = ++parse->nMem;
- sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0,
- (void *)space, P4_SPACEPTR);
-
- sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor, 0,
- space_ptr_reg);
- struct key_def *def = key_def_dup(pk_def);
- if (def == NULL) {
- sqlite3OomFault(parse->db);
- goto delete_from_cleanup;
- }
- sqlite3VdbeAppendP4(v, def, P4_KEYDEF);
+ sqlite3VdbeAddOp4(v, OP_OpenWrite, tab_cursor, 0, 0,
+ (void *) space, P4_SPACEPTR);
VdbeComment((v, "%s", space->index[0]->def->name));
if (one_pass == ONEPASS_MULTI)
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 62b85d5..6aede16 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -204,13 +204,8 @@ vdbe_has_table_read(struct Parse *parser, const struct Table *table)
* and Write cursors.
*/
if (op->opcode == OP_OpenRead || op->opcode == OP_OpenWrite) {
- assert(i > 1);
- struct VdbeOp *space_var_op =
- sqlite3VdbeGetOp(v, i - 1);
- assert(space_var_op != NULL);
- assert(space_var_op->opcode == OP_LoadPtr);
- struct space *space = space_var_op->p4.space;
-
+ assert(op->p4type == P4_SPACEPTR);
+ struct space *space = op->p4.space;
if (space->def->id == table->def->id)
return true;
@@ -1581,10 +1576,6 @@ sqlite3OpenTableAndIndices(Parse * pParse, /* Parsing context */
*piIdxCur = iBase;
struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(pTab->tnum));
assert(space != NULL);
- int space_ptr_reg = ++pParse->nMem;
- sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space,
- P4_SPACEPTR);
-
/* One iteration of this cycle adds OpenRead/OpenWrite which
* opens cursor for current index.
*
@@ -1633,9 +1624,8 @@ sqlite3OpenTableAndIndices(Parse * pParse, /* Parsing context */
if (aToOpen == 0 || aToOpen[i + 1]) {
int idx_id =
SQLITE_PAGENO_TO_INDEXID(pIdx->tnum);
- sqlite3VdbeAddOp3(v, op, iIdxCur, idx_id,
- space_ptr_reg);
- sql_vdbe_set_p4_key_def(pParse, pIdx);
+ sqlite3VdbeAddOp4(v, op, iIdxCur, idx_id, 0,
+ (void *) space, P4_SPACEPTR);
sqlite3VdbeChangeP5(v, p5);
VdbeComment((v, "%s", pIdx->zName));
}
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index ca119e0..8ff5bbc 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -678,29 +678,28 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
x = sqlite3FkLocateIndex(pParse,
pParent, pFK,
&pIdx, 0);
- if (x == 0) {
- if (pIdx == 0) {
- sqlite3OpenTable(pParse,
- i,
- pParent,
- OP_OpenRead);
- } else {
- int idx_id =
- SQLITE_PAGENO_TO_INDEXID(
- pIdx->
- tnum);
- sqlite3VdbeAddOp3(v,
- OP_OpenRead,
- i,
- idx_id,
- 0);
- sql_vdbe_set_p4_key_def(pParse,
- pIdx);
- }
- } else {
+ if (x != 0) {
k = 0;
break;
}
+ if (pIdx == NULL) {
+ sqlite3OpenTable(pParse, i,
+ pParent,
+ OP_OpenRead);
+ continue;
+ }
+ struct space *space =
+ space_cache_find(pIdx->pTable->
+ def->id);
+ int idx_id =
+ SQLITE_PAGENO_TO_INDEXID(pIdx->
+ tnum);
+ assert(space != NULL);
+ sqlite3VdbeAddOp4(v, OP_OpenRead, i,
+ idx_id, 0,
+ (void *) space,
+ P4_SPACEPTR);
+
}
assert(pParse->nErr > 0 || pFK == 0);
if (pFK)
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 4a1eef0..18bf949 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3008,11 +3008,6 @@ struct Parse {
#define OPFLAG_NOOP_IF_NULL 0x02 /* OP_FCopy: if source register is NULL
* then do nothing
*/
-#define OPFLAG_FRESH_PTR 0x20 /* OP_Open**: set if space pointer
- * comes from OP_SIDtoPtr, i.e. it
- * is fresh, even in case schema
- * changes previously.
- */
/*
* Each trigger present in the database schema is stored as an
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index a0eb41b..49a17b9 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1077,19 +1077,6 @@ case OP_Int64: { /* out2 */
break;
}
-/* Opcode: LoadPtr * P2 * P4 *
- * Synopsis: r[P2] = P4
- *
- * P4 is a generic or space pointer. Copy it into register P2.
- */
-case OP_LoadPtr: {
- pOut = out2Prerelease(p, pOp);
- assert(pOp->p4type == P4_PTR || pOp->p4type == P4_SPACEPTR );
- pOut->u.p = pOp->p4.space;
- pOut->flags = MEM_Ptr;
- break;
-}
-
#ifndef SQLITE_OMIT_FLOATING_POINT
/* Opcode: Real * P2 * P4 *
* Synopsis: r[P2]=P4
@@ -3137,26 +3124,15 @@ case OP_SetCookie: {
}
/* Opcode: OpenRead P1 P2 P3 P4 P5
- * Synopsis: index id = P2, space ptr = P3
+ * Synopsis: index id = P2, space ptr = P4
*
- * Open a cursor for a space specified by pointer in P3 and index
+ * Open a cursor for a space specified by pointer in P4 and index
* id in P2. Give the new cursor an identifier of P1. The P1
* values need not be contiguous but all P1 values should be
* small integers. It is an error for P1 to be negative.
- *
- * The P4 value may be a pointer to a key_def structure.
- * If it is a pointer to a key_def structure, then said structure
- * defines the content and collatining sequence of the index
- * being opened. Otherwise, P4 is NULL.
- *
- * If schema has changed since compile time, VDBE ends execution
- * with appropriate error message. The only exception is
- * when P5 is set to OPFLAG_FRESH_PTR, which means that
- * space pointer has been fetched in runtime right before
- * this opcode.
*/
/* Opcode: ReopenIdx P1 P2 P3 P4 P5
- * Synopsis: index id = P2, space ptr = P3
+ * Synopsis: index id = P2, space ptr = P4
*
* The ReopenIdx opcode works exactly like OpenRead except that
* it first checks to see if the cursor on P1 is already open
@@ -3166,7 +3142,7 @@ case OP_SetCookie: {
* The ReopenIdx opcode may only be used with P5 == 0.
*/
/* Opcode: OpenWrite P1 P2 P3 P4 P5
- * Synopsis: index id = P2, space ptr = P3
+ * Synopsis: index id = P2, space ptr = P4
*
* For now, OpenWrite is an alias for OpenRead.
* It exists just due legacy reasons and should be removed:
@@ -3182,9 +3158,7 @@ case OP_ReopenIdx: {
assert(pOp->p5==0 || pOp->p5==OPFLAG_SEEKEQ);
pCur = p->apCsr[pOp->p1];
p2 = pOp->p2;
- pIn3 = &aMem[pOp->p3];
- assert(pIn3->flags & MEM_Ptr);
- if (pCur && pCur->uc.pCursor->space == (struct space *) pIn3->u.p &&
+ if (pCur && pCur->uc.pCursor->space == pOp->p4.space &&
pCur->uc.pCursor->index->def->iid == (uint32_t)p2)
goto open_cursor_set_hints;
/* If the cursor is not currently open or is open on a different
@@ -3194,13 +3168,7 @@ case OP_OpenRead:
case OP_OpenWrite:
assert(pOp->opcode==OP_OpenWrite || pOp->p5==0 || pOp->p5==OPFLAG_SEEKEQ);
- /*
- * Even if schema has changed, pointer can come from
- * OP_SIDtoPtr opcode, which converts space id to pointer
- * during runtime.
- */
- if (box_schema_version() != p->schema_ver &&
- (pOp->p5 & OPFLAG_FRESH_PTR) == 0) {
+ if (box_schema_version() != p->schema_ver) {
p->expired = 1;
rc = SQLITE_ERROR;
sqlite3VdbeError(p, "schema version has changed: " \
@@ -3208,9 +3176,7 @@ case OP_OpenWrite:
goto abort_due_to_error;
}
p2 = pOp->p2;
- pIn3 = &aMem[pOp->p3];
- assert(pIn3->flags & MEM_Ptr);
- struct space *space = ((struct space *) pIn3->u.p);
+ struct space *space = pOp->p4.space;
assert(space != NULL);
struct index *index = space_index(space, p2);
assert(index != NULL);
@@ -4461,26 +4427,6 @@ case OP_SDelete: {
break;
}
-/* Opcode: SIDtoPtr P1 P2 * * *
- * Synopsis: space id = P1, space[out] = r[P2]
- *
- * This opcode makes look up by space id and save found space
- * into register, specified by the content of register P2.
- * Such trick is needed during DLL routine, since schema may
- * change and pointers become expired.
- */
-case OP_SIDtoPtr: {
- assert(pOp->p1 > 0);
- assert(pOp->p2 >= 0);
-
- pIn2 = out2Prerelease(p, pOp);
- struct space *space = space_by_id(pOp->p1);
- assert(space != NULL);
- pIn2->u.p = (void *) space;
- pIn2->flags = MEM_Ptr;
- break;
-}
-
/* Opcode: IdxDelete P1 P2 P3 * *
* Synopsis: key=r[P2@P3]
*
--
2.7.4
^ permalink raw reply [flat|nested] 18+ messages in thread* [tarantool-patches] Re: [PATCH v2 4/4] sql: remove OP_LoadPtr
2018-07-10 17:08 ` [tarantool-patches] [PATCH v2 4/4] sql: remove OP_LoadPtr Kirill Shcherbatov
@ 2018-07-10 18:34 ` n.pettik
2018-07-10 20:23 ` Vladislav Shpilevoy
0 siblings, 1 reply; 18+ messages in thread
From: n.pettik @ 2018-07-10 18:34 UTC (permalink / raw)
To: tarantool-patches; +Cc: Vladislav Shpilevoy
> @@ -2969,12 +2964,9 @@ sql_create_index(struct Parse *parse, struct Token *token,
> goto exit_create_index;
>
> sql_set_multi_write(parse, true);
> -
> -
> - sqlite3VdbeAddOp2(v, OP_SIDtoPtr, BOX_INDEX_ID,
> - index_space_ptr_reg);
> - sqlite3VdbeAddOp4Int(v, OP_OpenWrite, iCursor, 0,
> - index_space_ptr_reg, 6);
> + sqlite3VdbeAddOp4(v, OP_OpenWrite, iCursor, 0, 0,
> + (void *)space_by_id(BOX_INDEX_ID),
> + P4_SPACEPTR);
> sqlite3VdbeChangeP5(v, OPFLAG_SEEKEQ);
Wait, AFAIK SIDtoPtr was used deliberately taking into consideration
the fact that DDL may change ptr to space. So conversion id -> space ptr
is delayed until vdbe execution (right before cursor opening). Am I wrong?
^ permalink raw reply [flat|nested] 18+ messages in thread
* [tarantool-patches] Re: [PATCH v2 4/4] sql: remove OP_LoadPtr
2018-07-10 18:34 ` [tarantool-patches] " n.pettik
@ 2018-07-10 20:23 ` Vladislav Shpilevoy
2018-07-10 20:34 ` n.pettik
0 siblings, 1 reply; 18+ messages in thread
From: Vladislav Shpilevoy @ 2018-07-10 20:23 UTC (permalink / raw)
To: n.pettik, tarantool-patches
On 10/07/2018 21:34, n.pettik wrote:
>
>> @@ -2969,12 +2964,9 @@ sql_create_index(struct Parse *parse, struct Token *token,
>> goto exit_create_index;
>>
>> sql_set_multi_write(parse, true);
>> -
>> -
>> - sqlite3VdbeAddOp2(v, OP_SIDtoPtr, BOX_INDEX_ID,
>> - index_space_ptr_reg);
>> - sqlite3VdbeAddOp4Int(v, OP_OpenWrite, iCursor, 0,
>> - index_space_ptr_reg, 6);
>> + sqlite3VdbeAddOp4(v, OP_OpenWrite, iCursor, 0, 0,
>> + (void *)space_by_id(BOX_INDEX_ID),
>> + P4_SPACEPTR);
>> sqlite3VdbeChangeP5(v, OPFLAG_SEEKEQ);
>
> Wait, AFAIK SIDtoPtr was used deliberately taking into consideration
> the fact that DDL may change ptr to space. So conversion id -> space ptr
> is delayed until vdbe execution (right before cursor opening). Am I wrong?
Yes, you are wrong here, because it was used to restore pointer to
struct space "_index", but "_index" is a system space, and it is never
altered. So pointer to "_index" is always valid, and it makes no sense
to restore it on runtime.
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [tarantool-patches] Re: [PATCH v2 0/4] sql: get rid off sqlite3NestedParse
2018-07-10 17:08 [tarantool-patches] [PATCH v2 0/4] sql: get rid off sqlite3NestedParse Kirill Shcherbatov
` (3 preceding siblings ...)
2018-07-10 17:08 ` [tarantool-patches] [PATCH v2 4/4] sql: remove OP_LoadPtr Kirill Shcherbatov
@ 2018-07-11 13:45 ` Kirill Yukhin
4 siblings, 0 replies; 18+ messages in thread
From: Kirill Yukhin @ 2018-07-11 13:45 UTC (permalink / raw)
To: tarantool-patches; +Cc: korablev, Kirill Shcherbatov
Hello,
On 10 июл 20:08, Kirill Shcherbatov wrote:
> As we are going to implement parser as separate library,
> we should get rid off sqlite3NestedParse calls.
> Last usages in statistics update could be trivially rewrited
> to do not start parsing. We manually build AST structures
> simmilar to parser did and manually call sql_table_delete_from.
> Finilly, I've removed all sqlite3NestedParse mentionings.
> Also slightly refactored OP_Open{Write,Read} opcodes and
> vdbe_emit_open_cursor function to use index_id instead of tnum.
>
> Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-3496-no-nested-parse
> Issue: https://github.com/tarantool/tarantool/issues/3496
I've pushed the patchset into 2.0 branch.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 18+ messages in thread