[Tarantool-patches] [PATCH v1 1/1] sql: introduce operator []
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sun Nov 21 18:53:54 MSK 2021
Thanks for the patch!
See 8 comments below.
>> @TarantoolBot document
>> Title: Operator [] in SQL
>>
>> Operator [] allows to get elements of MAP, ARRAY and ANY values.
>>
>> Rules for operator []:
>> 1) operator applied to the value to the left of `[` ("left-value");
>> 2) if left-value is not MAP, ARRAY or ANY, an error is thrown;
>> 3) if there is no values between `[` and `]`, left-value is returned as
>> the result;
1. Why isn't it an error? Did I understand correctly that you mean []
with nothing inside?
>> 4) if there is one value between `[` and `]` ("right-value"), the result
>> is:
>> a) if left-value is ANY and its primitive type is not MAP or ARRAY,
>> the result is NULL;
2. Why not error? This looks a bit controversial with how hard the rules
for explicit and implict casts are.
>> b) if the type or primitive type of left-value is ARRAY, and
>> right-value is INTEGER and its value is greater than 0 and not
>> greater than the number of elements in ARRAY, the result will be a
>> value with right-value as the index, otherwise the result will be
>> NULL;
3. What if the right-value is not an integer?
>> c) if the type or primitive type of left-value is MAP and it contains
>> right-value as one of its keys, the result is the value with
>> right-value as the key in left-value, otherwise the result is NULL;
>> 5) if there is more than one value between `[` and `]` than
>> left-value[a, b, c, ...] == left-value[a][b][c]... except it will
>> return NULL, if any of the `[]` operators return NULL.
>>
>> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
>> index 789d8906c..abf47b8ef 100644
>> --- a/src/box/sql/expr.c
>> +++ b/src/box/sql/expr.c
>> @@ -3461,6 +3461,30 @@ expr_code_map(struct Parse *parser, struct Expr *expr, int reg)
>> sqlVdbeAddOp3(vdbe, OP_Map, count, reg, values_reg);
>> }
>>
>> +static void
>> +expr_code_getitem(struct Parse *parser, struct Expr *expr, int reg)
4. The same as for the array/map patches - should be sql_expr_emit_... or
expr_emit - depending on what prefix will you choose to use everywhere.
>> +{
>> + struct Vdbe *vdbe = parser->pVdbe;
>> + struct ExprList *list = expr->x.pList;
>> + assert(list != NULL);
>> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
>> index cfb88bffe..195dfde2b 100644
>> --- a/src/box/sql/mem.c
>> +++ b/src/box/sql/mem.c
>> @@ -3107,6 +3107,65 @@ mem_encode_map(const struct Mem *mems, uint32_t count, uint32_t *size,
>> return map;
>> }
>>
>> +static int
>> +mp_getitem(const char **data, const struct Mem *key)
>> +{
>> + if ((mp_typeof(**data) != MP_ARRAY && mp_typeof(**data) != MP_MAP)) {
5. Double (()). Need only one.
>> + *data = NULL;
>> + return 0;
>> + }
>> + const char *end = *data;
>> + if (mp_typeof(**data) == MP_ARRAY) {
>> + uint32_t size = mp_decode_array(data);
>> + if (!mem_is_uint(key) || key->u.u == 0 || key->u.u > size) {
>> + *data = NULL;
>> + return 0;
>> + }
>> + for (uint32_t i = 0; i < key->u.u - 1; ++i)
>> + mp_next(data);
>> + return 0;
>> + }
>> + struct Mem mem;
>> + mem_create(&mem);
>> + uint32_t size = mp_decode_map(data);
>> + for (uint32_t i = 0; i < size; ++i) {
>> + uint32_t len;
>> + if (mem_from_mp_ephemeral(&mem, *data, &len) != 0)
>> + return -1;
6. Lets add mem_is_trivial() assertion. Since you do not destroy the mem.
>> + *data += len;
>> + assert(!mem_is_map(&mem) && !mem_is_array(&mem));
>> + if (mem_cmp_scalar(&mem, key, NULL) == 0)
>> + return 0;
>> + mp_next(data);
>> + }
>> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
>> index db7fef71a..30922b7f6 100644
>> --- a/src/box/sql/parse.y
>> +++ b/src/box/sql/parse.y
>> @@ -1126,6 +1127,22 @@ expr(A) ::= CAST(X) LP expr(E) AS typedef(T) RP(Y). {
>> sqlExprAttachSubtrees(pParse->db, A.pExpr, E.pExpr, 0);
>> }
>>
>> +expr(A) ::= expr(X) LB exprlist(Y) RB(E). {
>> + struct Expr *expr = sql_expr_new_dequoted(pParse->db, TK_GETITEM, NULL);
7. sql_expr_new_anon().
>> + if (expr == NULL) {
>> + sql_expr_list_delete(pParse->db, Y);
>> + pParse->is_aborted = true;
>> + return;
>> + }
>> + Y = sql_expr_list_append(pParse->db, Y, X.pExpr);
>> + expr->x.pList = Y;
>> + expr->type = FIELD_TYPE_ANY;
>> + sqlExprSetHeightAndFlags(pParse, expr);
>> + A.pExpr = expr;
>> + A.zStart = X.zStart;
>> + A.zEnd = &E.z[E.n];
>> +}
8. I got an assertion fail.
box.cfg{listen = 3313}
s = box.schema.create_space('TEST', {format = {{'ID'}, {'VALUE'}}})
_ = s:create_index('pk')
s:replace{1, {[{key = 100}] = 200}}
box.execute('SELECT value[1] FROM test')
Assertion failed: (!mem_is_map(&mem) && !mem_is_array(&mem)), function mp_getitem, file /Users/gerold/Work/Repositories/tarantool/src/box/sql/mem.c, line 3136.
More information about the Tarantool-patches
mailing list