[tarantool-patches] [PATCH] sql: fix type in meta for unsigned binding

[tarantool-patches] [PATCH] sql: fix type in meta for unsigned binding

[Nikita Pettik]

It was decided that for all integer literals we would return "INTEGER"
type, not "UNSIGNED". Accidentally, after substitution of unsigned
binding value type was set to "UNSIGNED". Let's fix that and set
"INTEGER" type.
---
Branch: https://github.com/tarantool/tarantool/tree/np/fix-meta-uint-binding

 src/box/sql/vdbeapi.c  | 2 +-
 test/sql/iproto.result | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index ecf1b3601..f5885230b 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -1005,7 +1005,7 @@ sql_bind_uint64(struct sql_stmt *stmt, int i, uint64_t value)
 	struct Vdbe *p = (struct Vdbe *) stmt;
 	if (vdbeUnbind(p, i) != 0)
 		return -1;
-	int rc = sql_bind_type(p, i, "UNSIGNED");
+	int rc = sql_bind_type(p, i, "INTEGER");
 	mem_set_u64(&p->aVar[i - 1], value);
 	return rc;
 }
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index 26d12a51a..9639ba7a6 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -374,11 +374,11 @@ cn:execute('select $2, $1, $3', parameters)
 ---
 - metadata:
   - name: $2
-    type: UNSIGNED
+    type: INTEGER
   - name: $1
-    type: UNSIGNED
+    type: INTEGER
   - name: $3
-    type: UNSIGNED
+    type: INTEGER
   rows:
   - [22, 11, 33]
 ...
-- 
2.15.1

[tarantool-patches] Re: [PATCH] sql: fix type in meta for unsigned binding

[Konstantin Osipov]

* Nikita Pettik <korablev@tarantool.org> [19/07/24 19:59]:
> It was decided that for all integer literals we would return "INTEGER"
> type, not "UNSIGNED". Accidentally, after substitution of unsigned
> binding value type was set to "UNSIGNED". Let's fix that and set
> "INTEGER" type.

the patch is lgtm but I'm not sure it covers all cases.

I've seen a lot of unsigned inthe tests.


-- 
Konstantin Osipov, Moscow, Russia

[tarantool-patches] Re: [PATCH] sql: fix type in meta for unsigned binding

[n.pettik]

> On 24 Jul 2019, at 20:10, Konstantin Osipov <kostja@tarantool.org> wrote:
> 
> * Nikita Pettik <korablev@tarantool.org> [19/07/24 19:59]:
>> It was decided that for all integer literals we would return "INTEGER"
>> type, not "UNSIGNED". Accidentally, after substitution of unsigned
>> binding value type was set to "UNSIGNED". Let's fix that and set
>> "INTEGER" type.
> 
> the patch is lgtm but I'm not sure it covers all cases.
> 
> I've seen a lot of unsigned inthe tests.

I see only one more unsigned appearance:

box.execute("SELECT CAST('123' AS UNSIGNED);")
---
- metadata:
  - name: CAST('123' AS UNSIGNED)
    type: unsigned
  rows:
  - [123]
…

But I guess in this case user would be less surprised
if one saw unsigned type than integer.

> -- 
> Konstantin Osipov, Moscow, Russia

[tarantool-patches] Re: [PATCH] sql: fix type in meta for unsigned binding

[Konstantin Osipov]

* n.pettik <korablev@tarantool.org> [19/07/24 20:45]:
> 
> 
> > On 24 Jul 2019, at 20:10, Konstantin Osipov <kostja@tarantool.org> wrote:
> > 
> > * Nikita Pettik <korablev@tarantool.org> [19/07/24 19:59]:
> >> It was decided that for all integer literals we would return "INTEGER"
> >> type, not "UNSIGNED". Accidentally, after substitution of unsigned
> >> binding value type was set to "UNSIGNED". Let's fix that and set
> >> "INTEGER" type.
> > 
> > the patch is lgtm but I'm not sure it covers all cases.
> > 
> > I've seen a lot of unsigned inthe tests.
> 
> I see only one more unsigned appearance:
> 
> box.execute("SELECT CAST('123' AS UNSIGNED);")
> ---
> - metadata:
>   - name: CAST('123' AS UNSIGNED)
>     type: unsigned
>   rows:
>   - [123]
> …
> 
> But I guess in this case user would be less surprised
> if one saw unsigned type than integer.

This cast is very good. Very, very, good :) This cast is even better:

tarantool> select cast(NULL as unsigned)
{
  metadata = {
    {
      name = "cast(NULL as unsigned)",
      type = "unsigned"
    }
  },
  rows = {
    "[null]"
  }
}

This cast is what static types were all about. Not about a
separate compile-time pass with static analysis - it's an
implementation detail, and I'm not sure we'll ever need it,
it is merely a performance optimization. 

-- 
Konstantin Osipov, Moscow, Russia