Tarantool development patches archive
 help / color / mirror / Atom feed
From: "n.pettik" <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Imeev Mergen <imeevma@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH v3 2/2] sql: return last_insert_id via IPROTO
Date: Mon, 27 Aug 2018 17:25:53 +0300	[thread overview]
Message-ID: <12075140-4CA8-4857-BB13-84286FF72498@tarantool.org> (raw)
In-Reply-To: <48d0ec1183bfe373652eef3cf13fb70e6a631b80.1535107514.git.imeevma@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3659 bytes --]


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


[-- Attachment #2: Type: text/html, Size: 6501 bytes --]

      reply	other threads:[~2018-08-27 14:25 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-24 10:57 [tarantool-patches] [PATCH v3 0/2] " imeevma
2018-08-24 10:57 ` [tarantool-patches] [PATCH v3 1/2] sql: move autoincrement in vdbe imeevma
2018-08-27 14:27   ` [tarantool-patches] " n.pettik
2018-08-24 10:57 ` [tarantool-patches] [PATCH v3 2/2] sql: return last_insert_id via IPROTO imeevma
2018-08-27 14:25   ` n.pettik [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=12075140-4CA8-4857-BB13-84286FF72498@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH v3 2/2] sql: return last_insert_id via IPROTO' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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