[tarantool-patches] Re: [PATCH 2/5] sql: fix resulting type calculation for CASE-WHEN stmt
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Fri Jul 26 01:12:34 MSK 2019
Hi! Thanks for the patch!
On 24/07/2019 13:42, Nikita Pettik wrote:
> Before this patch, resulting type for CASE-WHEN statement was assumed to
> be the same as type of argument of first THEN clause. Obviously, it is
> wrong and could lead to sad consequence (e.g. creating ephemeral table
> with inconsistent format). To deal with this, we check all THEN
> arguments: if all of them have the same type, then such type will be
> resulting of the whole statement; if at least two types are different,
> we can't determine actual resulting type during compilation stage and
> assign SCALAR as a most general type in SQL now.
>
> Need for #4206
> ---
> src/box/sql/expr.c | 27 +++++++++++++++++++++------
> test/sql/types.result | 35 +++++++++++++++++++++++++++++++++++
> test/sql/types.test.lua | 8 ++++++++
> 3 files changed, 64 insertions(+), 6 deletions(-)
>
> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index d7104d8a0..97f5bd180 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -85,18 +85,33 @@ sql_expr_type(struct Expr *pExpr)
> return sql_type_result(rhs_type, lhs_type);
> case TK_CONCAT:
> return FIELD_TYPE_STRING;
> - case TK_CASE:
> - assert(pExpr->x.pList->nExpr >= 2);
> + case TK_CASE: {
> + struct ExprList *cs = pExpr->x.pList;
> + assert(cs->nExpr >= 2);
> /*
> * CASE expression comes at least with one
> * WHEN and one THEN clauses. So, first
> * expression always represents WHEN
> * argument, and the second one - THEN.
> - *
> - * TODO: We must ensure that all THEN clauses
> - * have arguments of the same type.
> + * In case at least one type of THEN argument
> + * is different from others then we can't
> + * determine type of returning value at compiling
> + * stage and set SCALAR (i.e. most general) type.
> */
> - return sql_expr_type(pExpr->x.pList->a[1].pExpr);
> + enum field_type ref_type = sql_expr_type(cs->a[1].pExpr);
> + for (int i = 1; i <= cs->nExpr / 2; i = i * 2) {
> + if (ref_type != sql_expr_type(cs->a[i + 1].pExpr))
> + return FIELD_TYPE_SCALAR;
> + }
Something is wrong with that cycle.
box.execute("SELECT CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4 WHEN 'e' THEN 5 WHEN 'f' THEN 6 END")
---
- metadata:
- name: CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4
WHEN 'e' THEN 5 WHEN 'f' THEN 6 END
type: scalar
rows:
- [1]
...
But it is not scalar, it is integer - all 'then's
here return an integer. Perhaps this is because in
the cycle you do i*=2 instead of i+=2, so you compare
not 1 vs 3, 1 vs 5, 1 vs 7, etc, (all 'then' are by
odd indexes), but 1 vs 2, 1 vs 3, 1 vs 5, 1 vs 9, etc.
More information about the Tarantool-patches
mailing list