Re: [Tarantool-patches] [PATCH v2 3/6] sql: extend result set with collation

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the patch!

I've pushed my review fixes on top of this commit. See it below
and on the branch.

> diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
> index 48db0bf43..e57a80334 100644
> --- a/src/box/sql/vdbeapi.c
> +++ b/src/box/sql/vdbeapi.c
> @@ -745,6 +745,15 @@ sql_column_datatype(sql_stmt *stmt, int n)
>  	return p->metadata[n].type;
>  }
>  
> +const char *
> +sql_column_coll(sql_stmt *stmt, int n)
> +{
> +	struct Vdbe *p = (struct Vdbe *) stmt;
> +	assert(n < sql_column_count(stmt) && n >= 0);
> +	return p->metadata[n].collation;
> +}
> +
> +

1. Double empty line.

>  /******************************* sql_bind_  **************************
>   *
>   * Routines used to attach values to wildcards in a compiled SQL statement.
> diff --git a/test/sql/full_metadata.test.lua b/test/sql/full_metadata.test.lua
> new file mode 100644
> index 000000000..4aa2492a1
> --- /dev/null
> +++ b/test/sql/full_metadata.test.lua

2. Seems like you use tabs in that test file. According to
our code style, we use 4-space blocks in Lua files instead
of tabs.

================================================================================

commit 635cd1999708d0d731e46ddda64a97e4da55d082
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date:   Tue Dec 24 15:46:28 2019 +0100

    Review fix 3/6

diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index e57a80334..61b98e0c5 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -753,7 +753,6 @@ sql_column_coll(sql_stmt *stmt, int n)
 	return p->metadata[n].collation;
 }
 
-
 /******************************* sql_bind_  **************************
  *
  * Routines used to attach values to wildcards in a compiled SQL statement.
diff --git a/test/sql/full_metadata.result b/test/sql/full_metadata.result
index aebb938fe..76d6107a1 100644
--- a/test/sql/full_metadata.result
+++ b/test/sql/full_metadata.result
@@ -26,18 +26,18 @@ test_run:cmd("setopt delimiter ';'")
  | - true
  | ...
 if remote then
-	box.schema.user.grant('guest','read, write, execute', 'universe')
-	box.schema.user.grant('guest', 'create', 'space')
-	cn = netbox.connect(box.cfg.listen)
-	execute = function(...) return cn:execute(...) end
+    box.schema.user.grant('guest','read, write, execute', 'universe')
+    box.schema.user.grant('guest', 'create', 'space')
+    cn = netbox.connect(box.cfg.listen)
+    execute = function(...) return cn:execute(...) end
 else
-	execute = function(...)
-		local res, err = box.execute(...)
-		if err ~= nil then
-			error(err)
-		end
-		return res
-	end
+    execute = function(...)
+        local res, err = box.execute(...)
+        if err ~= nil then
+            error(err)
+        end
+        return res
+    end
 end;
  | ---
  | ...
@@ -90,9 +90,9 @@ test_run:cmd("setopt delimiter ';'")
  | - true
  | ...
 if remote then
-	cn:close()
-	box.schema.user.revoke('guest', 'read, write, execute', 'universe')
-	box.schema.user.revoke('guest', 'create', 'space')
+    cn:close()
+    box.schema.user.revoke('guest', 'read, write, execute', 'universe')
+    box.schema.user.revoke('guest', 'create', 'space')
 end;
  | ---
  | ...
diff --git a/test/sql/full_metadata.test.lua b/test/sql/full_metadata.test.lua
index 4aa2492a1..7b4637b1e 100644
--- a/test/sql/full_metadata.test.lua
+++ b/test/sql/full_metadata.test.lua
@@ -8,18 +8,18 @@ remote = test_run:get_cfg('remote') == 'true'
 execute = nil
 test_run:cmd("setopt delimiter ';'")
 if remote then
-	box.schema.user.grant('guest','read, write, execute', 'universe')
-	box.schema.user.grant('guest', 'create', 'space')
-	cn = netbox.connect(box.cfg.listen)
-	execute = function(...) return cn:execute(...) end
+    box.schema.user.grant('guest','read, write, execute', 'universe')
+    box.schema.user.grant('guest', 'create', 'space')
+    cn = netbox.connect(box.cfg.listen)
+    execute = function(...) return cn:execute(...) end
 else
-	execute = function(...)
-		local res, err = box.execute(...)
-		if err ~= nil then
-			error(err)
-		end
-		return res
-	end
+    execute = function(...)
+        local res, err = box.execute(...)
+        if err ~= nil then
+            error(err)
+        end
+        return res
+    end
 end;
 test_run:cmd("setopt delimiter ''");
 
@@ -34,9 +34,9 @@ execute("PRAGMA full_metadata = false;")
 
 test_run:cmd("setopt delimiter ';'")
 if remote then
-	cn:close()
-	box.schema.user.revoke('guest', 'read, write, execute', 'universe')
-	box.schema.user.revoke('guest', 'create', 'space')
+    cn:close()
+    box.schema.user.revoke('guest', 'read, write, execute', 'universe')
+    box.schema.user.revoke('guest', 'create', 'space')
 end;
 test_run:cmd("setopt delimiter ''");