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 409742520E for ; Fri, 11 May 2018 13:29:38 -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 BEkVfgCr-qTk for ; Fri, 11 May 2018 13:29:38 -0400 (EDT) Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (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 ED8F725207 for ; Fri, 11 May 2018 13:29:37 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH 3/4] sql: refactor usages of table's tuple count From: n.pettik In-Reply-To: Date: Fri, 11 May 2018 20:29:36 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <9A22704F-CD79-4115-B034-6A7322DDD51A@tarantool.org> References: <7c93d6e8d43dbf9c912daca4ff1762d1fe412b35.1524515002.git.korablev@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: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy >> +#ifdef DEFAULT_TUPLE_COUNT >> +#undef DEFAULT_TUPLE_COUNT >=20 > 1. Why do you need these guards? sqliteInt.h already has #ifndef = SQLITEINT_H > #define SQLITEINT_H guard. If it does not help, then it is error and = must be > fixed with no ifdef + undef for each definition. Just for case. Since you don=E2=80=99t like this way, I have removed = them: @@ -2206,15 +2206,9 @@ struct IndexSample { tRowcnt *anDLt; /* Est. number of distinct keys less = than this sample */ }; =20 -#ifdef DEFAULT_TUPLE_COUNT -#undef DEFAULT_TUPLE_COUNT -#endif #define DEFAULT_TUPLE_COUNT 1048576 - -#ifdef DEFAULT_TUPLE_LOG_COUNT -#undef DEFAULT_TUPLE_LOG_COUNT -#endif -#define DEFAULT_TUPLE_LOG_COUNT sqlite3LogEst(DEFAULT_TUPLE_COUNT) +/** [10*log_{2}(1048576)] =3D=3D 200 */ +#define DEFAULT_TUPLE_LOG_COUNT 200 >=20 >> +#endif >> +#define DEFAULT_TUPLE_COUNT 1048576 >> + >> +#ifdef DEFAULT_TUPLE_LOG_COUNT >> +#undef DEFAULT_TUPLE_LOG_COUNT >> +#endif >> +#define DEFAULT_TUPLE_LOG_COUNT sqlite3LogEst(DEFAULT_TUPLE_COUNT) >=20 > 2. In the original code it was 200 to avoid calculation of > sqlite3LogEst(1048576) on each temporary table creation. Lets do the = same. I did it just to easily adjusting tuple count without carrying about = logarithms. M=E2=80=99kay, added two independent macroses and appropriate assertion = on each usage (so release build will not be affected by calling sqlite3LogEst()):=20 +++ b/src/box/sql/analyze.c @@ -1636,6 +1636,7 @@ sql_space_tuple_log_count(struct Table *tab) if (space =3D=3D NULL) return tab->tuple_log_count; struct index *pk =3D space_index(space, 0); + assert(sqlite3LogEst(DEFAULT_TUPLE_COUNT) =3D=3D = DEFAULT_TUPLE_LOG_COUNT); /* If space represents VIEW, return default number. */ if (pk =3D=3D NULL) return DEFAULT_TUPLE_LOG_COUNT; diff --git a/src/box/sql/select.c b/src/box/sql/select.c index db3fb3948..c02322b40 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1966,6 +1966,7 @@ sqlite3ResultSetOfSelect(Parse * pParse, Select * = pSelect) pTab->nTabRef =3D 1; pTab->zName =3D 0; pTab->tuple_log_count =3D DEFAULT_TUPLE_LOG_COUNT; + assert(sqlite3LogEst(DEFAULT_TUPLE_COUNT) =3D=3D = DEFAULT_TUPLE_LOG_COUNT); sqlite3ColumnsFromExprList(pParse, pSelect->pEList, &pTab->nCol, &pTab->aCol); sqlite3SelectAddColumnTypeAndCollation(pParse, pTab, pSelect); @@ -4495,7 +4496,8 @@ withExpand(Walker * pWalker, struct SrcList_item = *pFrom) pTab->zName =3D sqlite3DbStrDup(db, pCte->zName); pTab->iPKey =3D -1; pTab->tuple_log_count =3D DEFAULT_TUPLE_LOG_COUNT; - assert(200 =3D=3D sqlite3LogEst(1048576)); + assert(sqlite3LogEst(DEFAULT_TUPLE_COUNT) =3D=3D + DEFAULT_TUPLE_LOG_COUNT); pTab->tabFlags |=3D TF_Ephemeral; pFrom->pSelect =3D sqlite3SelectDup(db, pCte->pSelect, = 0); if (db->mallocFailed) @@ -4688,6 +4690,8 @@ selectExpander(Walker * pWalker, Select * p) &pTab->nCol, = &pTab->aCol); pTab->iPKey =3D -1; pTab->tuple_log_count =3D = DEFAULT_TUPLE_LOG_COUNT; + assert(sqlite3LogEst(DEFAULT_TUPLE_COUNT) =3D=3D + DEFAULT_TUPLE_LOG_COUNT); pTab->tabFlags |=3D TF_Ephemeral; >=20 >> diff --git a/src/box/sql/where.c b/src/box/sql/where.c >> index 51b53c2df..e6f9ef431 100644 >> --- a/src/box/sql/where.c >> +++ b/src/box/sql/where.c >> @@ -1326,8 +1326,22 @@ whereRangeScanEst(Parse * pParse, /* = Parsing & code generating context */ >> } >> /* Determine iLower and iUpper using ($P) only. = */ >> if (nEq =3D=3D 0) { >> + /* >> + * In this simple case, there are no any >> + * equality constraints, so initially = all rows >> + * are in range. >> + */ >> iLower =3D 0; >> - iUpper =3D p->nRowEst0; >> + uint32_t space_id =3D >> + = SQLITE_PAGENO_TO_SPACEID(p->tnum); >> + struct space *space =3D = space_by_id(space_id); >> + assert(space !=3D NULL); >> + uint32_t iid =3D >> + = SQLITE_PAGENO_TO_INDEXID(p->tnum); >> + struct index *idx =3D >> + space_index(space, iid); >> + assert(idx !=3D NULL); >> + iUpper =3D idx->vtab->size(idx); >=20 > 3. Lets use index_count. I guess you mean index_size()? - iUpper =3D idx->vtab->size(idx); + iUpper =3D index_size(idx); > And looks like this case is repeated not the first and > even not the second time. Lets implement a function sql_table_size(), = that takes > a table pointer, and does PAGENO_TO... + space_by_id + space_index + = index_size. > I would put it in one of the previous patches, where this case appears = first. Well, I grepped and found index_size() only in two places across SQL = module: the first is here, and the second one in sql_index_tuple_size(), which = already takes space/index arguments.