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 CA2182BE28 for ; Mon, 29 Oct 2018 08:25:59 -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 oLUxr1q8PuQs for ; Mon, 29 Oct 2018 08:25:59 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 819AF2BA10 for ; Mon, 29 Oct 2018 08:25:59 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: [tarantool-patches] Re: [PATCH] sql: remove nKey from struct BtCursor From: "n.pettik" In-Reply-To: <64674871-e5f0-0cb2-540c-d95cfce47409@tarantool.org> Date: Mon, 29 Oct 2018 15:25:56 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <1AF05058-8C8F-4E46-A541-AEA551675E80@tarantool.org> References: <20181026145926.15345-1-korablev@tarantool.org> <64674871-e5f0-0cb2-540c-d95cfce47409@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 > On 29 Oct 2018, at 13:27, Vladislav Shpilevoy = wrote: >=20 > Hi! Thanks for the patch! >=20 > Consider my review fixes and the end of the email. >=20 > Why are you sure that nKey is always 0? As I see > in the code, it is initialized in > tarantoolSqlite3MovetoUnpacked as > sqlite3VdbeMsgpackRecordLen. > How does COUNT work, when you specify WHERE? For example, > SELECT COUNT(*) WHERE key_column =3D ;. Looks like > it can be effectively forwarded to index_count(). OP_Count (ergo tarantoolSqlite3Count() function) is used only for = queries without WHERE clause - grep is_simple_count() function. Otherwise, we rely on =E2=80=9Cgeneric" opcodes to perform any aggregate functions - = OP_AggStep0, OP_AggStep and OP_AggFinal. In this case index is still used (however, may be not so effectively). Lets look at simple example: CREATE TABLE t1 (id INT PRIMARY KEY, a INT); CREATE INDEX i1 ON t1 (a); INSERT INTO t1 VALUES (1,1), (2, -6), (3, 100), (6, -6); PRAGMA vdbe_debug =3D 1; SELECT COUNT(*) FROM t1 WHERE a =3D -6; VDBE Trace: 101> 0 Init 0 1 0 00 Start at 1 SQL-trace: select count(*) from t1 where a =3D -6 101> 1 Null 0 1 1 00 r[1..1]=3DNULL REG[1] =3D NULL 101> 2 IteratorOpen 1 1 0 space 02 index id =3D = 1, space ptr =3D space or reg[0]; I1 101> 3 Explain 0 0 0 SEARCH TABLE T1 USING COVERING = INDEX I1 (A=3D?) 00=20 101> 4 Integer -6 2 0 00 r[2]=3D-6 REG[2] =3D i:-6 101> 5 SeekGE 1 9 2 1 00 key=3Dr[2] REG[2] =3D i:-6 101> 7 AggStep0 0 0 1 count(0) 00 accum=3Dr[1] = step(r[0]) 101> 8 Next 1 6 1 00=20 101> 6 IdxGT 1 9 2 1 00 key=3Dr[2] 101> 7 AggStep 0 0 1 count(0) 00 accum=3Dr[1] = step(r[0]) 101> 8 Next 1 6 1 00=20 101> 9 AggFinal 1 0 0 count(0) 00 accum=3Dr[1] = N=3D0 101> 10 Copy 1 3 0 00 r[3]=3Dr[1] REG[3] =3D i:2 101> 11 ResultRow 3 1 0 00 output=3Dr[3] REG[3] =3D i:2 101> 12 Halt 0 0 0 00=20 Here we open an iterator on index i1, seek for value -6 and iterate (alongside with counting) until there are elements with given value. I guess, if we utilised index_count in form of: index_count(idx, EQ, -6, nKey); then it would be more efficient. On the other hand, such optimization is quite specific and available for queries with one filter condition = under WHERE clause. Still it =E2=80=9Chas right to exist=E2=80=9D. *I must have put this explanation to the commit message, sorry for that. I=E2=80=99ve extended commit message a bit with more explanation = comments. * If you don=E2=80=99t mind, I will open ticket concerning this = optimisation. But at the current moment nKey doesn=E2=80=99t really make any sense. > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D >=20 > diff --git a/src/box/sql.c b/src/box/sql.c > index 9d58b8fcf..9aca618ce 100644 > --- a/src/box/sql.c > +++ b/src/box/sql.c > @@ -340,7 +340,7 @@ int tarantoolSqlite3EphemeralCount(struct BtCursor = *pCur, i64 *pnEntry) > assert(pCur->curFlags & BTCF_TEphemCursor); > struct index *primary_index =3D space_index(pCur->space, 0 /* PK = */); > - *pnEntry =3D index_count(primary_index, pCur->iter_type, 0, 0); > + *pnEntry =3D index_count(primary_index, pCur->iter_type, NULL, = 0); > return SQLITE_OK; > } > @@ -348,7 +348,7 @@ int tarantoolSqlite3Count(BtCursor *pCur, i64 = *pnEntry) > { > assert(pCur->curFlags & BTCF_TaCursor); > - *pnEntry =3D index_count(pCur->index, pCur->iter_type, 0, 0); > + *pnEntry =3D index_count(pCur->index, pCur->iter_type, NULL, 0); > return SQLITE_OK; > } Ok, applied.