diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 3b3b1f01da624868260db4b2564416cbcaa24039..9a4f38bb9670f61738b60bd0b3b2a3ce68b3ede5 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -846,16 +846,6 @@ vdbe_field_ref_fetch_data(struct vdbe_field_ref *field_ref, uint32_t fieldno) return field_begin; } -static inline enum field_type -vdbe_field_ref_fetch_type(struct vdbe_field_ref *field_ref, uint32_t fieldno) -{ - const struct tuple_field *tf = - vdbe_field_ref_fetch_field(field_ref, fieldno); - if (tf == NULL || tf->type == FIELD_TYPE_ANY) - return field_type_MAX; - return tf->type; -} - /** * Fetch field by fieldno using vdbe_field_ref and store result * in dest_mem. @@ -879,17 +869,6 @@ vdbe_field_ref_fetch(struct vdbe_field_ref *field_ref, uint32_t fieldno, if (vdbe_decode_msgpack_into_mem(data, dest_mem, &dummy) != 0) return -1; - /* - * MsgPack map, array or extension (unsupported in sql). - * Wrap it in a blob verbatim. - */ - if (dest_mem->flags == 0) { - dest_mem->z = (char *) data; - dest_mem->n = vdbe_field_ref_fetch_data(field_ref, - fieldno + 1) - data; - dest_mem->flags = MEM_Blob | MEM_Ephem | MEM_Subtype; - dest_mem->subtype = SQL_SUBTYPE_MSGPACK; - } /* * Add 0 termination (at most for strings) * Not sure why do we check MEM_Ephem @@ -909,7 +888,6 @@ vdbe_field_ref_fetch(struct vdbe_field_ref *field_ref, uint32_t fieldno, dest_mem->flags |= MEM_Term; } UPDATE_MAX_BLOBSIZE(dest_mem); - dest_mem->field_type = vdbe_field_ref_fetch_type(field_ref, fieldno); return 0; } diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 91b64316e36fa42703a7b64f727357c3c79e4142..7724763773b72e63a2feddac868225a128490df3 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -2793,38 +2793,62 @@ vdbe_decode_msgpack_into_mem(const char *buf, struct Mem *mem, uint32_t *len) { const char *start_buf = buf; switch (mp_typeof(*buf)) { - case MP_ARRAY: - case MP_MAP: - case MP_EXT: - default: { - mem->flags = 0; + case MP_ARRAY: { + mem->z = (char *)buf; + mp_next(&buf); + mem->n = buf - mem->z; + mem->flags = MEM_Blob | MEM_Ephem | MEM_Subtype; + mem->subtype = SQL_SUBTYPE_MSGPACK; + mem->field_type = FIELD_TYPE_ARRAY; + break; + } + case MP_MAP: { + mem->z = (char *)buf; + mp_next(&buf); + mem->n = buf - mem->z; + mem->flags = MEM_Blob | MEM_Ephem | MEM_Subtype; + mem->subtype = SQL_SUBTYPE_MSGPACK; + mem->field_type = FIELD_TYPE_MAP; + break; + } + case MP_EXT: { + mem->z = (char *)buf; + mp_next(&buf); + mem->n = buf - mem->z; + mem->flags = MEM_Blob | MEM_Ephem; + mem->field_type = FIELD_TYPE_VARBINARY; break; } case MP_NIL: { mp_decode_nil(&buf); mem->flags = MEM_Null; + mem->field_type = field_type_MAX; break; } case MP_BOOL: { mem->u.b = mp_decode_bool(&buf); mem->flags = MEM_Bool; + mem->field_type = FIELD_TYPE_BOOLEAN; break; } case MP_UINT: { uint64_t v = mp_decode_uint(&buf); mem->u.u = v; mem->flags = MEM_UInt; + mem->field_type = FIELD_TYPE_INTEGER; break; } case MP_INT: { mem->u.i = mp_decode_int(&buf); mem->flags = MEM_Int; + mem->field_type = FIELD_TYPE_INTEGER; break; } case MP_STR: { /* XXX u32->int */ mem->n = (int) mp_decode_strl(&buf); mem->flags = MEM_Str | MEM_Ephem; + mem->field_type = FIELD_TYPE_STRING; install_blob: mem->z = (char *)buf; buf += mem->n; @@ -2834,18 +2858,33 @@ vdbe_decode_msgpack_into_mem(const char *buf, struct Mem *mem, uint32_t *len) /* XXX u32->int */ mem->n = (int) mp_decode_binl(&buf); mem->flags = MEM_Blob | MEM_Ephem; + mem->field_type = FIELD_TYPE_VARBINARY; goto install_blob; } case MP_FLOAT: { mem->u.r = mp_decode_float(&buf); - mem->flags = sqlIsNaN(mem->u.r) ? MEM_Null : MEM_Real; + if (sqlIsNaN(mem->u.r)) { + mem->flags = MEM_Null; + mem->field_type = FIELD_TYPE_DOUBLE; + } else { + mem->flags = MEM_Real; + mem->field_type = FIELD_TYPE_DOUBLE; + } break; } case MP_DOUBLE: { mem->u.r = mp_decode_double(&buf); - mem->flags = sqlIsNaN(mem->u.r) ? MEM_Null : MEM_Real; + if (sqlIsNaN(mem->u.r)) { + mem->flags = MEM_Null; + mem->field_type = FIELD_TYPE_DOUBLE; + } else { + mem->flags = MEM_Real; + mem->field_type = FIELD_TYPE_DOUBLE; + } break; } + default: + unreachable(); } *len = (uint32_t)(buf - start_buf); return 0; @@ -2868,15 +2907,8 @@ sqlVdbeRecordUnpackMsgpack(struct key_def *key_def, /* Information about the rec pMem->z = 0; uint32_t sz = 0; vdbe_decode_msgpack_into_mem(zParse, pMem, &sz); - if (sz == 0) { - /* MsgPack array, map or ext. Treat as blob. */ - pMem->z = (char *)zParse; - mp_next(&zParse); - pMem->n = zParse - pMem->z; - pMem->flags = MEM_Blob | MEM_Ephem; - } else { - zParse += sz; - } + assert(sz != 0); + zParse += sz; pMem++; } } diff --git a/test/sql-tap/gh-5913-segfault-on-select-uuid.test.lua b/test/sql-tap/gh-5913-segfault-on-select-uuid.test.lua new file mode 100755 index 0000000000000000000000000000000000000000..60978c2b575f93ea8a319849e229e65538063cec --- /dev/null +++ b/test/sql-tap/gh-5913-segfault-on-select-uuid.test.lua @@ -0,0 +1,99 @@ +#!/usr/bin/env tarantool +local test = require("sqltester") +test:plan(6) + +local uuid = require("uuid").fromstr("11111111-1111-1111-1111-111111111111") +local decimal = require("decimal").new(111.111) + +box.schema.create_space('T') +box.space.T:format({{name = "I", type = "integer"}, {name = "U", type = "uuid"}, + {name = "D", type = "decimal"}}) +box.space.T:create_index("primary") +box.space.T:insert({1, uuid, decimal}) + +-- +-- Make sure that there is no segmentation fault on select from field that +-- contains UUID or DECIMAL. Currently SQL does not support UUID and DECIMAL, +-- so they treated as VARBINARY. +-- +test:do_execsql_test( + "gh-5913-1", + [[ + SELECT i, u, d FROM t; + SELECT i from t; + ]], { + 1 + }) + +box.schema.create_space('T1') +box.space.T1:format({{name = "I", type = "integer"}, + {name = "U", type = "uuid", is_nullable = true}, + {name = "D", type = "decimal", is_nullable = true}}) +box.space.T1:create_index("primary") + +-- +-- Since SQL does not support UUID and DECIMAL and they treated as VARBINARY, +-- they cannot be inserted from SQL. +-- +test:do_catchsql_test( + "gh-5913-2", + [[ + INSERT INTO t1 SELECT i, u, NULL FROM t; + ]], { + 1, "Type mismatch: can not convert varbinary to uuid" + }) + +test:do_catchsql_test( + "gh-5913-3", + [[ + INSERT INTO t1 SELECT i, NULL, d FROM t; + ]], { + 1, "Type mismatch: can not convert varbinary to decimal" + }) + +-- +-- Still, if UUID or DECIMAL fields does not selected directly, insert is +-- working properly. +-- +test:do_execsql_test( + "gh-5913-4", + [[ + INSERT INTO t1 SELECT * FROM t; + SELECT count() FROM t1; + ]], { + 1 + }) + +box.schema.create_space('TU') +box.space.TU:format({{name = "I", type = "integer"}, + {name = "U", type = "uuid"}}) +box.space.TU:create_index("primary") +box.space.TU:insert({1, uuid}) + +box.schema.create_space('TD') +box.space.TD:format({{name = "I", type = "integer"}, + {name = "D", type = "decimal"}}) +box.space.TD:create_index("primary") +box.space.TD:insert({1, decimal}) + +-- +-- Update of UUID or VARBINARY also does not lead to segfault, however throws an +-- error since after changing value cannot be inserted into the field from SQL. +-- +test:do_catchsql_test( + "gh-5913-5", + [[ + UPDATE tu SET u = u; + ]], { + 1, "Type mismatch: can not convert varbinary to uuid" + }) + +test:do_catchsql_test( + "gh-5913-6", + [[ + UPDATE td SET d = d; + ]], { + 1, "Type mismatch: can not convert varbinary to decimal" + }) + +test:finish_test()