[Tarantool-patches] [PATCH v1 1/1] sql: check arguments types of built-in functions
Mergen Imeev
imeevma at tarantool.org
Fri Oct 16 04:20:26 MSK 2020
Hi! Thank you for the review! I send this patch to Nikita.
My answers and diff below.
On Thu, Oct 15, 2020 at 11:23:00PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the fixes!
>
> On 14.10.2020 00:44, Mergen Imeev wrote:
> > Hi! Thank you for the review! My answers, diff and new patch below.
> >
> > On Fri, Oct 09, 2020 at 11:08:32PM +0200, Vladislav Shpilevoy wrote:
> >> Hi! Thanks for the patch!
> >>
> >> See 12 comments/questions below.
> >>
> >> 1.
> >>
> >> tarantool> box.execute("SELECT GREATEST(1, '2')")
> >> ---
> >> - metadata:
> >> - name: COLUMN_1
> >> type: scalar
> >> rows:
> >> - ['2']
> >> ...
> >>
> >> Is it supposed to happen? Shouldn't the function check that all
> >> arguments are of the same type? The same for LEAST().
> >>
> > At the moment LEAST() ang GREATEST() works using SCALAR rules from BOX for
> > comparison. Not sure if this is right, though.
>
> Isn't this patch aimed to fix such places?
>
No, as far as I know comparison rules for this functions are not defined.
However, I thought that applying SCALAR rules here is quite good
decision. Not sure though.
> >>> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> >>> index 0aedb2d3d..d348c3d09 100644
> >>> --- a/src/box/sql/func.c
> >>> +++ b/src/box/sql/func.c
> >>> @@ -467,30 +467,21 @@ lengthFunc(sql_context * context, int argc, sql_value ** argv)
> >>>
> >>> assert(argc == 1);
> >>> UNUSED_PARAMETER(argc);
> >>> - switch (sql_value_type(argv[0])) {
> >>> - case MP_BIN:
> >>> - case MP_ARRAY:
> >>> - case MP_MAP:
> >>> - case MP_INT:
> >>> - case MP_UINT:
> >>> - case MP_BOOL:
> >>> - case MP_DOUBLE:{
> >>> - sql_result_uint(context, sql_value_bytes(argv[0]));
> >>> - break;
> >>> - }
> >>> - case MP_STR:{
> >>> - const unsigned char *z = sql_value_text(argv[0]);
> >>> - if (z == 0)
> >>> - return;
> >>> - len = sql_utf8_char_count(z, sql_value_bytes(argv[0]));
> >>> - sql_result_uint(context, len);
> >>> - break;
> >>> - }
> >>> - default:{
> >>> - sql_result_null(context);
> >>> - break;
> >>> - }
> >>> - }
> >>> + enum mp_type type = mem_mp_type(argv[0]);
> >>> + if (type == MP_NIL)
> >>> + return sql_result_null(context);
> >>> + if (type == MP_STR) {
> >>> + const unsigned char *z = sql_value_text(argv[0]);
> >>> + if (z == NULL)
> >>> + return sql_result_null(context);
> >>> + len = sql_utf8_char_count(z, sql_value_bytes(argv[0]));
> >>> + return sql_result_uint(context, len);
> >>> + }
> >>> + if (type == MP_BIN)
> >>> + return sql_result_uint(context, sql_value_bytes(argv[0]));
> >>> + diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> >>> + sql_value_to_diag_str(argv[0]), "string or varbinary");
> >>> + context->is_aborted = true;
> >>
> >> 6. What was wrong with leaving it a switch-case? Here and in other places.
> >> For more than 2 similarly looking checks switch usually looks better, and
> >> probably works better as well (but this I don't know for sure, I just
> >> read some stuff about comilers being able to generate kind of smart goto
> >> table for swithches).
> >>
> > Mostly because shich can be used only in functions that accepts only one
> > argument.
>
> Not exactly. As I see, mostly when you accept multiple arguments, they have
> the same type. So you could switch-test one of them, and the compare == to
> others.
>
I still believe that in near future all these checks will be removed from these
functions. All these checks will be executed by OP_ApplyType, so the more they
will be similar to each other, the easier it will be to remove them. Just my
thoughts.
> > I think it is better that all of built-in functions (except typeof())
> > have the same structure. Just my thoughts, though.
>
> Whatever, leave it as you want. Personally I would chose switch where
> possible, but no strict rules here.
>
> >>> @@ -988,9 +979,9 @@ randomBlob(sql_context * context, int argc, sql_value ** argv)
> >>> unsigned char *p;
> >>> assert(argc == 1);
> >>> UNUSED_PARAMETER(argc);
> >>> - if (mp_type_is_bloblike(sql_value_type(argv[0]))) {
> >>> + if (!mp_type_is_numeric(sql_value_type(argv[0]))) {
> >>> diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> >>> - sql_value_to_diag_str(argv[0]), "numeric");
> >>> + sql_value_to_diag_str(argv[0]), "integer");
> >>
> >> 9. You check for numeric, but print integer? So if I pass 1.1 it will
> >> pass, and will return 1 random byte. But should throw an error. Is there
> >> a test on that?
> >>
> > According to our implicit cast rules, numbers of all types may be converted to
> > INTEGER. However, currently these functions cannot fully work with our INTEGER
> > type. They may only wor with int64. This also will be mentioned in the issue.
>
> What issue?
>
I mean this issue:
>> I plan to create an issue. In this issue I plan to suggest to rework built-in
>> functions so they could work right according to ANSI. Defining argument types
>> will be part of the issue.
I haven't filled this issue fow now. I will do this after this patch is pushed
to master.
> See 9 comments below. Please, send next version to Nikita. We need to
> hurry up, and I am sure he has a lot to add.
>
> > From d1a97a66e58ad09aa9b1f7dea02b42f5d560c0bc Mon Sep 17 00:00:00 2001
> > From: Mergen Imeev <imeevma at gmail.com>
> > Date: Thu, 8 Oct 2020 00:23:18 +0300
> > Subject: [PATCH] sql: check arguments types of built-in functions
> >
> > After this patch, the argument types of the SQL built-in functions will
> > be checked. Implicit casting rules will be applied during this check.
> >
> > Closes #4159
> >
> > diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> > index 0aedb2d3d..0da6c8f06 100644
> > --- a/src/box/sql/func.c
> > +++ b/src/box/sql/func.c
> > @@ -467,30 +467,21 @@ lengthFunc(sql_context * context, int argc, sql_value ** argv)
> >
> > assert(argc == 1);
> > UNUSED_PARAMETER(argc);
> > - switch (sql_value_type(argv[0])) {
> > - case MP_BIN:
> > - case MP_ARRAY:
> > - case MP_MAP:
> > - case MP_INT:
> > - case MP_UINT:
> > - case MP_BOOL:
> > - case MP_DOUBLE:{
> > - sql_result_uint(context, sql_value_bytes(argv[0]));
> > - break;
> > - }
> > - case MP_STR:{
> > - const unsigned char *z = sql_value_text(argv[0]);
> > - if (z == 0)
> > - return;
> > - len = sql_utf8_char_count(z, sql_value_bytes(argv[0]));
> > - sql_result_uint(context, len);
> > - break;
> > - }
> > - default:{
> > - sql_result_null(context);
> > - break;
> > - }
> > - }
> > + enum mp_type type = mem_mp_type(argv[0]);
> > + if (type == MP_NIL)
> > + return sql_result_null(context);
> > + if (type == MP_STR) {
> > + const unsigned char *z = sql_value_text(argv[0]);
> > + if (z == NULL)
> > + return sql_result_null(context);
> > + len = sql_utf8_char_count(z, sql_value_bytes(argv[0]));
> > + return sql_result_uint(context, len);
> > + }
> > + if (type == MP_BIN)
> > + return sql_result_uint(context, sql_value_bytes(argv[0]));
> > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> > + sql_value_to_diag_str(argv[0]), "string or varbinary");
> > + context->is_aborted = true;
> > }
> >
> > /*
> > @@ -504,44 +495,24 @@ absFunc(sql_context * context, int argc, sql_value ** argv)
> > {
> > assert(argc == 1);
> > UNUSED_PARAMETER(argc);
> > - switch (sql_value_type(argv[0])) {
> > - case MP_UINT: {
> > - sql_result_uint(context, sql_value_uint64(argv[0]));
> > - break;
> > - }
> > - case MP_INT: {
> > + enum mp_type type = mem_mp_type(argv[0]);
> > + if (type == MP_NIL)
> > + return sql_result_null(context);
> > + if (type == MP_UINT)
> > + return sql_result_uint(context, sql_value_uint64(argv[0]));
> > + if (type == MP_INT) {
> > int64_t value = sql_value_int64(argv[0]);
> > - assert(value < 0);
> > - sql_result_uint(context, -value);
> > - break;
> > - }
> > - case MP_NIL:{
> > - /* IMP: R-37434-19929 Abs(X) returns NULL if X is NULL. */
> > - sql_result_null(context);
> > - break;
> > - }
> > - case MP_BOOL:
> > - case MP_BIN:
> > - case MP_ARRAY:
> > - case MP_MAP: {
> > - diag_set(ClientError, ER_INCONSISTENT_TYPES, "number",
> > - mem_type_to_str(argv[0]));
> > - context->is_aborted = true;
> > - return;
> > + return sql_result_uint(context, (uint64_t)(-value));
> > }
> > - default:{
> > - /* Because sql_value_double() returns 0.0 if the argument is not
> > - * something that can be converted into a number, we have:
> > - * IMP: R-01992-00519 Abs(X) returns 0.0 if X is a string or blob
> > - * that cannot be converted to a numeric value.
> > - */
> > - double rVal = sql_value_double(argv[0]);
> > - if (rVal < 0)
> > - rVal = -rVal;
> > - sql_result_double(context, rVal);
> > - break;
> > - }
> > + if (type == MP_DOUBLE) {
> > + double value = sql_value_double(argv[0]);
> > + if (value < 0)
> > + value = -value;
> > + return sql_result_double(context, value);
> > }
> > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> > + sql_value_to_diag_str(argv[0]), "number");
> > + context->is_aborted = true;
> > }
> >
> > /**
> > @@ -564,34 +535,31 @@ position_func(struct sql_context *context, int argc, struct Mem **argv)
> > enum mp_type needle_type = sql_value_type(needle);
> > enum mp_type haystack_type = sql_value_type(haystack);
> >
> > - if (haystack_type == MP_NIL || needle_type == MP_NIL)
> > - return;
> > - /*
> > - * Position function can be called only with string
> > - * or blob params.
> > - */
> > - struct Mem *inconsistent_type_arg = NULL;
> > - if (needle_type != MP_STR && needle_type != MP_BIN)
> > - inconsistent_type_arg = needle;
> > - if (haystack_type != MP_STR && haystack_type != MP_BIN)
> > - inconsistent_type_arg = haystack;
> > - if (inconsistent_type_arg != NULL) {
> > - diag_set(ClientError, ER_INCONSISTENT_TYPES,
> > - "text or varbinary",
> > - mem_type_to_str(inconsistent_type_arg));
> > + if (needle_type != MP_NIL && needle_type != MP_STR &&
> > + needle_type != MP_BIN) {
> > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> > + sql_value_to_diag_str(needle), "string or varbinary");
> > context->is_aborted = true;
> > return;
> > }
> > - /*
> > - * Both params of Position function must be of the same
> > - * type.
> > - */
> > - if (haystack_type != needle_type) {
> > - diag_set(ClientError, ER_INCONSISTENT_TYPES,
> > - mem_type_to_str(needle), mem_type_to_str(haystack));
> > - context->is_aborted = true;
> > - return;
> > + if (haystack_type != MP_NIL && haystack_type != needle_type) {
> > + if (needle_type != MP_NIL) {
> > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> > + sql_value_to_diag_str(haystack),
> > + mem_type_to_str(needle));
> > + context->is_aborted = true;
> > + return;
> > + }
> > + if (haystack_type != MP_STR && haystack_type != MP_BIN) {
> > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> > + sql_value_to_diag_str(haystack),
> > + "string or varbinary");
> > + context->is_aborted = true;
> > + return;
> > + }
> > }
> > + if (needle_type == MP_NIL || haystack_type == MP_NIL)
> > + return;
> >
> > int n_needle_bytes = sql_value_bytes(needle);
> > int n_haystack_bytes = sql_value_bytes(haystack);
> > @@ -707,6 +675,17 @@ printfFunc(sql_context * context, int argc, sql_value ** argv)
> > }
> > }
> >
> > +static bool
> > +is_num_to_int_possible(struct Mem *mem)
>
> 1. The function lacks a comment. Why is it allowed to convert a double
> to an int anywhere? Need to mention that this is the allowed implicit
> cast.
>
Added a comment. Also moved mp_type_is_numeric() check here and renamed this
function.
> > +{
> > + enum mp_type type = mem_mp_type(mem);
> > + assert(mp_type_is_numeric(type));
> > + if (type == MP_INT || type == MP_UINT)
> > + return true;
> > + assert(type == MP_DOUBLE);
> > + return mem->u.r >= (double)INT64_MIN && mem->u.r < (double)UINT64_MAX;
>
> 2. Why not <= UINT64_MAX, why <?
>
Because (double)UINT64_MAX == 2^64, which is not part INTEGER.
> Also would be better to handle such checks inside sql_value_int(). It
> already can return a proper success status from sqlVdbeIntValue(), but
> ignores its result. Maybe not a part of this patchset though due to
> lack of time.
>
I agree that it is better to fix later, since we need to fix all functions that
receive integer values through this function.
> > @@ -735,22 +713,45 @@ substrFunc(sql_context * context, int argc, sql_value ** argv)
> > context->is_aborted = true;
> > return;
> > }
> > - if (sql_value_is_null(argv[1])
> > - || (argc == 3 && sql_value_is_null(argv[2]))
> > - ) {
> > + enum mp_type type0 = mem_mp_type(argv[0]);
> > + if (type0 != MP_NIL && type0 != MP_STR && type0 != MP_BIN) {
> > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> > + sql_value_to_diag_str(argv[0]), "string or varbinary");
> > + context->is_aborted = true;
> > return;
> > }
> > - p0type = sql_value_type(argv[0]);
> > + enum mp_type type1 = mem_mp_type(argv[1]);
> > + if (type1 != MP_NIL && (!mp_type_is_numeric(type1) ||
> > + !is_num_to_int_possible(argv[1]))) {
> > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> > + sql_value_to_diag_str(argv[1]), "integer");
> > + context->is_aborted = true;
> > + return;
> > + }
> > + enum mp_type type2 = MP_UINT;
> > + if (argc == 3) {
> > + type2 = mem_mp_type(argv[2]);
> > + if (type2 != MP_NIL && (!mp_type_is_numeric(type2) ||
> > + !is_num_to_int_possible(argv[2]))) {
>
> 3. This construction 'is_numeric && int_possible' is repeated
> 6 times in this file. Maybe time to extract into a new function.
> You could just move mp_type_is_numeric() into is_num_to_int_possible(),
> for example.
>
Fixed. Moved to is_num_to_int_possible().
> > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> > + sql_value_to_diag_str(argv[2]), "integer");
> > + context->is_aborted = true;
> > + return;
> > + }
> > + }
> > + if (type0 == MP_NIL || type1 == MP_NIL || type2 == MP_NIL)
> > + return;
> > +
> > p1 = sql_value_int(argv[1]);
>
> 4. is_num_to_int_possible() allows values > INT64_MAX, so it will
> overflow here.
>
True, however it works a bit differently than usual overflow. This should be
fixed in all places sql_value_int(), but I suggest to do this as part of a
different issue, for example one I mentioned above (which is not created
for now, though).
> > - if (p0type == MP_BIN) {
> > + if (type0 == MP_BIN) {
> > len = sql_value_bytes(argv[0]);
> > z = sql_value_blob(argv[0]);
> > - if (z == 0)
> > + if (z == NULL)
> > return;
> > assert(len == sql_value_bytes(argv[0]));
> > } else {
> > z = sql_value_text(argv[0]);
> > - if (z == 0)
> > + if (z == NULL)
> > return;
> > len = 0;
> > if (p1 < 0)
> > @@ -763,9 +764,9 @@ substrFunc(sql_context * context, int argc, sql_value ** argv)
> > negP2 = 1;
> > }
> > } else {
> > - p2 = sql_context_db_handle(context)->
> > - aLimit[SQL_LIMIT_LENGTH];
> > + p2 = sql_context_db_handle(context)->aLimit[SQL_LIMIT_LENGTH];
> > }
> > +
>
> 5. This whole diff hunk and the 2 changes above it are not necessary.
>
Removed.
> > if (p1 < 0) {
> > p1 += len;
> > if (p1 < 0) {
> > @@ -836,22 +837,32 @@ roundFunc(sql_context * context, int argc, sql_value ** argv)
> > context->is_aborted = true;
> > return;
> > }
> > + enum mp_type type0 = mem_mp_type(argv[0]);
> > + if (type0!= MP_NIL && !mp_type_is_numeric(type0)) {
>
> 6. Missing whitespace after type0.
>
Fixed.
> > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> > + sql_value_to_diag_str(argv[0]), "numeric");
> > + context->is_aborted = true;
> > + return;
> > + }
> > + enum mp_type type1 = MP_UINT;
> > if (argc == 2) {
> > - if (sql_value_is_null(argv[1]))
> > + type1 = mem_mp_type(argv[1]);
> > + if (type1 != MP_NIL && (!mp_type_is_numeric(type1) ||
> > + !is_num_to_int_possible(argv[1]))) {
> > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> > + sql_value_to_diag_str(argv[1]), "integer");
> > + context->is_aborted = true;
> > return;
> > + }
> > + }
> > + if (type0 == MP_NIL || type1 == MP_NIL)
> > + return;
> > +
> > + if (argc == 2) {
> > n = sql_value_int(argv[1]);
> > if (n < 0)
> > n = 0;
> > }
> > - if (sql_value_is_null(argv[0]))
> > - return;
> > - enum mp_type mp_type = sql_value_type(argv[0]);
> > - if (mp_type_is_bloblike(mp_type)) {
> > - diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> > - sql_value_to_diag_str(argv[0]), "numeric");
> > - context->is_aborted = true;
> > - return;
> > - }
> > r = sql_value_double(argv[0]);
> > /* If Y==0 and X will fit in a 64-bit int,
> > * handle the rounding directly,
> > @@ -907,9 +918,11 @@ case_type##ICUFunc(sql_context *context, int argc, sql_value **argv) \
> > int n; \
> > UNUSED_PARAMETER(argc); \
> > int arg_type = sql_value_type(argv[0]); \
> > - if (mp_type_is_bloblike(arg_type)) { \
> > - diag_set(ClientError, ER_INCONSISTENT_TYPES, "text", \
> > - "varbinary"); \
> > + if (arg_type == MP_NIL) \
> > + return; \
> > + if (arg_type != MP_STR) { \
>
> 7. I would better use a single 'if' for != MP_STR, and would check for
> == MP_NIL inside it. Because almost always the value != MP_NIL, and
> therefore we would avoid unnecessary branching on the common path. Up
> to you.
>
Fixed here and in one more place.
> > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, \
> > + sql_value_to_diag_str(argv[0]), "string"); \
> > context->is_aborted = true; \
> > return; \
> > } \
> > @@ -1750,6 +1846,12 @@ trim_func_one_arg(struct sql_context *context, sql_value *arg)
> > enum mp_type val_type = sql_value_type(arg);
> > if (val_type == MP_NIL)
> > return;
> > + if (val_type != MP_STR && !mp_type_is_bloblike(val_type)) {
>
> 8. mp_type_is_bloblike() is true for arrays and maps. Does not look like
> a good check for TRIM function. The same for other usages of this func.
>
Fixed in all places in this file.
> > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> > + sql_value_to_diag_str(arg), "string or varbinary");
> > + context->is_aborted = true;
> > + return;
> > + }
> > diff --git a/test/sql-tap/position.test.lua b/test/sql-tap/position.test.lua
> > index e0455abc9..0a2c42173 100755
> > --- a/test/sql-tap/position.test.lua
> > +++ b/test/sql-tap/position.test.lua
> > @@ -238,7 +238,7 @@ test:do_test(
> > return test:catchsql "SELECT position(34, 123456.78);"
> > end, {
> > -- <position-1.24>
> > - 1, "Inconsistent types: expected text or varbinary got real"
> > + 1, "Type mismatch: can not convert 34 to string or varbinary"
>
> 9. Tbh, the old error looked better. The new one is strange, because 34
> can be converted to a string. It just does not happen implicitly. So the
> message is misleading.
Agree, but this is how error looks in all other places where such implicit case
is disallowed. I think we should fix this error, but not in this issue.
Diff:
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 0da6c8f06..4d79c6cb6 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -675,11 +675,16 @@ printfFunc(sql_context * context, int argc, sql_value ** argv)
}
}
+/*
+ * Make sure you can get INTEGER values from Mem according to implicit casting
+ * rules. Currently, only numbers can be converted to INTEGER.
+ */
static bool
-is_num_to_int_possible(struct Mem *mem)
+is_get_int_possible(struct Mem *mem)
{
enum mp_type type = mem_mp_type(mem);
- assert(mp_type_is_numeric(type));
+ if (!mp_type_is_numeric(type))
+ return false;
if (type == MP_INT || type == MP_UINT)
return true;
assert(type == MP_DOUBLE);
@@ -721,8 +726,7 @@ substrFunc(sql_context * context, int argc, sql_value ** argv)
return;
}
enum mp_type type1 = mem_mp_type(argv[1]);
- if (type1 != MP_NIL && (!mp_type_is_numeric(type1) ||
- !is_num_to_int_possible(argv[1]))) {
+ if (type1 != MP_NIL && (!is_get_int_possible(argv[1]))) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(argv[1]), "integer");
context->is_aborted = true;
@@ -731,8 +735,7 @@ substrFunc(sql_context * context, int argc, sql_value ** argv)
enum mp_type type2 = MP_UINT;
if (argc == 3) {
type2 = mem_mp_type(argv[2]);
- if (type2 != MP_NIL && (!mp_type_is_numeric(type2) ||
- !is_num_to_int_possible(argv[2]))) {
+ if (type2 != MP_NIL && (!is_get_int_possible(argv[2]))) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(argv[2]), "integer");
context->is_aborted = true;
@@ -746,12 +749,12 @@ substrFunc(sql_context * context, int argc, sql_value ** argv)
if (type0 == MP_BIN) {
len = sql_value_bytes(argv[0]);
z = sql_value_blob(argv[0]);
- if (z == NULL)
+ if (z == 0)
return;
assert(len == sql_value_bytes(argv[0]));
} else {
z = sql_value_text(argv[0]);
- if (z == NULL)
+ if (z == 0)
return;
len = 0;
if (p1 < 0)
@@ -764,9 +767,9 @@ substrFunc(sql_context * context, int argc, sql_value ** argv)
negP2 = 1;
}
} else {
- p2 = sql_context_db_handle(context)->aLimit[SQL_LIMIT_LENGTH];
+ p2 = sql_context_db_handle(context)->
+ aLimit[SQL_LIMIT_LENGTH];
}
-
if (p1 < 0) {
p1 += len;
if (p1 < 0) {
@@ -838,7 +841,7 @@ roundFunc(sql_context * context, int argc, sql_value ** argv)
return;
}
enum mp_type type0 = mem_mp_type(argv[0]);
- if (type0!= MP_NIL && !mp_type_is_numeric(type0)) {
+ if (type0 != MP_NIL && !mp_type_is_numeric(type0)) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(argv[0]), "numeric");
context->is_aborted = true;
@@ -847,8 +850,7 @@ roundFunc(sql_context * context, int argc, sql_value ** argv)
enum mp_type type1 = MP_UINT;
if (argc == 2) {
type1 = mem_mp_type(argv[1]);
- if (type1 != MP_NIL && (!mp_type_is_numeric(type1) ||
- !is_num_to_int_possible(argv[1]))) {
+ if (type1 != MP_NIL && (!is_get_int_possible(argv[1]))) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(argv[1]), "integer");
context->is_aborted = true;
@@ -918,9 +920,9 @@ case_type##ICUFunc(sql_context *context, int argc, sql_value **argv) \
int n; \
UNUSED_PARAMETER(argc); \
int arg_type = sql_value_type(argv[0]); \
- if (arg_type == MP_NIL) \
- return; \
if (arg_type != MP_STR) { \
+ if (arg_type == MP_NIL) \
+ return; \
diag_set(ClientError, ER_SQL_TYPE_MISMATCH, \
sql_value_to_diag_str(argv[0]), "string"); \
context->is_aborted = true; \
@@ -1003,8 +1005,7 @@ randomBlob(sql_context * context, int argc, sql_value ** argv)
UNUSED_PARAMETER(argc);
if (sql_value_is_null(argv[0]))
return;
- if (!mp_type_is_numeric(mem_mp_type(argv[0])) ||
- !is_num_to_int_possible(argv[0])) {
+ if (!is_get_int_possible(argv[0])) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(argv[0]), "integer");
context->is_aborted = true;
@@ -1506,8 +1507,7 @@ charFunc(sql_context * context, int argc, sql_value ** argv)
unsigned c;
if (sql_value_is_null(argv[i]))
continue;
- if (!mp_type_is_numeric(mem_mp_type(argv[i])) ||
- !is_num_to_int_possible(argv[i])) {
+ if (!is_get_int_possible(argv[i])) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(argv[i]), "integer");
context->is_aborted = true;
@@ -1584,8 +1584,7 @@ zeroblobFunc(sql_context * context, int argc, sql_value ** argv)
UNUSED_PARAMETER(argc);
if (sql_value_is_null(argv[0]))
return;
- if (!mp_type_is_numeric(mem_mp_type(argv[0])) ||
- !is_num_to_int_possible(argv[0])) {
+ if (!is_get_int_possible(argv[0])) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(argv[0]), "integer");
context->is_aborted = true;
@@ -1844,15 +1843,15 @@ trim_func_one_arg(struct sql_context *context, sql_value *arg)
/* In case of VARBINARY type default trim octet is X'00'. */
const unsigned char *default_trim;
enum mp_type val_type = sql_value_type(arg);
- if (val_type == MP_NIL)
- return;
- if (val_type != MP_STR && !mp_type_is_bloblike(val_type)) {
+ if (val_type != MP_STR && val_type != MP_BIN) {
+ if (val_type == MP_NIL)
+ return;
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(arg), "string or varbinary");
context->is_aborted = true;
return;
}
- if (mp_type_is_bloblike(val_type))
+ if (val_type == MP_BIN)
default_trim = (const unsigned char *) "\0";
else
default_trim = (const unsigned char *) " ";
@@ -1880,7 +1879,7 @@ trim_func_two_args(struct sql_context *context, sql_value *arg1,
{
const unsigned char *input_str, *trim_set;
enum mp_type type2 = sql_value_type(arg2);
- if (type2 != MP_NIL && type2 != MP_STR && !mp_type_is_bloblike(type2)) {
+ if (type2 != MP_NIL && type2 != MP_STR && type2 != MP_BIN) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(arg2), "string or varbinary");
context->is_aborted = true;
@@ -1898,7 +1897,7 @@ trim_func_two_args(struct sql_context *context, sql_value *arg1,
input_str, input_str_sz);
return;
}
- if (type1 != MP_NIL && type1 != MP_STR && !mp_type_is_bloblike(type1)) {
+ if (type1 != MP_NIL && type1 != MP_STR && type1 != MP_BIN) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(arg1), "string or varbinary");
context->is_aborted = true;
@@ -1935,14 +1934,14 @@ trim_func_three_args(struct sql_context *context, sql_value *arg1,
{
assert(sql_value_type(arg1) == MP_INT || sql_value_type(arg1) == MP_UINT);
enum mp_type type2 = sql_value_type(arg2);
- if (type2 != MP_NIL && type2 != MP_STR && !mp_type_is_bloblike(type2)) {
+ if (type2 != MP_NIL && type2 != MP_STR && type2 != MP_BIN) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(arg2), "string or varbinary");
context->is_aborted = true;
return;
}
enum mp_type type3 = sql_value_type(arg3);
- if (type3 != MP_NIL && type3 != MP_STR && !mp_type_is_bloblike(type3)) {
+ if (type3 != MP_NIL && type3 != MP_STR && type3 != MP_BIN) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(arg2), "string or varbinary");
context->is_aborted = true;
More information about the Tarantool-patches
mailing list