[tarantool-patches] Re: [PATCH v3 2/2] sql: return last_insert_id via IPROTO

[n.pettik]

> After this patch client will get last_insert_id
> as additional metadata when he executes some
> statements.

Some statements? Lets write ‘when SQL statements are executed’ or sort of.

> 
> Part of #2618
> 
> @TarantoolBot document
> Title: SQL function last_insert_id() and IPROTO key last_insert_id.
> Function last_insert_id() returns first primary key value
> autogenerated in last INSERT/REPLACE statement in current
> session.

Mention that it is set only for AUTOINCREMENT primary key.
In other cases it will be 0.
Moreover, if you copy this feature from MySQL, you can take
part of its description: it is quite good documented there:
https://dev.mysql.com/doc/refman/8.0/en/mysql-insert-id.html <https://dev.mysql.com/doc/refman/8.0/en/mysql-insert-id.html>

The only doubt I have concerning this patch - last_insert_id()
returns FIRST autogenerated id for last stmt, which in turn
seems quite misleading (I expected that it returns LAST id of LAST stmt).
I known that MySQL uses exactly this variant, but should we follow this way?

> User can have more than one session and this
> function will work properly for each one of them. Return
> value of function is undetermined when more than one
> INSERT/REPLACE statements executed asynchronously.
> IPROTO key last_insert_id is a metadata returned through
> IPROTO after such statements as INSERT, REPLACE, UPDATE
> etc. Value of this key is equal to value returned by
> function last_insert_id() executed after the statement.
> Example:
> CREATE TABLE test (id INTEGER PRIMARY KEY AUTOINCREMENT, a INTEGER);
> INSERT INTO test VALUES (NULL, 1);
> SELECT last_insert_id();
> 
> diff --git a/src/box/session.cc b/src/box/session.cc
> index 64714cd..2b8dab9 100644
> --- a/src/box/session.cc
> +++ b/src/box/session.cc
> @@ -108,6 +108,7 @@ session_create(enum session_type type)
> 	session->type = type;
> 	session->sql_flags = default_flags;
> 	session->sql_default_engine = SQL_STORAGE_ENGINE_MEMTX;
> +	session->sql_last_insert_id = 0;
> 
> 	/* For on_connect triggers. */
> 	credentials_init(&session->credentials, guest_user->auth_token,
> diff --git a/src/box/session.h b/src/box/session.h
> index df1dcbc..6ee22bc 100644
> --- a/src/box/session.h
> +++ b/src/box/session.h
> @@ -92,6 +92,11 @@ union session_meta {
> struct session {
> 	/** Session id. */
> 	uint64_t id;
> +	/**
> +	 * First primary key autogenerated in last INSERT/REPLACE
> +	 * statement in which primary key was generated.
> +	 */

If AUTOINCEMENT is specified, otherwise it is 0.

> +	int64_t sql_last_insert_id;
> 	/** SQL Tarantool Default storage engine. */
> 	uint8_t sql_default_engine;
> 	/** SQL Connection flag for current user session */
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index 45056a7..2607cc3 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -38,6 +38,7 @@
> #include "vdbeInt.h"
> #include "version.h"
> #include "coll.h"
> +#include "box/session.h"
> #include <unicode/ustring.h>
> #include <unicode/ucasemap.h>
> #include <unicode/ucnv.h>
> @@ -601,6 +602,23 @@ changes(sqlite3_context * context, int NotUsed, sqlite3_value ** NotUsed2)
> }
> 
> /*
> + * Return first primary key autogenerated in last INSERT/REPLACE
> + * statement in which primary key was generated in current
> + * session.
> + *
> + * @param context Context being used.
> + * @param not_used Unused.
> + * @param not_used2 Unused.
> + */
> +static void
> +last_insert_id(sqlite3_context *context, int not_used,
> +	       sqlite3_value **not_used2)

Nitpick: use ’struct’ prefixes.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20180827/401e8cf1/attachment.html>