[tarantool-patches] Re: [PATCH v2 1/4] sql: get rid off sqlite3NestedParse in clean stats
Kirill Shcherbatov
kshcherbatov at tarantool.org
Wed Jul 11 10:22:33 MSK 2018
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]);
More information about the Tarantool-patches
mailing list