From d1d90c865e2f88728aab2b666323db07d58116ab Mon Sep 17 00:00:00 2001
From: Georgy Moshkin <gmoshkin@picodata.io>
Date: Fri, 31 May 2024 18:49:09 +0300
Subject: [PATCH] fix: pretty sql parsing error messages
Previously when running `pico.sql [[ create table foo ]]` we would get
this output:
```
---
- null
- "sbroad: rule parsing error: --> 1:9\n |\n1 | create table foo \n | ^---\n
\ |\n = expected Unique"
...
```
But now the output will be:
```
---
- null
- |+
sbroad: rule parsing error: --> 1:9
|
1 | create table foo
| ^---
|
= expected Unique
...
```
---
src/sql.rs | 25 ++++++++++++++++++++++++-
src/sql/init.lua | 2 +-
test/int/test_cli_ux.py | 27 +++++++++++++++++++++++++++
test/int/test_sql.py | 4 ++--
4 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/src/sql.rs b/src/sql.rs
index 7e97897205..d224de318c 100644
--- a/src/sql.rs
+++ b/src/sql.rs
@@ -282,7 +282,30 @@ pub fn with_tracer(ctx: Context, tracer_kind: TracerKind) -> Context {
/// Dispatches a query to the cluster.
#[proc(packed_args)]
pub fn dispatch_query(encoded_params: EncodedPatternWithParams) -> traft::Result<Tuple> {
- dispatch_sql_query(encoded_params)
+ let res = dispatch_sql_query(encoded_params);
+ res.map_err(|e| {
+ match e {
+ Error::Sbroad(SbroadError::ParsingError(entity, message)) if message.contains('\n') => {
+ // Tweak the error message so that tarantool's yaml handler
+ // prints it in human-readable form
+ //
+ // `+ 1` here for one extra '\n' at the end
+ let mut buffer = String::with_capacity(message.len() + 1);
+ for line in message.lines() {
+ // There must not be any spaces at the end of lines,
+ // otherwise the string will be formatted incorrectly
+ buffer.push_str(line.trim_end());
+ buffer.push('\n');
+ }
+ // There must be at least one empty line so that tarantool
+ // formats the string correctly (it's a special hack they use
+ // for the help feature in the lua console).
+ buffer.push('\n');
+ SbroadError::ParsingError(entity, buffer.into()).into()
+ }
+ e => e,
+ }
+ })
}
pub fn dispatch_sql_query(encoded_params: EncodedPatternWithParams) -> traft::Result<Tuple> {
diff --git a/src/sql/init.lua b/src/sql/init.lua
index 7700f8b93f..963bddadc1 100644
--- a/src/sql/init.lua
+++ b/src/sql/init.lua
@@ -43,7 +43,7 @@ local function sql(...)
)
if ok == false then
- return nil, res
+ return nil, tostring(res)
end
return helper.format_result(res[1])
diff --git a/test/int/test_cli_ux.py b/test/int/test_cli_ux.py
index 3989aa8179..5612830f07 100644
--- a/test/int/test_cli_ux.py
+++ b/test/int/test_cli_ux.py
@@ -234,3 +234,30 @@ def test_sql_explain_ok(cluster: Cluster):
cli.expect_exact("execution options:")
cli.expect_exact("sql_vdbe_max_steps = 45000")
cli.expect_exact("vtable_max_rows = 5000")
+
+
+def test_lua_console_sql_error_messages(cluster: Cluster):
+ i1 = cluster.add_instance(wait_online=True)
+
+ result = i1.eval(
+ """
+ console = require 'console'
+ return console.eval ' pico.sql [[ create table foo ]] '
+ """
+ )
+
+ assert (
+ result
+ == """---
+- null
+- |+
+ sbroad: rule parsing error: --> 1:9
+ |
+ 1 | create table foo
+ | ^---
+ |
+ = expected Unique
+
+...
+"""
+ )
diff --git a/test/int/test_sql.py b/test/int/test_sql.py
index 451d160a77..07cd864d03 100644
--- a/test/int/test_sql.py
+++ b/test/int/test_sql.py
@@ -3466,7 +3466,7 @@ def test_rename_user(cluster: Cluster):
# Existed name
with pytest.raises(
ReturnError,
- match=f'User with name "{biba}" exists. Unable to rename user "{boba}".',
+ match=f'User with name "{biba}" exists. Unable to rename user "{boba}"',
):
data = i1.sudo_sql(
f"""
@@ -3479,7 +3479,7 @@ def test_rename_user(cluster: Cluster):
with pytest.raises(
ReturnError,
match="""\
-box error: AccessDenied: Alter access to user 'boba' is denied for user 'biba'.\
+box error: AccessDenied: Alter access to user 'boba' is denied for user 'biba'\
""",
):
data = i1.sql(
--
GitLab