Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org,
	AKhatskevich <avkhatskevich@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH] sql: disallow id duplicate in insert & update
Date: Mon, 9 Apr 2018 23:30:00 +0300	[thread overview]
Message-ID: <ee120049-21b7-e521-acd9-833a672eddec@tarantool.org> (raw)
In-Reply-To: <20180409042952.25735-1-avkhatskevich@tarantool.org>

LGTM.

On 09/04/2018 07:29, AKhatskevich wrote:
> Before this patch one was able to exec query like this:
>    `insert into t(col1, col1) values(1, 1)`
>    `update t set col1 = 1, col1 = 2;`
> which is nonsense.
> 
> New policy is to throw an error in case of duplicates in columns
> description.
> 
> Closes #2358
> ---
>   src/box/sql/insert.c          | 15 +++++++++++++++
>   src/box/sql/sqliteLimit.h     |  9 +++++++++
>   src/box/sql/update.c          |  7 +++++++
>   test/sql-tap/insert1.test.lua | 30 +++++++++++++++++++++++++++++-
>   test/sql-tap/update.test.lua  | 23 ++++++++++++++++++++++-
>   5 files changed, 82 insertions(+), 2 deletions(-)
> 
> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> index 42254ddf4..33175e54c 100644
> --- a/src/box/sql/insert.c
> +++ b/src/box/sql/insert.c
> @@ -35,6 +35,7 @@
>    */
>   #include "sqliteInt.h"
>   #include "box/session.h"
> +#include "bit/bit.h"
>   
>   /*
>    * Generate code that will open pTab as cursor iCur.
> @@ -450,6 +451,12 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
>   	 * is appears in the original table.  (The index of the INTEGER
>   	 * PRIMARY KEY in the original table is pTab->iPKey.)
>   	 */
> +	/* Create bitmask to mark used columns of the table. */
> +	void *used_columns = tt_static_buf();
> +	/* The size of used_columns buffer is checked during compilation time
> +	 * using SQLITE_MAX_COLUMN constant.
> +	 */
> +	memset(used_columns, 0, (pTab->nCol + 7) / 8);
>   	bIdListInOrder = 1;
>   	if (pColumn) {
>   		for (i = 0; i < pColumn->nId; i++) {
> @@ -477,6 +484,14 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
>   				pParse->checkSchema = 1;
>   				goto insert_cleanup;
>   			}
> +			if (bit_test(used_columns, j)) {
> +				const char *err;
> +				err = "table id list: duplicate column name %s";
> +				sqlite3ErrorMsg(pParse,
> +						err, pColumn->a[i].zName);
> +				goto insert_cleanup;
> +			}
> +			bit_set(used_columns, j);
>   		}
>   	}
>   
> diff --git a/src/box/sql/sqliteLimit.h b/src/box/sql/sqliteLimit.h
> index 138268259..f6b580665 100644
> --- a/src/box/sql/sqliteLimit.h
> +++ b/src/box/sql/sqliteLimit.h
> @@ -35,6 +35,7 @@
>    *
>    * This file defines various limits of what SQLite can process.
>    */
> +#include "trivia/util.h"
>   
>   enum {
>   	/*
> @@ -74,6 +75,14 @@ enum {
>   #ifndef SQLITE_MAX_COLUMN
>   #define SQLITE_MAX_COLUMN 2000
>   #endif
> +/*
> + * tt_static_buf() is used to store bitmask for used columns in a table during
> + * SQL parsing stage. The following statement checks if static buffer is big
> + * enough to store the bitmask.
> + */
> +#if SQLITE_MAX_COLUMN > TT_STATIC_BUF_LEN * 8
> +#error "Bitmask for used table columns cannot fit into static buffer"
> +#endif
>   
>   /*
>    * The maximum length of a single SQL statement in bytes.
> diff --git a/src/box/sql/update.c b/src/box/sql/update.c
> index bf413252d..91717d7bd 100644
> --- a/src/box/sql/update.c
> +++ b/src/box/sql/update.c
> @@ -233,6 +233,13 @@ sqlite3Update(Parse * pParse,		/* The parser context */
>   				if (pPk && table_column_is_in_pk(pTab, j)) {
>   					chngPk = 1;
>   				}
> +				if (aXRef[j] != -1) {
> +					sqlite3ErrorMsg(pParse,
> +							"set id list: duplicate"
> +							" column name %s",
> +							pChanges->a[i].zName);
> +					goto update_cleanup;
> +				}
>   				aXRef[j] = i;
>   				break;
>   			}
> diff --git a/test/sql-tap/insert1.test.lua b/test/sql-tap/insert1.test.lua
> index 847b25ad2..750732d37 100755
> --- a/test/sql-tap/insert1.test.lua
> +++ b/test/sql-tap/insert1.test.lua
> @@ -1,7 +1,7 @@
>   #!/usr/bin/env tarantool
>   test = require("sqltester")
>   yaml = require('yaml')
> -test:plan(24)
> +test:plan(28)
>   
>   --!./tcltestrunner.lua
>   -- 2001 September 15
> @@ -516,5 +516,33 @@ test:do_execsql_test("insert-4.7", [[
>   --   SELECT * FROM t12c;
>   -- } {one xyzzy two}
>   -- integrity_check insert-99.0
> +
> +
> +test:do_execsql_test(
> +    "insert-13.0",
> +    [[
> +        create table test(a primary key, b)
> +    ]])
> +
> +test:do_catchsql_test(
> +    "insert-13.1",
> +    [[
> +        insert into test(a, a, b) values(1, 1, 1)
> +    ]],
> +    {1, "table id list: duplicate column name A"})
> +
> +test:do_catchsql_test(
> +    "insert-13.2",
> +    [[
> +        insert into test(a, b, b) values(1, 1, 1)
> +    ]],
> +    {1, "table id list: duplicate column name B"})
> +
> +test:do_execsql_test(
> +    "insert-13.3",
> +    [[
> +        drop table test;
> +    ]])
> +
>   test:finish_test()
>   
> diff --git a/test/sql-tap/update.test.lua b/test/sql-tap/update.test.lua
> index f6e756731..a4efe4015 100755
> --- a/test/sql-tap/update.test.lua
> +++ b/test/sql-tap/update.test.lua
> @@ -1,6 +1,6 @@
>   #!/usr/bin/env tarantool
>   test = require("sqltester")
> -test:plan(108)
> +test:plan(111)
>   
>   --!./tcltestrunner.lua
>   -- 2001 September 15
> @@ -1109,5 +1109,26 @@ test:do_catchsql_test("update-10.10", [[
>   --   UPDATE t15 SET c=printf("y%d",a) WHERE c IS NULL;
>   --   SELECT a,b,c,'|' FROM t15 ORDER BY a;
>   -- } {5 zyx y5 | 10 abc y10 | 15 wvu y15 | 20 def y20 | 25 tsr y25 | 30 ghi y30 | 35 qpo y35 |}
> +
> +test:do_execsql_test(
> +    "insert-15.0",
> +    [[
> +        create table test(a primary key);
> +        insert into test(a) values(1);
> +    ]])
> +
> +test:do_catchsql_test(
> +    "insert-15.1",
> +    [[
> +        update test set a = 2, a = 3;
> +    ]],
> +    {1, "set id list: duplicate column name A"})
> +
> +test:do_execsql_test(
> +  "insert-15.2",
> +  [[
> +      drop table test;
> +  ]])
> +
>   test:finish_test()
>   
> 

  reply	other threads:[~2018-04-09 20:30 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <DF3A58E7-87E9-4809-95D4-200923CD88CD@tarantool.org>
2018-04-09  4:29 ` [tarantool-patches] " AKhatskevich
2018-04-09 20:30   ` Vladislav Shpilevoy [this message]
2018-04-16 13:35     ` [tarantool-patches] " Kirill Yukhin

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=ee120049-21b7-e521-acd9-833a672eddec@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=avkhatskevich@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH] sql: disallow id duplicate in insert & update' \
    /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