Tarantool development patches archive
 help / color / mirror / Atom feed
From: imeevma@tarantool.org
To: v.shpilevoy@tarantool.org
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] [PATCH v9 2/7] sql: fix error code for SQL errors in execute.c
Date: Fri, 22 Mar 2019 13:50:35 +0300	[thread overview]
Message-ID: <bdcf3aac581fc6c34a248fa1f9b7ce956ad84e90.1553251042.git.imeevma@gmail.com> (raw)
In-Reply-To: <cover.1553251041.git.imeevma@gmail.com>

Currently, functions sql_execute() and sql_prepare_and_execute()
set the ER_SQL_EXECUTE code for all errors that occur during the
execution of a SQL command. This is considered incorrect because
some of these errors may have their own error code.

In addition, all errors that do not have an error code are VDBE
errors due to issue #3965, so it makes sense to set the error
code ER_VDBE_EXECUTE for all errors without an error code.

Part of #3505
---
 src/box/errcode.h                            |  1 +
 src/box/execute.c                            | 14 ++++++++--
 test/box/misc.result                         |  1 +
 test/sql/collation.result                    |  2 +-
 test/sql/gh-2362-select-access-rights.result | 12 +++------
 test/sql/iproto.result                       | 39 ++++++++++++----------------
 6 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 7764aa3..e39a89e 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -240,6 +240,7 @@ struct errcode_record {
 	/*185 */_(ER_SQL_UNKNOWN_TOKEN,		"Syntax error: unrecognized token: '%.*s'") \
 	/*186 */_(ER_SQL_PARSER_GENERIC,	"%s") \
 	/*187 */_(ER_SQL_ANALYZE_ARGUMENT,	"ANALYZE statement argument %s is not a base table") \
+	/*188 */_(ER_VDBE_EXECUTE,		"Error during execution of VDBE byte-code: %s") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/execute.c b/src/box/execute.c
index 7c77df2..5810086 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -523,7 +523,12 @@ sql_execute(sql *db, struct sql_stmt *stmt, struct port *port,
 		assert(rc != SQL_ROW && rc != SQL_OK);
 	}
 	if (rc != SQL_DONE) {
-		diag_set(ClientError, ER_SQL_EXECUTE, sql_errmsg(db));
+		if (db->errCode != SQL_TARANTOOL_ERROR) {
+			const char *err = (char *)sql_value_text(db->pErr);
+			if (err == NULL)
+				err = sqlErrStr(db->errCode);
+			diag_set(ClientError, ER_VDBE_EXECUTE, err);
+		}
 		return -1;
 	}
 	return 0;
@@ -541,7 +546,12 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
 		return -1;
 	}
 	if (sql_prepare_v2(db, sql, len, &stmt, NULL) != SQL_OK) {
-		diag_set(ClientError, ER_SQL_EXECUTE, sql_errmsg(db));
+		if (db->errCode != SQL_TARANTOOL_ERROR) {
+			const char *err = (char *)sql_value_text(db->pErr);
+			if (err == NULL)
+				err = sqlErrStr(db->errCode);
+			diag_set(ClientError, ER_VDBE_EXECUTE, err);
+		}
 		return -1;
 	}
 	assert(stmt != NULL);
diff --git a/test/box/misc.result b/test/box/misc.result
index c350bbd..4f1116e 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -516,6 +516,7 @@ t;
   185: box.error.SQL_UNKNOWN_TOKEN
   186: box.error.SQL_PARSER_GENERIC
   187: box.error.SQL_ANALYZE_ARGUMENT
+  188: box.error.VDBE_EXECUTE
 ...
 test_run:cmd("setopt delimiter ''");
 ---
diff --git a/test/sql/collation.result b/test/sql/collation.result
index 3794990..10872ca 100644
--- a/test/sql/collation.result
+++ b/test/sql/collation.result
@@ -102,7 +102,7 @@ cn = remote.connect(box.cfg.listen)
 ...
 cn:execute('select 1 limit ? collate not_exist', {1})
 ---
-- error: 'Failed to execute SQL statement: Syntax error near ''COLLATE'''
+- error: Syntax error near 'COLLATE'
 ...
 cn:close()
 ---
diff --git a/test/sql/gh-2362-select-access-rights.result b/test/sql/gh-2362-select-access-rights.result
index 0e5b9bf..8f1ecfa 100644
--- a/test/sql/gh-2362-select-access-rights.result
+++ b/test/sql/gh-2362-select-access-rights.result
@@ -46,8 +46,7 @@ c = nb.connect(box.cfg.listen)
 ...
 c:execute("SELECT * FROM t1;")
 ---
-- error: 'Failed to execute SQL statement: Read access to space ''T1'' is denied for
-    user ''guest'''
+- error: Read access to space 'T1' is denied for user 'guest'
 ...
 box.schema.user.grant('guest','read', 'space', 'T2')
 ---
@@ -57,8 +56,7 @@ c = nb.connect(box.cfg.listen)
 ...
 c:execute('SELECT * FROM t1, t2 WHERE t1.s1 = t2.s1')
 ---
-- error: 'Failed to execute SQL statement: Read access to space ''T1'' is denied for
-    user ''guest'''
+- error: Read access to space 'T1' is denied for user 'guest'
 ...
 box.sql.execute("CREATE VIEW v AS SELECT * FROM t1")
 ---
@@ -71,8 +69,7 @@ v = nb.connect(box.cfg.listen)
 ...
 c:execute('SELECT * FROM v')
 ---
-- error: 'Failed to execute SQL statement: Read access to space ''T1'' is denied for
-    user ''guest'''
+- error: Read access to space 'T1' is denied for user 'guest'
 ...
 box.sql.execute('CREATE TABLE t3 (s1 INT PRIMARY KEY, fk INT, FOREIGN KEY (fk) REFERENCES t1(s2))')
 ---
@@ -85,8 +82,7 @@ v = nb.connect(box.cfg.listen)
 ...
 c:execute('INSERT INTO t3 VALUES (1, 1)')
 ---
-- error: 'Failed to execute SQL statement: Read access to space ''T1'' is denied for
-    user ''guest'''
+- error: Read access to space 'T1' is denied for user 'guest'
 ...
 -- Cleanup
 box.schema.user.revoke('guest','read','space', 'V')
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index c700a80..7ad61e5 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -92,11 +92,11 @@ cn:execute('delete from test where a = 12')
 -- SQL errors.
 cn:execute('insert into not_existing_table values ("kek")')
 ---
-- error: 'Failed to execute SQL statement: Space ''NOT_EXISTING_TABLE'' does not exist'
+- error: Space 'NOT_EXISTING_TABLE' does not exist
 ...
 cn:execute('insert qwerty gjsdjq  q  qwd qmq;; q;qwd;')
 ---
-- error: 'Failed to execute SQL statement: Syntax error near ''qwerty'''
+- error: Syntax error near 'qwerty'
 ...
 -- Empty result.
 cn:execute('select id as identifier from test where a = 5;')
@@ -109,7 +109,7 @@ cn:execute('select id as identifier from test where a = 5;')
 -- netbox API errors.
 cn:execute(100)
 ---
-- error: 'Failed to execute SQL statement: Syntax error near ''100'''
+- error: Syntax error near '100'
 ...
 cn:execute('select 1', nil, {dry_run = true})
 ---
@@ -118,11 +118,11 @@ cn:execute('select 1', nil, {dry_run = true})
 -- Empty request.
 cn:execute('')
 ---
-- error: 'Failed to execute SQL statement: Failed to execute an empty SQL statement'
+- error: Failed to execute an empty SQL statement
 ...
 cn:execute('   ;')
 ---
-- error: 'Failed to execute SQL statement: Failed to execute an empty SQL statement'
+- error: Failed to execute an empty SQL statement
 ...
 --
 -- gh-3467: allow only positive integers under limit clause.
@@ -154,18 +154,15 @@ cn:execute('select * from test limit ?', {2})
 ...
 cn:execute('select * from test limit ?', {-2})
 ---
-- error: 'Failed to execute SQL statement: Only positive integers are allowed in the
-    LIMIT clause'
+- error: Only positive integers are allowed in the LIMIT clause
 ...
 cn:execute('select * from test limit ?', {2.7})
 ---
-- error: 'Failed to execute SQL statement: Only positive integers are allowed in the
-    LIMIT clause'
+- error: Only positive integers are allowed in the LIMIT clause
 ...
 cn:execute('select * from test limit ?', {'Hello'})
 ---
-- error: 'Failed to execute SQL statement: Only positive integers are allowed in the
-    LIMIT clause'
+- error: Only positive integers are allowed in the LIMIT clause
 ...
 cn:execute('select * from test limit 1 offset ?', {2})
 ---
@@ -181,18 +178,15 @@ cn:execute('select * from test limit 1 offset ?', {2})
 ...
 cn:execute('select * from test limit 1 offset ?', {-2})
 ---
-- error: 'Failed to execute SQL statement: Only positive integers are allowed in the
-    OFFSET clause'
+- error: Only positive integers are allowed in the OFFSET clause
 ...
 cn:execute('select * from test limit 1 offset ?', {2.7})
 ---
-- error: 'Failed to execute SQL statement: Only positive integers are allowed in the
-    OFFSET clause'
+- error: Only positive integers are allowed in the OFFSET clause
 ...
 cn:execute('select * from test limit 1 offset ?', {'Hello'})
 ---
-- error: 'Failed to execute SQL statement: Only positive integers are allowed in the
-    OFFSET clause'
+- error: Only positive integers are allowed in the OFFSET clause
 ...
 --
 -- Parameters binding.
@@ -363,7 +357,7 @@ sql = 'select '..string.rep('?, ', box.schema.SQL_BIND_PARAMETER_MAX)..'?'
 ...
 cn:execute(sql)
 ---
-- error: 'Failed to execute SQL statement: too many SQL variables'
+- error: too many SQL variables
 ...
 -- Try too many parameter values.
 sql = 'select ?'
@@ -567,12 +561,11 @@ cn:execute('drop table if exists test3')
 --
 cn:execute('select ?1, ?2, ?3', {1, 2, 3})
 ---
-- error: 'Failed to execute SQL statement: Syntax error near ''?1'''
+- error: Syntax error near '?1'
 ...
 cn:execute('select $name, $name2', {1, 2})
 ---
-- error: 'Failed to execute SQL statement: variable number must be between $1 and
-    $65000'
+- error: variable number must be between $1 and $65000
 ...
 parameters = {}
 ---
@@ -658,8 +651,8 @@ future1:wait_result()
 future2:wait_result()
 ---
 - null
-- 'Failed to execute SQL statement: Duplicate key exists in unique index ''pk_unnamed_TEST_1''
-  in space ''TEST'''
+- 'Error during execution of VDBE byte-code: Duplicate key exists in unique index
+  ''pk_unnamed_TEST_1'' in space ''TEST'''
 ...
 future3:wait_result()
 ---
-- 
2.7.4

  parent reply	other threads:[~2019-03-22 10:50 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-22 10:50 [tarantool-patches] [PATCH v9 0/7] sql: remove box.sql.execute imeevma
2019-03-22 10:50 ` [tarantool-patches] [PATCH v9 1/7] sql: add column name to SQL change counter imeevma
2019-03-22 15:42   ` [tarantool-patches] " Konstantin Osipov
2019-03-25 19:34     ` Mergen Imeev
2019-03-29 12:00   ` Kirill Yukhin
2019-03-22 10:50 ` imeevma [this message]
2019-03-22 15:45   ` [tarantool-patches] Re: [PATCH v9 2/7] sql: fix error code for SQL errors in execute.c Konstantin Osipov
2019-03-26 21:48   ` Vladislav Shpilevoy
2019-03-27 11:43     ` Konstantin Osipov
2019-03-28 17:46     ` Mergen Imeev
2019-03-29 12:01   ` Kirill Yukhin
2019-03-22 10:50 ` [tarantool-patches] [PATCH v9 3/7] sql: remove box.sql.debug() imeevma
2019-03-22 15:46   ` [tarantool-patches] " Konstantin Osipov
2019-03-25 19:39     ` Mergen Imeev
2019-03-26 21:48       ` Vladislav Shpilevoy
2019-03-28 17:48         ` Mergen Imeev
2019-03-28 18:01           ` Vladislav Shpilevoy
2019-03-29 12:02   ` Kirill Yukhin
2019-03-22 10:50 ` [tarantool-patches] [PATCH v9 4/7] lua: remove exceptions from function luaL_tofield() imeevma
2019-03-22 15:53   ` [tarantool-patches] " Konstantin Osipov
2019-03-29 19:26     ` Vladislav Shpilevoy
2019-03-26 21:48   ` Vladislav Shpilevoy
2019-03-28 17:54     ` Mergen Imeev
2019-03-28 18:40       ` Vladislav Shpilevoy
2019-03-28 19:56         ` Mergen Imeev
2019-03-28 21:41           ` Mergen Imeev
2019-03-29 21:06           ` Vladislav Shpilevoy
2019-03-22 10:50 ` [tarantool-patches] [PATCH v9 5/7] iproto: create port_sql imeevma
2019-03-22 15:55   ` [tarantool-patches] " Konstantin Osipov
2019-03-22 10:50 ` [tarantool-patches] [PATCH v9 6/7] sql: create box.execute() imeevma
2019-03-22 15:57   ` [tarantool-patches] " Konstantin Osipov
2019-03-22 10:50 ` [tarantool-patches] [PATCH v9 7/7] sql: remove box.sql.execute() imeevma
2019-03-26 21:48   ` [tarantool-patches] " Vladislav Shpilevoy
2019-03-28 20:13     ` Mergen Imeev
2019-03-29 21:06       ` Vladislav Shpilevoy
2019-03-29 21:07 ` [tarantool-patches] Re: [PATCH v9 0/7] sql: remove box.sql.execute Vladislav Shpilevoy

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=bdcf3aac581fc6c34a248fa1f9b7ce956ad84e90.1553251042.git.imeevma@gmail.com \
    --to=imeevma@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [tarantool-patches] [PATCH v9 2/7] sql: fix error code for SQL errors in execute.c' \
    /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