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 284D924623 for ; Mon, 22 Jul 2019 07:27:01 -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 7Zzhbe-y8UZU for ; Mon, 22 Jul 2019 07:27:01 -0400 (EDT) Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (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 DBB852460A for ; Mon, 22 Jul 2019 07:26:50 -0400 (EDT) Date: Mon, 22 Jul 2019 14:26:47 +0300 From: Mergen Imeev Subject: [tarantool-patches] Re: [PATCH v4 4/4] sql: do not increase row-count if INSERT or REPLACE failed Message-ID: <20190722112646.GA29744@tarantool.org> References: <1C2735EE-5E20-43D6-B8BA-3B83C7B54482@tarantool.org> <20190719093650.GB22697@tarantool.org> <6AE2D6A3-B992-4BBC-9BE7-6A064F316E66@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <6AE2D6A3-B992-4BBC-9BE7-6A064F316E66@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: "n.pettik" Cc: tarantool-patches@freelists.org Hi! Thank you for review! Just before sending patch to Kirill, I discovered that the function sequence_get_value() returns only 0. Because of this, it makes no sense to check the result of the function. Since this function used a special argument to return a value, I deleted this argument and made the function now return this value. Diff below. On Mon, Jul 22, 2019 at 01:48:44PM +0300, n.pettik wrote: > I’ve force pushed a couple of cosmetic fixes. > Also added check of return value of > vdbe_add_new_autoinc_id() - it can fail due to OOM. > > Now LGTM. i> diff --git a/src/box/sequence.c b/src/box/sequence.c index 5996519..1aacc50 100644 --- a/src/box/sequence.c +++ b/src/box/sequence.c @@ -363,8 +363,8 @@ sequence_data_iterator_create(void) return &iter->base; } -int -sequence_get_value(struct sequence *seq, int64_t *out_value) +int64_t +sequence_get_value(struct sequence *seq) { uint32_t key = seq->def->id; uint32_t hash = sequence_hash(key); @@ -372,6 +372,5 @@ sequence_get_value(struct sequence *seq, int64_t *out_value) assert(pos != light_sequence_end); struct sequence_data data = light_sequence_get(&sequence_data_index, pos); - *out_value = data.value; - return 0; + return data.value; } diff --git a/src/box/sequence.h b/src/box/sequence.h index b56236e..976020a 100644 --- a/src/box/sequence.h +++ b/src/box/sequence.h @@ -171,11 +171,10 @@ sequence_data_iterator_create(void); * Get last element of given sequence. * * @param seq sequence to get value from. - * @param[out] out_value last element of sequence. - * @retval 0 on success, -1 on error. + * @retval last element of sequence. */ -int -sequence_get_value(struct sequence *seq, int64_t *out_value); +int64_t +sequence_get_value(struct sequence *seq); #if defined(__cplusplus) } /* extern "C" */ diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index f771843..431b4d9 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -4224,9 +4224,7 @@ case OP_IdxInsert: { } if (rc == 0 && pOp->p3 > 0 && ((aMem[pOp->p3].flags) & MEM_Null) != 0) { assert(space->sequence != NULL); - int64_t value; - if (sequence_get_value(space->sequence, &value) != 0) - goto abort_due_to_error; + int64_t value = sequence_get_value(space->sequence); if (vdbe_add_new_autoinc_id(p, value) != 0) goto abort_due_to_error; }