* [tarantool-patches] [PATCH] sql: correct confusing message
@ 2018-03-27 11:49 Hollow
2018-03-27 13:13 ` [tarantool-patches] " n.pettik
0 siblings, 1 reply; 3+ messages in thread
From: Hollow @ 2018-03-27 11:49 UTC (permalink / raw)
To: tarantool-patches; +Cc: korablev, Hollow
Required to fix the following message:
"- error: expressions prohibited in PRIMARY KEY and UNIQUE constraints"
Currently this message appears when user tries to CREATE
functional index but it doesn't seem clear what is actually wrong.
Thus far the message was corrected to the one that explains
what is wrong with expression in the command.
Closes #3236
---
src/box/sql/build.c | 13 ++++++-----
test/sql-tap/colname.test.lua | 6 ++---
test/sql/message-func-indexes.result | 40 ++++++++++++++++++++++++++++++++++
test/sql/message-func-indexes.test.lua | 17 +++++++++++++++
4 files changed, 67 insertions(+), 9 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..8de9e62 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1,4 +1,4 @@
-/*
+ /*
* Copyright 2010-2017, Tarantool AUTHORS, please see AUTHORS file.
*
* Redistribution and use in source and binary forms, with or
@@ -987,7 +987,8 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */
sqlite3ExprSkipCollate(pList->a[i].pExpr);
assert(pCExpr != 0);
if (pCExpr->op != TK_ID) {
- sqlite3ErrorMsg(pParse, "expressions prohibited in PRIMARY KEY");
+ sqlite3ErrorMsg(pParse, "PRIMARY KEY can't be based on "
+ "non-COLUMN entities");
goto primary_key_exit;
}
const char *zCName = pCExpr->u.zToken;
@@ -1802,7 +1803,7 @@ emitNewSysSpaceSequenceRecord(Parse *pParse, int space_id, const char reg_seq_id
/* 1. Space id */
sqlite3VdbeAddOp2(v, OP_SCopy, space_id, first_col + 1);
-
+
/* 2. Sequence id */
sqlite3VdbeAddOp2(v, OP_IntCopy, reg_seq_id, first_col + 2);
@@ -2015,7 +2016,7 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
sqlite3OpenTable(pParse, iCursor, sys_space_sequence,
OP_OpenWrite);
-
+
reg_space_seq_record = emitNewSysSpaceSequenceRecord(
pParse,
iSpaceId,
@@ -3065,8 +3066,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..b6e7fc9 100755
--- a/test/sql-tap/colname.test.lua
+++ b/test/sql-tap/colname.test.lua
@@ -637,19 +637,19 @@ test:do_test(
test:do_catchsql_test(
"colname-11.1",
[[ create table t1(a, b, c, primary key('A'))]],
- {1, "expressions prohibited in PRIMARY KEY"})
+ {1, "PRIMARY KEY can't be based on non-COLUMN entities"})
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..30359df
--- /dev/null
+++ b/test/sql/message-func-indexes.result
@@ -0,0 +1,40 @@
+test_run = require('test_run').new()
+---
+...
+-- Creating tables
+box.sql.execute("CREATE TABLE t1(id PRIMARY KEY, a)")
+---
+...
+box.sql.execute("CREATE TABLE t2(object INT PRIMARY KEY, price INT, count INT)")
+---
+...
+-- Expressions that're supposed to create functional indexes are supposed to 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: 'near "1": syntax error'
+...
+box.sql.execute("CREATE INDEX i6 ON t2(count + count)")
+---
+- 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..43dd30a
--- /dev/null
+++ b/test/sql/message-func-indexes.test.lua
@@ -0,0 +1,17 @@
+test_run = require('test_run').new()
+
+-- Creating tables
+box.sql.execute("CREATE TABLE t1(id PRIMARY KEY, a)")
+box.sql.execute("CREATE TABLE t2(object INT PRIMARY KEY, price INT, count INT)")
+
+-- Expressions that're supposed to create functional indexes are supposed to 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 + count)")
+
+-- Cleaning up
+box.sql.execute("DROP TABLE t1")
+box.sql.execute("DROP TABLE t2")
--
2.7.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: correct confusing message
2018-03-27 11:49 [tarantool-patches] [PATCH] sql: correct confusing message Hollow
@ 2018-03-27 13:13 ` n.pettik
0 siblings, 0 replies; 3+ messages in thread
From: n.pettik @ 2018-03-27 13:13 UTC (permalink / raw)
To: Hollow; +Cc: tarantool-patches
Hello.
First of all, please use real name: it is hard to understand who
has sent patch.
Also, add link to the branch and link to the issue after ‘—‘:
after git format-patch you still can edit text file.
For example:
‘''
Closes #3236
---
Branch: xxx
Issue: xxx
src/box/sql/build.c | 13 ++++++——
‘''
Finally, before sending patch, make sure that there is no redundant changes
(i.e. non functional or/and has nothing in common with intentions of the patch).
You can do it with ‘git diff HEAD~1’ command or any GUI tool (such as gitk).
Consider comments below: fix them or argue in case you are not agree.
You can send patch v2, or reply on this message and provide fixed code.
> On 27 Mar 2018, at 14:49, Hollow <hollow653@gmail.com> wrote:
>
> Required to fix the following message:
>
> "- error: expressions prohibited in PRIMARY KEY and UNIQUE constraints"
>
> Currently this message appears when user tries to CREATE
> functional index but it doesn't seem clear what is actually wrong.
Provide some clarification: this message may appear when user
creates non-unique index — it is initial problem.
User also can create UNIQUE index, and then this message will be correct.
> Thus far the message was corrected to the one that explains
> what is wrong with expression in the command.
>
> Closes #3236
> ---
> src/box/sql/build.c | 13 ++++++-----
> test/sql-tap/colname.test.lua | 6 ++---
> test/sql/message-func-indexes.result | 40 ++++++++++++++++++++++++++++++++++
> test/sql/message-func-indexes.test.lua | 17 +++++++++++++++
> 4 files changed, 67 insertions(+), 9 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..8de9e62 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -1,4 +1,4 @@
> -/*
> + /*
> * Copyright 2010-2017, Tarantool AUTHORS, please see AUTHORS file.
Redundant diff.
> *
> * Redistribution and use in source and binary forms, with or
> @@ -987,7 +987,8 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */
> sqlite3ExprSkipCollate(pList->a[i].pExpr);
> assert(pCExpr != 0);
> if (pCExpr->op != TK_ID) {
> - sqlite3ErrorMsg(pParse, "expressions prohibited in PRIMARY KEY");
> + sqlite3ErrorMsg(pParse, "PRIMARY KEY can't be based on "
> + "non-COLUMN entities”);
I guess previous message was OK. I see no reason to change it: they are almost identical.
> goto primary_key_exit;
> }
> const char *zCName = pCExpr->u.zToken;
> @@ -1802,7 +1803,7 @@ emitNewSysSpaceSequenceRecord(Parse *pParse, int space_id, const char reg_seq_id
>
> /* 1. Space id */
> sqlite3VdbeAddOp2(v, OP_SCopy, space_id, first_col + 1);
> -
> +
Redundant diff.
Even if there is wrong indentation, it has nothing in common with this patch.
For example, you can fix it in separate patch/commit, if you want do it.
> /* 2. Sequence id */
> sqlite3VdbeAddOp2(v, OP_IntCopy, reg_seq_id, first_col + 2);
>
> @@ -2015,7 +2016,7 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
>
> sqlite3OpenTable(pParse, iCursor, sys_space_sequence,
> OP_OpenWrite);
> -
> +
Redundant diff.
> reg_space_seq_record = emitNewSysSpaceSequenceRecord(
> pParse,
> iSpaceId,
> @@ -3065,8 +3066,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”);
I wouldn’t outline word ‘indexes’ with upper-case.
You can check out style of error messages in src/box/errcode.h
They must be identical for all parts of Tarantool.
> 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..b6e7fc9 100755
> --- a/test/sql-tap/colname.test.lua
> +++ b/test/sql-tap/colname.test.lua
> @@ -637,19 +637,19 @@ test:do_test(
> test:do_catchsql_test(
> "colname-11.1",
> [[ create table t1(a, b, c, primary key('A'))]],
> - {1, "expressions prohibited in PRIMARY KEY"})
> + {1, "PRIMARY KEY can't be based on non-COLUMN entities"})
>
> 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..30359df
> --- /dev/null
> +++ b/test/sql/message-func-indexes.result
> @@ -0,0 +1,40 @@
> +test_run = require('test_run').new()
> +---
> +...
> +-- Creating tables
> +box.sql.execute("CREATE TABLE t1(id PRIMARY KEY, a)")
> +---
> +...
> +box.sql.execute("CREATE TABLE t2(object INT PRIMARY KEY, price INT, count INT)")
> +---
> +...
> +-- Expressions that're supposed to create functional indexes are supposed to 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: 'near "1": syntax error’
I guess this test is not correct within this patch.
> +...
> +box.sql.execute("CREATE INDEX i6 ON t2(count + count)")
> +---
> +- 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..43dd30a
> --- /dev/null
> +++ b/test/sql/message-func-indexes.test.lua
> @@ -0,0 +1,17 @@
> +test_run = require('test_run').new()
> +
> +-- Creating tables
> +box.sql.execute("CREATE TABLE t1(id PRIMARY KEY, a)”)
We are going to introduce strict typing in SQL, so it would be great
if you specified type of columns (in order to avoid future refactoring tests).
> +box.sql.execute("CREATE TABLE t2(object INT PRIMARY KEY, price INT, count INT)")
> +
> +-- Expressions that're supposed to create functional indexes are supposed to return certain message
Please, fit comments in 80 chars (also, in source code they should fit into 66).
It would be better if you put dot at the end of sentence.
> +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 + count)")
> +
> +-- Cleaning up
> +box.sql.execute("DROP TABLE t1")
> +box.sql.execute("DROP TABLE t2")
^ permalink raw reply [flat|nested] 3+ messages in thread
* [tarantool-patches] [PATCH] sql: correct confusing message
@ 2018-03-27 16:02 N.Tatunov
0 siblings, 0 replies; 3+ messages in thread
From: N.Tatunov @ 2018-03-27 16:02 UTC (permalink / raw)
To: tarantool-patches; +Cc: korablev, Hollow
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);
-
+
/* 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);
-
+
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-27 16:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-27 11:49 [tarantool-patches] [PATCH] sql: correct confusing message Hollow
2018-03-27 13:13 ` [tarantool-patches] " n.pettik
2018-03-27 16:02 [tarantool-patches] " N.Tatunov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox