[Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence
Oleg Babin
olegrok at tarantool.org
Mon Mar 16 22:05:58 MSK 2020
Hi! Thanks a lot for review!
On 15/03/2020 19:13, Vladislav Shpilevoy wrote:
> 1. I just realized, C function also should be documented,
> since you added it to the public C API.
I've add a section about C API changes to docbot request:
| C API:
|
| ```C
| int
| box_sequence_current(uint32_t seq_id, int64_t *result);
| ```
|
| Where:
| * seq_id - sequence identifier;
| * result - pointer to a variable where the current sequence
| value will be stored on success.
|
| Returns 0 on success and -1 otherwise. In case of an error user
| could get it via `box_error_last()`.
>> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
>> index aba439ffb..55efb9896 100644
>> --- a/src/box/lua/schema.lua
>> +++ b/src/box/lua/schema.lua
>> @@ -1812,6 +1816,15 @@ sequence_mt.next = function(self)
>> return internal.sequence.next(self.id)
>> end
>>
>> +local sequence_value = ffi.new('int64_t[1]')
>
> 2. Specifically for this there is buffer.reg1/reg2. A pre-allocated
> set of scalar values and pointers at them.
>
> ====================
> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
> index 55efb9896..dafa749a9 100644
> --- a/src/box/lua/schema.lua
> +++ b/src/box/lua/schema.lua
> @@ -7,7 +7,7 @@ local fun = require('fun')
> local log = require('log')
> local fio = require('fio')
> local json = require('json')
> -local static_alloc = require('buffer').static_alloc
> +local buffer = require('buffer')
> local session = box.session
> local internal = require('box.internal')
> local function setmap(table)
> @@ -1816,13 +1816,13 @@ sequence_mt.next = function(self)
> return internal.sequence.next(self.id)
> end
>
> -local sequence_value = ffi.new('int64_t[1]')
> sequence_mt.current = function(self)
> - local rc = builtin.box_sequence_current(self.id, sequence_value)
> + local ai64 = buffer.reg1.ai64
> + local rc = builtin.box_sequence_current(self.id, ai64)
> if rc < 0 then
> box.error(box.error.last())
> end
> - return sequence_value[0]
> + return ai64[0]
> end
>
> sequence_mt.set = function(self, value)
> @@ -2308,7 +2308,7 @@ box.schema.user = {}
>
> box.schema.user.password = function(password)
> local BUF_SIZE = 128
> - local buf = static_alloc('char', BUF_SIZE)
> + local buf = buffer.static_alloc('char', BUF_SIZE)
> builtin.password_prepare(password, #password, buf, BUF_SIZE)
> return ffi.string(buf)
> end
> diff --git a/src/lua/buffer.lua b/src/lua/buffer.lua
> index ef0118c7c..9aac82b39 100644
> --- a/src/lua/buffer.lua
> +++ b/src/lua/buffer.lua
> @@ -63,6 +63,7 @@ union c_register {
> uint32_t u32;
> uint64_t u64;
> int64_t i64;
> + int64_t ai64[1];
> };
> ]]
> ====================
I've apply your changes to my branch. Thanks!
>> +sequence_mt.current = function(self)
>> + local rc = builtin.box_sequence_current(self.id, sequence_value)
>> + if rc < 0 then
>> + box.error(box.error.last())
>> + end
>> + return sequence_value[0]
>> +end
>> +
>> sequence_mt.set = function(self, value)
>> return internal.sequence.set(self.id, value)
>> end
>> diff --git a/src/box/sequence.c b/src/box/sequence.c
>> index 5ebfa2747..4afbc2666 100644
>> --- a/src/box/sequence.c
>> +++ b/src/box/sequence.c
>> @@ -367,14 +367,18 @@ sequence_data_iterator_create(void)
>> return &iter->base;
>> }
>>
>> -int64_t
>> -sequence_get_value(struct sequence *seq)
>> +int
>> +sequence_get_value(struct sequence *seq, int64_t *result)
>> {
>> uint32_t key = seq->def->id;
>> uint32_t hash = sequence_hash(key);
>> uint32_t pos = light_sequence_find_key(&sequence_data_index, hash, key);
>> - assert(pos != light_sequence_end);
>> + if (pos == light_sequence_end) {
>> + diag_set(ClientError, ER_SEQUENCE_NOT_STARTED, seq->def->name);
>> + return -1;
>> + }
>> struct sequence_data data = light_sequence_get(&sequence_data_index,
>> pos);
>> - return data.value;
>> + *result = data.value;
>> + return 0;
>> }
>> diff --git a/src/box/sequence.h b/src/box/sequence.h
>> index a164da9af..8c442872a 100644
>> --- a/src/box/sequence.h
>> +++ b/src/box/sequence.h
>> @@ -171,10 +171,11 @@ sequence_data_iterator_create(void);
>> * Get last element of given sequence.
>> *
>> * @param seq sequence to get value from.
>> - * @retval last element of sequence.
>> + * @result value of sequence.
>
> 3. According to doxygen doc, @result is the same as @return. So
> this comment says "return value of sequence". I think you wanted
> to refer to 'result' parameter. For that doxygen provides command
> @param.
>
>> + * Return 0 on success, -1 if sequence is not initialized.
>
> 4. Doxygen way of documenting return values is either
>
> @retval <value1> Meaning.
> @retval <value2> Meaning.
>
> Or
>
> @return/returns Meaning.
>
>> */
I looked how it was done for "sequence_next" and have changed in the
same manner:
| /**
| * Get last element of given sequence.
| *
| * @param seq sequence to get value from.
| * On success, return 0 and assign the current value of the
| * sequence to @result, otherwise return -1 and set diag.
| */
>> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
>> index 912a10153..0de07ca1d 100644
>> --- a/src/box/sql/vdbe.c
>> +++ b/src/box/sql/vdbe.c
>> @@ -4328,7 +4328,10 @@ case OP_IdxInsert: {
>> p->nChange++;
>> if (pOp->p3 > 0 && ((aMem[pOp->p3].flags) & MEM_Null) != 0) {
>> assert(space->sequence != NULL);
>> - int64_t value = sequence_get_value(space->sequence);
>> + int64_t value;
>> + if (sequence_get_value(space->sequence, &value) != 0) {
>> + goto abort_due_to_error;
>
> 5. We usually omit {} for one-line if-body.
>
>> + }
>> if (vdbe_add_new_autoinc_id(p, value) != 0)
>> goto abort_due_to_error;
>> }
Fixed.
I've already applied this changes to my branch:
https://github.com/tarantool/tarantool/tree/olegrok/4752-sequence-current
Diff:
==============
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 55efb9896..57deeee89 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -7,7 +7,7 @@ local fun = require('fun')
local log = require('log')
local fio = require('fio')
local json = require('json')
-local static_alloc = require('buffer').static_alloc
+local buffer = require('buffer')
local session = box.session
local internal = require('box.internal')
local function setmap(table)
@@ -1816,13 +1816,13 @@ sequence_mt.next = function(self)
return internal.sequence.next(self.id)
end
-local sequence_value = ffi.new('int64_t[1]')
sequence_mt.current = function(self)
- local rc = builtin.box_sequence_current(self.id, sequence_value)
+ local ai64 = buffer.reg1.ai64
+ local rc = builtin.box_sequence_current(self.id, ai64)
if rc < 0 then
box.error(box.error.last())
end
- return sequence_value[0]
+ return ai64[0]
end
sequence_mt.set = function(self, value)
@@ -2308,9 +2308,9 @@ box.schema.user = {}
box.schema.user.password = function(password)
local BUF_SIZE = 128
- local buf = static_alloc('char', BUF_SIZE)
+ local buf = buffer.static_alloc('char', BUF_SIZE)
builtin.password_prepare(password, #password, buf, BUF_SIZE)
end
local function chpasswd(uid, new_password)
diff --git a/src/box/sequence.h b/src/box/sequence.h
index 8c442872a..400bdc6f9 100644
--- a/src/box/sequence.h
+++ b/src/box/sequence.h
@@ -171,8 +171,8 @@ sequence_data_iterator_create(void);
* Get last element of given sequence.
*
* @param seq sequence to get value from.
- * @result value of sequence.
- * Return 0 on success, -1 if sequence is not initialized.
+ * On success, return 0 and assign the current value of the
+ * sequence to @result, otherwise return -1 and set diag.
*/
int
sequence_get_value(struct sequence *seq, int64_t *result);
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 0de07ca1d..e8a029a8a 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4329,9 +4329,8 @@ case OP_IdxInsert: {
if (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) {
+ if (sequence_get_value(space->sequence, &value) != 0)
goto abort_due_to_error;
- }
if (vdbe_add_new_autoinc_id(p, value) != 0)
goto abort_due_to_error;
}
diff --git a/src/lua/buffer.lua b/src/lua/buffer.lua
index ef0118c7c..9aac82b39 100644
--- a/src/lua/buffer.lua
+++ b/src/lua/buffer.lua
@@ -63,6 +63,7 @@ union c_register {
uint32_t u32;
uint64_t u64;
int64_t i64;
+ int64_t ai64[1];
};
]]
More information about the Tarantool-patches
mailing list