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 27DFE25597 for ; Fri, 18 May 2018 06:48:51 -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 Lbqg3RxDAwNH for ; Fri, 18 May 2018 06:48:51 -0400 (EDT) Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [217.69.128.34]) (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 D83AD25582 for ; Fri, 18 May 2018 06:48:50 -0400 (EDT) Date: Fri, 18 May 2018 13:48:47 +0300 From: Kirill Yukhin Subject: [tarantool-patches] Re: [PATCH 1/2] sql: allow key_def to be NULL for ephemeral create Message-ID: <20180518104847.xni3sj7halyqozqi@tarantool.org> References: <54fef8ed5144ab8ee384687caa3448a89ac62746.1526483543.git.kyukhin@tarantool.org> <20180517154939.ya64lgo4ifipmofk@tarantool.org> <20180518065712.aal76hawn3ofyzln@tarantool.org> <6186d3a5-8cb6-6ce9-c1ee-d100e04fec96@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <6186d3a5-8cb6-6ce9-c1ee-d100e04fec96@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: Vladislav Shpilevoy Cc: tarantool-patches@freelists.org On 18 мая 13:33, Vladislav Shpilevoy wrote: > Hello. Thanks for fixes! > > > diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c > > index 3455f52..2c1ce44 100644 > > --- a/src/box/sql/delete.c > > +++ b/src/box/sql/delete.c > > @@ -386,14 +386,9 @@ sqlite3DeleteFrom(Parse * pParse, /* The parser context */ > > iPk = pParse->nMem + 1; > > pParse->nMem += nPk; > > iEphCur = pParse->nTab++; > > - struct key_def *def = key_def_new(nPk); > > - if (def == NULL) { > > - sqlite3OomFault(db); > > - goto delete_from_cleanup; > > - } > > addrEphOpen = > > sqlite3VdbeAddOp4(v, OP_OpenTEphemeral, iEphCur, > > - nPk, 0, (char*)def, P4_KEYDEF); > > + nPk); > > Compilation error. Whoops, fixed. -- Regards, Kirill Yukhin commit 8640d51f1fb60022b4b61b84fd5c6111c5dbe916 Author: Kirill Yukhin Date: Wed May 16 09:47:02 2018 +0300 sql: allow key_def to be NULL for ephemeral create There're many cases in SQL FE, where exact key def passed to ephemeral table doesn't matter. It is used to store data only. Only field count makes sense in such a case. However it is passed separately (in P2). So, allow P4 field to be NULL for OP_OpenTEphemeral. Update delete from routine from sql/delete.c accordingly. Part of #3235 diff --git a/src/box/sql.c b/src/box/sql.c index 195fdfb..6d2e0b9 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -370,8 +370,9 @@ int tarantoolSqlite3Count(BtCursor *pCur, i64 *pnEntry) * * @retval SQLITE_OK on success, SQLITE_TARANTOOL_ERROR otherwise. */ -int tarantoolSqlite3EphemeralCreate(BtCursor *pCur, uint32_t field_count, - struct key_def *def) +int +tarantoolSqlite3EphemeralCreate(BtCursor *pCur, uint32_t field_count, + struct key_def *def) { assert(pCur); assert(pCur->curFlags & BTCF_TEphemCursor); @@ -381,7 +382,7 @@ int tarantoolSqlite3EphemeralCreate(BtCursor *pCur, uint32_t field_count, return SQL_TARANTOOL_ERROR; for (uint32_t part = 0; part < field_count; ++part) { struct coll *coll; - if (part < def->part_count) + if (def != NULL && part < def->part_count) coll = def->parts[part].coll; else coll = NULL; diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c index 3455f52..1a2f3d0 100644 --- a/src/box/sql/delete.c +++ b/src/box/sql/delete.c @@ -386,14 +386,9 @@ sqlite3DeleteFrom(Parse * pParse, /* The parser context */ iPk = pParse->nMem + 1; pParse->nMem += nPk; iEphCur = pParse->nTab++; - struct key_def *def = key_def_new(nPk); - if (def == NULL) { - sqlite3OomFault(db); - goto delete_from_cleanup; - } addrEphOpen = - sqlite3VdbeAddOp4(v, OP_OpenTEphemeral, iEphCur, - nPk, 0, (char*)def, P4_KEYDEF); + sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, iEphCur, + nPk); } else { pPk = sqlite3PrimaryKeyIndex(pTab); assert(pPk != 0); diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 5ada87e..54a7e4a 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -566,13 +566,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */ regRec = sqlite3GetTempReg(pParse); regCopy = sqlite3GetTempRange(pParse, nColumn); regTempId = sqlite3GetTempReg(pParse); - struct key_def *def = key_def_new(nColumn + 1); - if (def == NULL) { - sqlite3OomFault(db); - goto insert_cleanup; - } - sqlite3VdbeAddOp4(v, OP_OpenTEphemeral, srcTab, nColumn+1, - 0, (char*)def, P4_KEYDEF); + sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, srcTab, nColumn+1); /* Create counter for rowid */ sqlite3VdbeAddOp4Dup8(v, OP_Int64, 0 /* unused */, diff --git a/src/box/sql/select.c b/src/box/sql/select.c index bd895bc..de111d3 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -2215,13 +2215,7 @@ generateWithRecursiveQuery(Parse * pParse, /* Parsing context */ VdbeComment((v, "Orderby table")); destQueue.pOrderBy = pOrderBy; } else { - struct key_def *def = key_def_new(nCol + 1); - if (def == NULL) { - sqlite3OomFault(pParse->db); - goto end_of_recursive_query; - } - sqlite3VdbeAddOp4(v, OP_OpenTEphemeral, iQueue, nCol + 1, 0, - (char*)def, P4_KEYDEF); + sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, iQueue, nCol + 1); VdbeComment((v, "Queue table")); } if (iDistinct) { @@ -2422,13 +2416,7 @@ multiSelect(Parse * pParse, /* Parsing context */ if (dest.eDest == SRT_EphemTab) { assert(p->pEList); int nCols = p->pEList->nExpr; - struct key_def *def = key_def_new(nCols + 1); - if (def == NULL) { - sqlite3OomFault(db); - goto multi_select_end; - } - sqlite3VdbeAddOp4(v, OP_OpenTEphemeral, dest.iSDParm, nCols + 1, - 0, (char*)def, P4_KEYDEF); + sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, dest.iSDParm, nCols + 1); VdbeComment((v, "Destination temp")); dest.eDest = SRT_Table; } @@ -5608,11 +5596,8 @@ sqlite3Select(Parse * pParse, /* The parser context */ /* If the output is destined for a temporary table, open that table. */ if (pDest->eDest == SRT_EphemTab) { - struct key_def *def = key_def_new(pEList->nExpr + 1); - if (def == NULL) - sqlite3OomFault(db); - sqlite3VdbeAddOp4(v, OP_OpenTEphemeral, pDest->iSDParm, - pEList->nExpr + 1, 0, (char*)def, P4_KEYDEF); + sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, pDest->iSDParm, + pEList->nExpr + 1); VdbeComment((v, "Output table")); } diff --git a/src/box/sql/update.c b/src/box/sql/update.c index f3db92c..204adc2 100644 --- a/src/box/sql/update.c +++ b/src/box/sql/update.c @@ -358,13 +358,8 @@ sqlite3Update(Parse * pParse, /* The parser context */ sqlite3VdbeAddOp2(v, OP_Null, 0, iPk); if (isView) { - struct key_def *def = key_def_new(nKey); - if (def == NULL) { - sqlite3OomFault(db); - goto update_cleanup; - } - addrOpen = sqlite3VdbeAddOp4(v, OP_OpenTEphemeral, iEph, - nKey, 0, (char*)def, P4_KEYDEF); + addrOpen = sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, iEph, + nKey); } else { addrOpen = sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, iEph, nPk); sql_vdbe_set_p4_key_def(pParse, pPk); diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index e27fe2f..455198d 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -3225,9 +3225,9 @@ open_cursor_set_hints: /** * Opcode: OpenTEphemeral P1 P2 * P4 * * Synopsis: - * @param P1 index of new cursor to be created - * @param P2 number of columns in a new table - * @param P4 key def for new table + * @param P1 index of new cursor to be created. + * @param P2 number of columns in a new table. + * @param P4 key def for new table, NULL is allowed. * * This opcode creates Tarantool's ephemeral table and sets cursor P1 to it. */ @@ -3236,21 +3236,20 @@ case OP_OpenTEphemeral: { BtCursor *pBtCur; assert(pOp->p1 >= 0); assert(pOp->p2 > 0); - assert(pOp->p4.key_def != NULL); - assert(pOp->p4type == P4_KEYDEF); + assert(pOp->p4type != P4_KEYDEF || pOp->p4.key_def != NULL); pCx = allocateCursor(p, pOp->p1, pOp->p2, CURTYPE_TARANTOOL); if (pCx == 0) goto no_mem; pCx->nullRow = 1; - pCx->key_def = pOp->p4.key_def; pBtCur = pCx->uc.pCursor; /* Ephemeral spaces don't have space_id */ pBtCur->eState = CURSOR_INVALID; pBtCur->curFlags = BTCF_TEphemCursor; rc = tarantoolSqlite3EphemeralCreate(pCx->uc.pCursor, pOp->p2, - pCx->key_def); + pOp->p4.key_def); + pCx->key_def = pCx->uc.pCursor->index->def->key_def; if (rc) goto abort_due_to_error; break; }