Tarantool development patches archive
 help / color / mirror / Atom feed
From: "n.pettik" <korablev@tarantool.org>
To: Hollow <hollow653@gmail.com>
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH] sql: correct confusing message
Date: Tue, 27 Mar 2018 19:36:00 +0300	[thread overview]
Message-ID: <D6FF4904-8F6B-40A1-9992-8CDA424A84A7@tarantool.org> (raw)
In-Reply-To: <1522166528-23387-1-git-send-email-hollow653@gmail.com>

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

If you decided to send patch v2, you should specify it:
git format-patch --subject-prefix='PATCH v2’

Moreover, as a rule if you send patch v2,
you should add brief change log.

Overall, patch seems to be OK, except for very minor remark:
please, deal with your IDE or text editor to avoid extra diffs
due to wrong indentation.

You can update remotely your branch and send only diff
in reply to this message.

> On 27 Mar 2018, at 19:02, N.Tatunov <hollow653@gmail.com> wrote:
> 
> From: Hollow <hollow653@gmail.com>
> 
> Required to fix the following message:
> 
> "- error: expressions prohibited in PRIMARY KEY and UNIQUE constraints"
> 
> Currently this message appears even when user tries to CREATE
> non-unique functional index which isn't concerned with the
> following error message. Thus far the message was corrected
> to the proper one.
> 
> Closes #3236
> ---
> 
> Branch: https://github.com/tarantool/tarantool
> Issue: https://github.com/tarantool/tarantool/issues/3236
> 
> src/box/sql/build.c                    |  8 +++----
> test/sql-tap/colname.test.lua          |  4 ++--
> test/sql/message-func-indexes.result   | 41 ++++++++++++++++++++++++++++++++++
> test/sql/message-func-indexes.test.lua | 18 +++++++++++++++
> 4 files changed, 65 insertions(+), 6 deletions(-)
> create mode 100644 test/sql/message-func-indexes.result
> create mode 100644 test/sql/message-func-indexes.test.lua
> 
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index dd0f45c..c7f6b3c 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -1802,7 +1802,7 @@ emitNewSysSpaceSequenceRecord(Parse *pParse, int space_id, const char reg_seq_id
> 
> 	/* 1. Space id  */
> 	sqlite3VdbeAddOp2(v, OP_SCopy, space_id, first_col + 1);
> -	
> +    

Still redundant diff. Please check it carefully and don’t hurry.

> 	/* 2. Sequence id  */
> 	sqlite3VdbeAddOp2(v, OP_IntCopy, reg_seq_id, first_col + 2);
> 
> @@ -2015,7 +2015,7 @@ sqlite3EndTable(Parse * pParse,	/* Parse context */
> 
> 			sqlite3OpenTable(pParse, iCursor, sys_space_sequence,
> 					 OP_OpenWrite);
> -			
> +            

The same: redundant diff.

> 			reg_space_seq_record = emitNewSysSpaceSequenceRecord(
> 				pParse,
> 				iSpaceId,
> @@ -3065,8 +3065,8 @@ sqlite3CreateIndex(Parse * pParse,	/* All information about this parse */
> 		pCExpr = sqlite3ExprSkipCollate(pListItem->pExpr);
> 		if (pCExpr->op != TK_COLUMN) {
> 			sqlite3ErrorMsg(pParse,
> -					"expressions prohibited in PRIMARY KEY and "
> -					"UNIQUE constraints");
> +					"functional indexes aren't supported "
> +					"in the current version");
> 			goto exit_create_index;
> 		} else {
> 			j = pCExpr->iColumn;
> diff --git a/test/sql-tap/colname.test.lua b/test/sql-tap/colname.test.lua
> index 71010da..c96623e 100755
> --- a/test/sql-tap/colname.test.lua
> +++ b/test/sql-tap/colname.test.lua
> @@ -643,13 +643,13 @@ test:do_catchsql_test(
>     "colname-11.2",
>     [[CREATE TABLE t1(a, b, c, d, e, 
>       PRIMARY KEY(a), UNIQUE('b' COLLATE "unicode_ci" DESC));]],
> -    {1, "/expressions prohibited in PRIMARY KEY/"})
> +    {1, "/functional indexes aren't supported in the current version/"})
> 
> test:execsql("create table table1(a primary key, b, c)")
> 
> test:do_catchsql_test(
>     "colname-11.3",
>     [[ CREATE INDEX t1c ON table1('c'); ]],
> -    {1, "/expressions prohibited in PRIMARY KEY/"})
> +    {1, "/functional indexes aren't supported in the current version/"})
> 
> test:finish_test()
> diff --git a/test/sql/message-func-indexes.result b/test/sql/message-func-indexes.result
> new file mode 100644
> index 0000000..797cf28
> --- /dev/null
> +++ b/test/sql/message-func-indexes.result
> @@ -0,0 +1,41 @@
> +test_run = require('test_run').new()
> +---
> +...
> +-- Creating tables.
> +box.sql.execute("CREATE TABLE t1(id INTEGER PRIMARY KEY, a INTEGER)")
> +---
> +...
> +box.sql.execute("CREATE TABLE t2(object INTEGER PRIMARY KEY, price INTEGER, count INTEGER)")
> +---
> +...
> +-- Expressions that're supposed to create functional indexes
> +-- should return certain message.
> +box.sql.execute("CREATE INDEX i1 ON t1(a+1)")
> +---
> +- error: functional indexes aren't supported in the current version
> +...
> +box.sql.execute("CREATE INDEX i2 ON t1(a)")
> +---
> +...
> +box.sql.execute("CREATE INDEX i3 ON t2(price + 100)")
> +---
> +- error: functional indexes aren't supported in the current version
> +...
> +box.sql.execute("CREATE INDEX i4 ON t2(price)")
> +---
> +...
> +box.sql.execute("CREATE INDEX i5 ON t2(count + 1)")
> +---
> +- error: functional indexes aren't supported in the current version
> +...
> +box.sql.execute("CREATE INDEX i6 ON t2(count * price)")
> +---
> +- error: functional indexes aren't supported in the current version
> +...
> +-- Cleaning up.
> +box.sql.execute("DROP TABLE t1")
> +---
> +...
> +box.sql.execute("DROP TABLE t2") 
> +---
> +...
> diff --git a/test/sql/message-func-indexes.test.lua b/test/sql/message-func-indexes.test.lua
> new file mode 100644
> index 0000000..fac715b
> --- /dev/null
> +++ b/test/sql/message-func-indexes.test.lua
> @@ -0,0 +1,18 @@
> +test_run = require('test_run').new()
> +
> +-- Creating tables.
> +box.sql.execute("CREATE TABLE t1(id INTEGER PRIMARY KEY, a INTEGER)")
> +box.sql.execute("CREATE TABLE t2(object INTEGER PRIMARY KEY, price INTEGER, count INTEGER)")
> +
> +-- Expressions that're supposed to create functional indexes
> +-- should return certain message.
> +box.sql.execute("CREATE INDEX i1 ON t1(a+1)")
> +box.sql.execute("CREATE INDEX i2 ON t1(a)")
> +box.sql.execute("CREATE INDEX i3 ON t2(price + 100)")
> +box.sql.execute("CREATE INDEX i4 ON t2(price)")
> +box.sql.execute("CREATE INDEX i5 ON t2(count + 1)")
> +box.sql.execute("CREATE INDEX i6 ON t2(count * price)")
> +
> +-- Cleaning up.
> +box.sql.execute("DROP TABLE t1")
> +box.sql.execute("DROP TABLE t2") 
> -- 
> 2.7.4
> 
> 


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

  reply	other threads:[~2018-03-27 16:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-27 16:02 [tarantool-patches] " N.Tatunov
2018-03-27 16:36 ` n.pettik [this message]
  -- strict thread matches above, loose matches on Subject: below --
2018-03-27 11:49 Hollow
2018-03-27 13:13 ` [tarantool-patches] " n.pettik

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=D6FF4904-8F6B-40A1-9992-8CDA424A84A7@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=hollow653@gmail.com \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH] sql: correct confusing message' \
    /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