Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence
@ 2020-03-13 14:06 olegrok
  2020-03-15 16:13 ` Vladislav Shpilevoy
  0 siblings, 1 reply; 10+ messages in thread
From: olegrok @ 2020-03-13 14:06 UTC (permalink / raw)
  To: tarantool-patches, v.shpilevoy, korablev; +Cc: Oleg Babin

From: Oleg Babin <babinoleg@mail.ru>

This patch introduces "current" function for sequences.
It returns the last retrieved value of specified sequence or
throws an error if no value has been generated yet.

This patch partially reverts 3ff1f1e36e14381c0ebb5862943d4da281254767
(box: remove sequence_get) here similar function "get" was removed
to avoid possible misleading with "currval" function of PosgreSQL
that returns the last obtained value of the sequence in the scope
of current session. In contrast "current" returns the last globally
retrieved value of the sequence.

Closes #4752

@TarantoolBot document
Title: sequence:current()

This patch introduces "current" function for sequences.
It returns the last retrieved value of specified sequence or
throws an error if no value has been generated yet ("next"
has not been called yet or right after "reset" is called).

Example:

```lua
sq = box.schema.sequence.create('test')
---
...
sq:current()
---
- error: Sequence 'test' is not started
...
sq:next()
---
- 1
...
sq:current()
---
- 1
...
sq:set(42)
---
...
sq:current()
---
- 42
...
sq:reset()
---
...
sq:current()  -- error
---
- error: Sequence 'test' is not started
...
```
---
Issue: https://github.com/tarantool/tarantool/issues/4752
Branch: https://github.com/tarantool/tarantool/tree/olegrok/4752-sequence-current

@ChangeLog
- sequence:current() - a function to get sequence's current
  value, without changing it (gh-4752).

v1: https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/014683.html
Changes:
  - Use ClientError instead of SequenceError
v2: https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/014699.html
Changes:
  - Clarify description
  - Use FFI instead of Lua C API for box_sequence_current call

 extra/exports              |  1 +
 src/box/box.cc             | 14 ++++++++
 src/box/box.h              | 12 +++++++
 src/box/errcode.h          |  1 +
 src/box/lua/schema.lua     | 13 +++++++
 src/box/sequence.c         | 12 ++++---
 src/box/sequence.h         |  7 ++--
 src/box/sql/vdbe.c         |  5 ++-
 test/box/misc.result       |  1 +
 test/box/sequence.result   | 73 ++++++++++++++++++++++++++++++++++++++
 test/box/sequence.test.lua | 24 +++++++++++++
 11 files changed, 155 insertions(+), 8 deletions(-)

diff --git a/extra/exports b/extra/exports
index 3a0637317..cbb5adcf4 100644
--- a/extra/exports
+++ b/extra/exports
@@ -215,6 +215,7 @@ box_update
 box_upsert
 box_truncate
 box_sequence_next
+box_sequence_current
 box_sequence_set
 box_sequence_reset
 box_index_iterator
diff --git a/src/box/box.cc b/src/box/box.cc
index 09dd67ab4..a5052dba4 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1414,6 +1414,20 @@ box_sequence_next(uint32_t seq_id, int64_t *result)
 	*result = value;
 	return 0;
 }
+
+int
+box_sequence_current(uint32_t seq_id, int64_t *result)
+{
+	struct sequence *seq = sequence_cache_find(seq_id);
+	if (seq == NULL)
+		return -1;
+	if (access_check_sequence(seq) != 0)
+		return -1;
+	if (sequence_get_value(seq, result) != 0)
+		return -1;
+	return 0;
+}
+
 int
 box_sequence_set(uint32_t seq_id, int64_t value)
 {
diff --git a/src/box/box.h b/src/box/box.h
index f37a945eb..044d929d4 100644
--- a/src/box/box.h
+++ b/src/box/box.h
@@ -423,6 +423,18 @@ box_truncate(uint32_t space_id);
 API_EXPORT int
 box_sequence_next(uint32_t seq_id, int64_t *result);
 
+/**
+ * Get the last value returned by a sequence.
+ *
+ * \param seq_id sequence identifier
+ * \param[out] result pointer to a variable where the current sequence
+ * value will be stored on success
+ * \retval -1 on error (check box_error_last())
+ * \retval 0 on success
+ */
+API_EXPORT int
+box_sequence_current(uint32_t seq_id, int64_t *result);
+
 /**
  * Set a sequence value.
  *
diff --git a/src/box/errcode.h b/src/box/errcode.h
index d7ec97e8c..444171778 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -264,6 +264,7 @@ struct errcode_record {
 	/*209 */_(ER_SESSION_SETTING_INVALID_VALUE,	"Session setting %s expected a value of type %s") \
 	/*210 */_(ER_SQL_PREPARE,		"Failed to prepare SQL statement: %s") \
 	/*211 */_(ER_WRONG_QUERY_ID,		"Prepared statement with id %u does not exist") \
+	/*212 */_(ER_SEQUENCE_NOT_STARTED,		"Sequence '%s' is not started") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
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
@@ -72,6 +72,10 @@ ffi.cdef[[
     int
     box_txn_begin();
     /** \endcond public */
+    /** \cond public */
+    int
+    box_sequence_current(uint32_t seq_id, int64_t *result);
+    /** \endcond public */
     typedef struct txn_savepoint box_txn_savepoint_t;
 
     box_txn_savepoint_t *
@@ -1812,6 +1816,15 @@ 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)
+    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.
+ * Return 0 on success, -1 if sequence is not initialized.
  */
-int64_t
-sequence_get_value(struct sequence *seq);
+int
+sequence_get_value(struct sequence *seq, int64_t *result);
 
 #if defined(__cplusplus)
 } /* extern "C" */
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;
+		}
 		if (vdbe_add_new_autoinc_id(p, value) != 0)
 			goto abort_due_to_error;
 	}
diff --git a/test/box/misc.result b/test/box/misc.result
index 5ac5e0f26..047591b60 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -636,6 +636,7 @@ t;
   209: box.error.SESSION_SETTING_INVALID_VALUE
   210: box.error.SQL_PREPARE
   211: box.error.WRONG_QUERY_ID
+  212: box.error.SEQUENCE_NOT_STARTED
 ...
 test_run:cmd("setopt delimiter ''");
 ---
diff --git a/test/box/sequence.result b/test/box/sequence.result
index 0a6cfee2c..e1ac30fd1 100644
--- a/test/box/sequence.result
+++ b/test/box/sequence.result
@@ -188,10 +188,18 @@ sq.step, sq.min, sq.max, sq.start, sq.cycle
 - 1
 - false
 ...
+sq:current()  -- error
+---
+- error: Sequence 'test' is not started
+...
 sq:next() -- 1
 ---
 - 1
 ...
+sq:current()  -- 1
+---
+- 1
+...
 sq:next() -- 2
 ---
 - 2
@@ -199,6 +207,10 @@ sq:next() -- 2
 sq:set(100)
 ---
 ...
+sq:current()  -- 100
+---
+- 100
+...
 sq:next() -- 101
 ---
 - 101
@@ -210,6 +222,10 @@ sq:next() -- 102
 sq:reset()
 ---
 ...
+sq:current()  -- error
+---
+- error: Sequence 'test' is not started
+...
 sq:next() -- 1
 ---
 - 1
@@ -233,10 +249,18 @@ sq.step, sq.min, sq.max, sq.start, sq.cycle
 - -1
 - false
 ...
+sq:current()  -- error
+---
+- error: Sequence 'test' is not started
+...
 sq:next() -- -1
 ---
 - -1
 ...
+sq:current()  -- -1
+---
+- -1
+...
 sq:next() -- -2
 ---
 - -2
@@ -244,6 +268,10 @@ sq:next() -- -2
 sq:set(-100)
 ---
 ...
+sq:current()  -- -100
+---
+- -100
+...
 sq:next() -- -101
 ---
 - -101
@@ -255,6 +283,10 @@ sq:next() -- -102
 sq:reset()
 ---
 ...
+sq:current()  -- error
+---
+- error: Sequence 'test' is not started
+...
 sq:next() -- -1
 ---
 - -1
@@ -2289,3 +2321,44 @@ box.space._space_sequence:update({s.id}, {{'=', 2, t[2]}})
 s:drop()
 ---
 ...
+--
+-- gh-4752: introduce sequence:current() method which
+-- fetches current sequence value but doesn't modify
+-- sequence itself.
+--
+sq = box.schema.sequence.create('test')
+---
+...
+sq:current()
+---
+- error: Sequence 'test' is not started
+...
+sq:next()
+---
+- 1
+...
+sq:current()
+---
+- 1
+...
+sq:set(42)
+---
+...
+sq:current()
+---
+- 42
+...
+sq:current()
+---
+- 42
+...
+sq:reset()
+---
+...
+sq:current()
+---
+- error: Sequence 'test' is not started
+...
+sq:drop()
+---
+...
diff --git a/test/box/sequence.test.lua b/test/box/sequence.test.lua
index 8e00571e5..6b8e847b6 100644
--- a/test/box/sequence.test.lua
+++ b/test/box/sequence.test.lua
@@ -58,12 +58,16 @@ box.sequence.test == nil
 -- Default ascending sequence.
 sq = box.schema.sequence.create('test')
 sq.step, sq.min, sq.max, sq.start, sq.cycle
+sq:current()  -- error
 sq:next() -- 1
+sq:current()  -- 1
 sq:next() -- 2
 sq:set(100)
+sq:current()  -- 100
 sq:next() -- 101
 sq:next() -- 102
 sq:reset()
+sq:current()  -- error
 sq:next() -- 1
 sq:next() -- 2
 sq:drop()
@@ -71,12 +75,16 @@ sq:drop()
 -- Default descending sequence.
 sq = box.schema.sequence.create('test', {step = -1})
 sq.step, sq.min, sq.max, sq.start, sq.cycle
+sq:current()  -- error
 sq:next() -- -1
+sq:current()  -- -1
 sq:next() -- -2
 sq:set(-100)
+sq:current()  -- -100
 sq:next() -- -101
 sq:next() -- -102
 sq:reset()
+sq:current()  -- error
 sq:next() -- -1
 sq:next() -- -2
 sq:drop()
@@ -780,3 +788,19 @@ pk = s:create_index('pk', {sequence = true})
 t = box.space._space_sequence:get({s.id})
 box.space._space_sequence:update({s.id}, {{'=', 2, t[2]}})
 s:drop()
+
+--
+-- gh-4752: introduce sequence:current() method which
+-- fetches current sequence value but doesn't modify
+-- sequence itself.
+--
+sq = box.schema.sequence.create('test')
+sq:current()
+sq:next()
+sq:current()
+sq:set(42)
+sq:current()
+sq:current()
+sq:reset()
+sq:current()
+sq:drop()
-- 
2.23.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence
  2020-03-13 14:06 [Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence olegrok
@ 2020-03-15 16:13 ` Vladislav Shpilevoy
  2020-03-16 19:05   ` Oleg Babin
  0 siblings, 1 reply; 10+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-15 16:13 UTC (permalink / raw)
  To: olegrok, tarantool-patches, korablev; +Cc: Oleg Babin

Hi! Thanks for the patch!

See 5 comments below.

On 13/03/2020 15:06, olegrok@tarantool.org wrote:
> From: Oleg Babin <babinoleg@mail.ru>
> 
> This patch introduces "current" function for sequences.
> It returns the last retrieved value of specified sequence or
> throws an error if no value has been generated yet.
> 
> This patch partially reverts 3ff1f1e36e14381c0ebb5862943d4da281254767
> (box: remove sequence_get) here similar function "get" was removed
> to avoid possible misleading with "currval" function of PosgreSQL
> that returns the last obtained value of the sequence in the scope
> of current session. In contrast "current" returns the last globally
> retrieved value of the sequence.
> 
> Closes #4752
> 
> @TarantoolBot document
> Title: sequence:current()
> 
> This patch introduces "current" function for sequences.
> It returns the last retrieved value of specified sequence or
> throws an error if no value has been generated yet ("next"
> has not been called yet or right after "reset" is called).

1. I just realized, C function also should be documented,
since you added it to the public C API.

> 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];
 };
 ]]
==================== 

> +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.

>   */
> -int64_t
> -sequence_get_value(struct sequence *seq);
> +int
> +sequence_get_value(struct sequence *seq, int64_t *result);
>  
>  #if defined(__cplusplus)
>  } /* extern "C" */
> 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;
>  	}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence
  2020-03-15 16:13 ` Vladislav Shpilevoy
@ 2020-03-16 19:05   ` Oleg Babin
  2020-03-16 23:04     ` Vladislav Shpilevoy
  0 siblings, 1 reply; 10+ messages in thread
From: Oleg Babin @ 2020-03-16 19:05 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches, korablev; +Cc: Oleg Babin

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];
  };
  ]]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence
  2020-03-16 19:05   ` Oleg Babin
@ 2020-03-16 23:04     ` Vladislav Shpilevoy
  2020-03-17 14:19       ` Nikita Pettik
  2020-03-17 14:31       ` olegrok
  0 siblings, 2 replies; 10+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-16 23:04 UTC (permalink / raw)
  To: Oleg Babin, tarantool-patches, korablev; +Cc: Oleg Babin

>>> 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.
> | */

The problem is that sequence_next() comment also is not a
valid doxygen syntax. So copy-paste here is not a solution.

But ok, I guess it is time to give up on trying to make the
comments correct everywhere.

LGTM.

Nikita, please, do a second review.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence
  2020-03-16 23:04     ` Vladislav Shpilevoy
@ 2020-03-17 14:19       ` Nikita Pettik
  2020-03-17 19:16         ` Oleg Babin
  2020-03-17 14:31       ` olegrok
  1 sibling, 1 reply; 10+ messages in thread
From: Nikita Pettik @ 2020-03-17 14:19 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: Oleg Babin, tarantool-patches

On 17 Mar 00:04, Vladislav Shpilevoy wrote:
> >>> 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.
> > | */
> 
> The problem is that sequence_next() comment also is not a
> valid doxygen syntax. So copy-paste here is not a solution.
> 
> But ok, I guess it is time to give up on trying to make the
> comments correct everywhere.
> 
> LGTM.
> 
> Nikita, please, do a second review.

Okay, np. Oleg, could you please re-send patch with all updates?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence
  2020-03-16 23:04     ` Vladislav Shpilevoy
  2020-03-17 14:19       ` Nikita Pettik
@ 2020-03-17 14:31       ` olegrok
  2020-03-17 20:40         ` Nikita Pettik
  1 sibling, 1 reply; 10+ messages in thread
From: olegrok @ 2020-03-17 14:31 UTC (permalink / raw)
  To: korablev; +Cc: Oleg Babin, tarantool-patches

From: Oleg Babin <babinoleg@mail.ru>

This patch introduces "current" function for sequences.
It returns the last retrieved value of specified sequence or
throws an error if no value has been generated yet.

This patch partially reverts 3ff1f1e36e14381c0ebb5862943d4da281254767
(box: remove sequence_get) here similar function "get" was removed
to avoid possible misleading with "currval" function of PosgreSQL
that returns the last obtained value of the sequence in the scope
of current session. In contrast "current" returns the last globally
retrieved value of the sequence.

Closes #4752

@TarantoolBot document
Title: sequence:current()

This patch introduces "current" function for sequences.
It returns the last retrieved value of specified sequence or
throws an error if no value has been generated yet ("next"
has not been called yet or right after "reset" is called).

Lua:

Example:

```lua
sq = box.schema.sequence.create('test')
---
...
sq:current()
---
- error: Sequence 'test' is not started
...
sq:next()
---
- 1
...
sq:current()
---
- 1
...
sq:set(42)
---
...
sq:current()
---
- 42
...
sq:reset()
---
...
sq:current()  -- error
---
- error: Sequence 'test' is not started
...
```

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()`.
---
 extra/exports              |  1 +
 src/box/box.cc             | 14 ++++++++
 src/box/box.h              | 12 +++++++
 src/box/errcode.h          |  1 +
 src/box/lua/schema.lua     | 17 +++++++--
 src/box/sequence.c         | 12 ++++---
 src/box/sequence.h         |  7 ++--
 src/box/sql/vdbe.c         |  4 ++-
 src/lua/buffer.lua         |  1 +
 test/box/misc.result       |  1 +
 test/box/sequence.result   | 73 ++++++++++++++++++++++++++++++++++++++
 test/box/sequence.test.lua | 24 +++++++++++++
 12 files changed, 157 insertions(+), 10 deletions(-)

diff --git a/extra/exports b/extra/exports
index 3a0637317..cbb5adcf4 100644
--- a/extra/exports
+++ b/extra/exports
@@ -215,6 +215,7 @@ box_update
 box_upsert
 box_truncate
 box_sequence_next
+box_sequence_current
 box_sequence_set
 box_sequence_reset
 box_index_iterator
diff --git a/src/box/box.cc b/src/box/box.cc
index 09dd67ab4..a5052dba4 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1414,6 +1414,20 @@ box_sequence_next(uint32_t seq_id, int64_t *result)
 	*result = value;
 	return 0;
 }
+
+int
+box_sequence_current(uint32_t seq_id, int64_t *result)
+{
+	struct sequence *seq = sequence_cache_find(seq_id);
+	if (seq == NULL)
+		return -1;
+	if (access_check_sequence(seq) != 0)
+		return -1;
+	if (sequence_get_value(seq, result) != 0)
+		return -1;
+	return 0;
+}
+
 int
 box_sequence_set(uint32_t seq_id, int64_t value)
 {
diff --git a/src/box/box.h b/src/box/box.h
index f37a945eb..044d929d4 100644
--- a/src/box/box.h
+++ b/src/box/box.h
@@ -423,6 +423,18 @@ box_truncate(uint32_t space_id);
 API_EXPORT int
 box_sequence_next(uint32_t seq_id, int64_t *result);
 
+/**
+ * Get the last value returned by a sequence.
+ *
+ * \param seq_id sequence identifier
+ * \param[out] result pointer to a variable where the current sequence
+ * value will be stored on success
+ * \retval -1 on error (check box_error_last())
+ * \retval 0 on success
+ */
+API_EXPORT int
+box_sequence_current(uint32_t seq_id, int64_t *result);
+
 /**
  * Set a sequence value.
  *
diff --git a/src/box/errcode.h b/src/box/errcode.h
index d7ec97e8c..444171778 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -264,6 +264,7 @@ struct errcode_record {
 	/*209 */_(ER_SESSION_SETTING_INVALID_VALUE,	"Session setting %s expected a value of type %s") \
 	/*210 */_(ER_SQL_PREPARE,		"Failed to prepare SQL statement: %s") \
 	/*211 */_(ER_WRONG_QUERY_ID,		"Prepared statement with id %u does not exist") \
+	/*212 */_(ER_SEQUENCE_NOT_STARTED,		"Sequence '%s' is not started") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index aba439ffb..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)
@@ -72,6 +72,10 @@ ffi.cdef[[
     int
     box_txn_begin();
     /** \endcond public */
+    /** \cond public */
+    int
+    box_sequence_current(uint32_t seq_id, int64_t *result);
+    /** \endcond public */
     typedef struct txn_savepoint box_txn_savepoint_t;
 
     box_txn_savepoint_t *
@@ -1812,6 +1816,15 @@ sequence_mt.next = function(self)
     return internal.sequence.next(self.id)
 end
 
+sequence_mt.current = function(self)
+    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 ai64[0]
+end
+
 sequence_mt.set = function(self, value)
     return internal.sequence.set(self.id, value)
 end
@@ -2295,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/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..400bdc6f9 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.
+ * On success, return 0 and assign the current value of the
+ * sequence to @result, otherwise return -1 and set diag.
  */
-int64_t
-sequence_get_value(struct sequence *seq);
+int
+sequence_get_value(struct sequence *seq, int64_t *result);
 
 #if defined(__cplusplus)
 } /* extern "C" */
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 912a10153..e8a029a8a 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4328,7 +4328,9 @@ 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;
 		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];
 };
 ]]
 
diff --git a/test/box/misc.result b/test/box/misc.result
index 5ac5e0f26..047591b60 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -636,6 +636,7 @@ t;
   209: box.error.SESSION_SETTING_INVALID_VALUE
   210: box.error.SQL_PREPARE
   211: box.error.WRONG_QUERY_ID
+  212: box.error.SEQUENCE_NOT_STARTED
 ...
 test_run:cmd("setopt delimiter ''");
 ---
diff --git a/test/box/sequence.result b/test/box/sequence.result
index 0a6cfee2c..e1ac30fd1 100644
--- a/test/box/sequence.result
+++ b/test/box/sequence.result
@@ -188,10 +188,18 @@ sq.step, sq.min, sq.max, sq.start, sq.cycle
 - 1
 - false
 ...
+sq:current()  -- error
+---
+- error: Sequence 'test' is not started
+...
 sq:next() -- 1
 ---
 - 1
 ...
+sq:current()  -- 1
+---
+- 1
+...
 sq:next() -- 2
 ---
 - 2
@@ -199,6 +207,10 @@ sq:next() -- 2
 sq:set(100)
 ---
 ...
+sq:current()  -- 100
+---
+- 100
+...
 sq:next() -- 101
 ---
 - 101
@@ -210,6 +222,10 @@ sq:next() -- 102
 sq:reset()
 ---
 ...
+sq:current()  -- error
+---
+- error: Sequence 'test' is not started
+...
 sq:next() -- 1
 ---
 - 1
@@ -233,10 +249,18 @@ sq.step, sq.min, sq.max, sq.start, sq.cycle
 - -1
 - false
 ...
+sq:current()  -- error
+---
+- error: Sequence 'test' is not started
+...
 sq:next() -- -1
 ---
 - -1
 ...
+sq:current()  -- -1
+---
+- -1
+...
 sq:next() -- -2
 ---
 - -2
@@ -244,6 +268,10 @@ sq:next() -- -2
 sq:set(-100)
 ---
 ...
+sq:current()  -- -100
+---
+- -100
+...
 sq:next() -- -101
 ---
 - -101
@@ -255,6 +283,10 @@ sq:next() -- -102
 sq:reset()
 ---
 ...
+sq:current()  -- error
+---
+- error: Sequence 'test' is not started
+...
 sq:next() -- -1
 ---
 - -1
@@ -2289,3 +2321,44 @@ box.space._space_sequence:update({s.id}, {{'=', 2, t[2]}})
 s:drop()
 ---
 ...
+--
+-- gh-4752: introduce sequence:current() method which
+-- fetches current sequence value but doesn't modify
+-- sequence itself.
+--
+sq = box.schema.sequence.create('test')
+---
+...
+sq:current()
+---
+- error: Sequence 'test' is not started
+...
+sq:next()
+---
+- 1
+...
+sq:current()
+---
+- 1
+...
+sq:set(42)
+---
+...
+sq:current()
+---
+- 42
+...
+sq:current()
+---
+- 42
+...
+sq:reset()
+---
+...
+sq:current()
+---
+- error: Sequence 'test' is not started
+...
+sq:drop()
+---
+...
diff --git a/test/box/sequence.test.lua b/test/box/sequence.test.lua
index 8e00571e5..6b8e847b6 100644
--- a/test/box/sequence.test.lua
+++ b/test/box/sequence.test.lua
@@ -58,12 +58,16 @@ box.sequence.test == nil
 -- Default ascending sequence.
 sq = box.schema.sequence.create('test')
 sq.step, sq.min, sq.max, sq.start, sq.cycle
+sq:current()  -- error
 sq:next() -- 1
+sq:current()  -- 1
 sq:next() -- 2
 sq:set(100)
+sq:current()  -- 100
 sq:next() -- 101
 sq:next() -- 102
 sq:reset()
+sq:current()  -- error
 sq:next() -- 1
 sq:next() -- 2
 sq:drop()
@@ -71,12 +75,16 @@ sq:drop()
 -- Default descending sequence.
 sq = box.schema.sequence.create('test', {step = -1})
 sq.step, sq.min, sq.max, sq.start, sq.cycle
+sq:current()  -- error
 sq:next() -- -1
+sq:current()  -- -1
 sq:next() -- -2
 sq:set(-100)
+sq:current()  -- -100
 sq:next() -- -101
 sq:next() -- -102
 sq:reset()
+sq:current()  -- error
 sq:next() -- -1
 sq:next() -- -2
 sq:drop()
@@ -780,3 +788,19 @@ pk = s:create_index('pk', {sequence = true})
 t = box.space._space_sequence:get({s.id})
 box.space._space_sequence:update({s.id}, {{'=', 2, t[2]}})
 s:drop()
+
+--
+-- gh-4752: introduce sequence:current() method which
+-- fetches current sequence value but doesn't modify
+-- sequence itself.
+--
+sq = box.schema.sequence.create('test')
+sq:current()
+sq:next()
+sq:current()
+sq:set(42)
+sq:current()
+sq:current()
+sq:reset()
+sq:current()
+sq:drop()
-- 
2.23.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence
  2020-03-17 14:19       ` Nikita Pettik
@ 2020-03-17 19:16         ` Oleg Babin
  0 siblings, 0 replies; 10+ messages in thread
From: Oleg Babin @ 2020-03-17 19:16 UTC (permalink / raw)
  To: Nikita Pettik, Vladislav Shpilevoy; +Cc: Oleg Babin, tarantool-patches

On 17/03/2020 17:19, Nikita Pettik wrote:
> 
> Okay, np. Oleg, could you please re-send patch with all updates?
> 

Of course. I've re-sent the patch in 
https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/014862.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence
  2020-03-17 14:31       ` olegrok
@ 2020-03-17 20:40         ` Nikita Pettik
  2020-03-18  6:04           ` Oleg Babin
  0 siblings, 1 reply; 10+ messages in thread
From: Nikita Pettik @ 2020-03-17 20:40 UTC (permalink / raw)
  To: olegrok; +Cc: Oleg Babin, tarantool-patches

On 17 Mar 17:31, olegrok@tarantool.org wrote:
> From: Oleg Babin <babinoleg@mail.ru>
> 
> This patch introduces "current" function for sequences.
> It returns the last retrieved value of specified sequence or
> throws an error if no value has been generated yet.
> 
> This patch partially reverts 3ff1f1e36e14381c0ebb5862943d4da281254767
> (box: remove sequence_get) here similar function "get" was removed
> to avoid possible misleading with "currval" function of PosgreSQL
> that returns the last obtained value of the sequence in the scope
> of current session. In contrast "current" returns the last globally
> retrieved value of the sequence.
> 
> Closes #4752
> 
> @TarantoolBot document
> Title: sequence:current()
> 
> This patch introduces "current" function for sequences.
> It returns the last retrieved value of specified sequence or
> throws an error if no value has been generated yet ("next"
> has not been called yet or right after "reset" is called).

LGTM. Please, provide @Changelog and I will push it. Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence
  2020-03-17 20:40         ` Nikita Pettik
@ 2020-03-18  6:04           ` Oleg Babin
  2020-03-18 13:00             ` Nikita Pettik
  0 siblings, 1 reply; 10+ messages in thread
From: Oleg Babin @ 2020-03-18  6:04 UTC (permalink / raw)
  To: Nikita Pettik; +Cc: tarantool-patches



On 17/03/2020 23:40, Nikita Pettik wrote:
> 
> LGTM. Please, provide @Changelog and I will push it. Thanks.
> 

Sorry, I omitted this because it could be extracted from the root of 
e-mail thread.

Issue: https://github.com/tarantool/tarantool/issues/4752
Branch: 
https://github.com/tarantool/tarantool/tree/olegrok/4752-sequence-current

@ChangeLog
- sequence:current() - a function to get sequence's current
   value, without changing it (gh-4752).

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence
  2020-03-18  6:04           ` Oleg Babin
@ 2020-03-18 13:00             ` Nikita Pettik
  0 siblings, 0 replies; 10+ messages in thread
From: Nikita Pettik @ 2020-03-18 13:00 UTC (permalink / raw)
  To: Oleg Babin; +Cc: tarantool-patches

On 18 Mar 09:04, Oleg Babin wrote:
> 
> 
> On 17/03/2020 23:40, Nikita Pettik wrote:
> > 
> > LGTM. Please, provide @Changelog and I will push it. Thanks.
> > 
> 
> Sorry, I omitted this because it could be extracted from the root of e-mail
> thread.
> 
> Issue: https://github.com/tarantool/tarantool/issues/4752
> Branch:
> https://github.com/tarantool/tarantool/tree/olegrok/4752-sequence-current
> 
> @ChangeLog
> - sequence:current() - a function to get sequence's current
>   value, without changing it (gh-4752).

Pushed to master, updated 2.4 changelog correspondingly.
Branch has been dropped. Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-03-18 13:00 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-13 14:06 [Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence olegrok
2020-03-15 16:13 ` Vladislav Shpilevoy
2020-03-16 19:05   ` Oleg Babin
2020-03-16 23:04     ` Vladislav Shpilevoy
2020-03-17 14:19       ` Nikita Pettik
2020-03-17 19:16         ` Oleg Babin
2020-03-17 14:31       ` olegrok
2020-03-17 20:40         ` Nikita Pettik
2020-03-18  6:04           ` Oleg Babin
2020-03-18 13:00             ` Nikita Pettik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox