* [Tarantool-patches] [PATCH v1 1/1] sql: introduce operator []
@ 2021-11-19 14:44 Mergen Imeev via Tarantool-patches
2021-11-19 17:25 ` Konstantin Osipov via Tarantool-patches
0 siblings, 1 reply; 6+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-11-19 14:44 UTC (permalink / raw)
To: v.shpilevoy; +Cc: tarantool-patches
This patch introduces operator [] that allows to get elements from MAP,
ARRAY and ANY values.
Part of #4762
Part of #4763
@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;
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;
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;
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.
Examples:
```
tarantool> box.execute([[SELECT {'a' : 12, 3 : 34}[3];]])
---
- metadata:
- name: COLUMN_1
type: any
rows:
- [34]
...
```
```
tarantool> box.execute([[SELECT [12, [4, 5, 6], 34][2, 2];]])
---
- metadata:
- name: COLUMN_1
type: any
rows:
- [5]
...
```
---
https://github.com/tarantool/tarantool/issues/4762
https://github.com/tarantool/tarantool/issues/4763
https://github.com/tarantool/tarantool/tree/imeevma/gh-4763-syntax-for-map-array
| 1 +
src/box/sql/expr.c | 28 +++++
src/box/sql/mem.c | 59 +++++++++++
src/box/sql/mem.h | 18 ++++
src/box/sql/parse.y | 17 ++++
src/box/sql/vdbe.c | 33 ++++++
test/sql-tap/engine.cfg | 1 +
test/sql-tap/msgpack.test.lua | 186 ++++++++++++++++++++++++++++++++++
8 files changed, 343 insertions(+)
create mode 100755 test/sql-tap/msgpack.test.lua
--git a/extra/addopcodes.sh b/extra/addopcodes.sh
index 3f8cfdf02..51acfe38e 100755
--- a/extra/addopcodes.sh
+++ b/extra/addopcodes.sh
@@ -53,6 +53,7 @@ extras=" \
LINEFEED \
SPACE \
ILLEGAL \
+ GETITEM \
"
IFS=" "
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)
+{
+ struct Vdbe *vdbe = parser->pVdbe;
+ struct ExprList *list = expr->x.pList;
+ assert(list != NULL);
+ int count = list->nExpr;
+ struct Expr *value = list->a[count - 1].pExpr;
+
+ enum field_type type = value->op != TK_NULL ? sql_expr_type(value) :
+ field_type_MAX;
+ if (value->op != TK_VARIABLE && type != FIELD_TYPE_MAP &&
+ type != FIELD_TYPE_ARRAY && type != FIELD_TYPE_ANY) {
+ diag_set(ClientError, ER_SQL_PARSER_GENERIC, "Selecting is "
+ "only possible from any, map and array values");
+ parser->is_aborted = true;
+ return;
+ }
+ int reg_operands = parser->nMem + 1;
+ parser->nMem += count;
+ sqlExprCodeExprList(parser, list, reg_operands, 0, SQL_ECEL_FACTOR);
+ sqlVdbeAddOp3(vdbe, OP_Getitem, count, reg, reg_operands);
+}
+
/*
* Erase column-cache entry number i
*/
@@ -3920,6 +3944,10 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
expr_code_map(pParse, pExpr, target);
return target;
+ case TK_GETITEM:
+ expr_code_getitem(pParse, pExpr, target);
+ return target;
+
case TK_LT:
case TK_LE:
case TK_GT:
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)) {
+ *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;
+ *data += len;
+ assert(!mem_is_map(&mem) && !mem_is_array(&mem));
+ if (mem_cmp_scalar(&mem, key, NULL) == 0)
+ return 0;
+ mp_next(data);
+ }
+ mp_next(&end);
+ if (*data == end)
+ *data = NULL;
+ return 0;
+}
+
+int
+mem_getitem(const struct Mem *mem, const struct Mem *keys, int count,
+ struct Mem *res)
+{
+ assert(count > 0);
+ assert(mem_is_map(mem) || mem_is_array(mem));
+ const char *data = mem->z;
+ for (int i = 0; i < count && data != NULL; ++i) {
+ if (mp_getitem(&data, &keys[i]) != 0)
+ return -1;
+ }
+ if (data == NULL) {
+ mem_set_null(res);
+ return 0;
+ }
+ uint32_t len;
+ if (mem_from_mp(res, data, &len) != 0)
+ return -1;
+ res->flags |= MEM_Any;
+ return 0;
+}
+
/**
* Allocate a sequence of initialized vdbe memory registers
* on region.
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index f442d8673..90eb3b137 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -135,6 +135,19 @@ mem_is_num(const struct Mem *mem)
MEM_TYPE_DEC)) != 0;
}
+static inline bool
+mem_is_any(const struct Mem *mem)
+{
+ return (mem->flags & MEM_Any) != 0;
+}
+
+static inline bool
+mem_is_doc(const struct Mem *mem)
+{
+ return (mem->type & (MEM_TYPE_MAP | MEM_TYPE_ARRAY)) != 0 ||
+ (mem->flags & MEM_Any) != 0;
+}
+
static inline bool
mem_is_metatype(const struct Mem *mem)
{
@@ -884,3 +897,8 @@ mem_encode_array(const struct Mem *mems, uint32_t count, uint32_t *size,
char *
mem_encode_map(const struct Mem *mems, uint32_t count, uint32_t *size,
struct region *region);
+
+/** Return a value from ANY, MAP, or ARRAY MEM using the MEM array as keys. */
+int
+mem_getitem(const struct Mem *mem, const struct Mem *keys, int count,
+ struct Mem *res);
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
@@ -155,6 +155,7 @@ cmdx ::= cmd.
%left CONCAT.
%left COLLATE.
%right BITNOT.
+%left LB.
///////////////////// Begin and end transactions. ////////////////////////////
@@ -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);
+ 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];
+}
+
expr(A) ::= LB(X) exprlist(Y) RB(E). {
struct Expr *expr = sql_expr_new_dequoted(pParse->db, TK_ARRAY, NULL);
if (expr == NULL) {
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 86de3f98a..6b1810508 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1458,6 +1458,39 @@ case OP_Map: {
break;
}
+/**
+ * Opcode: Getitem P1 P2 P3 * *
+ * Synopsis: r[P2] = value[P3@(P1 - 1)]
+ *
+ * Get an element from the value in register P3[P1 - 1] using values in
+ * registers P3, ... P3 + (P1 - 2).
+ */
+case OP_Getitem: {
+ int count = pOp->p1 - 1;
+ struct Mem *value = &aMem[pOp->p3 + count];
+ if (!mem_is_doc(value) ) {
+ diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(value),
+ "any, map or array");
+ goto abort_due_to_error;
+ }
+
+ pOut = &aMem[pOp->p2];
+ if (count == 0) {
+ if (mem_copy(pOut, value) != 0)
+ goto abort_due_to_error;
+ break;
+ }
+ if (!mem_is_map(value) && !mem_is_array(value)) {
+ mem_set_null(pOut);
+ break;
+ }
+
+ struct Mem *keys = &aMem[pOp->p3];
+ if (mem_getitem(value, keys, count, pOut) != 0)
+ goto abort_due_to_error;
+ break;
+}
+
/* Opcode: Eq P1 P2 P3 P4 P5
* Synopsis: IF r[P3]==r[P1]
*
diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg
index 528212ab6..52ce5a1f6 100644
--- a/test/sql-tap/engine.cfg
+++ b/test/sql-tap/engine.cfg
@@ -35,6 +35,7 @@
"built-in-functions.test.lua": {
"memtx": {"engine": "memtx"}
},
+ "msgpack.test.lua": {},
"gh-6157-unnecessary-free-on-string.test.lua": {},
"gh-6376-wrong-double-to-dec-cmp.test.lua": {},
"gh-4077-iproto-execute-no-bind.test.lua": {},
diff --git a/test/sql-tap/msgpack.test.lua b/test/sql-tap/msgpack.test.lua
new file mode 100755
index 000000000..7872426a5
--- /dev/null
+++ b/test/sql-tap/msgpack.test.lua
@@ -0,0 +1,186 @@
+#!/usr/bin/env tarantool
+local test = require("sqltester")
+test:plan(22)
+
+-- Make sure that it is possible to get elements from MAP, ARRAY and ANY.
+test:do_execsql_test(
+ "msgpack-1.1",
+ [[
+ SELECT [123, 234, 356, 467][2];
+ ]], {
+ 234
+ })
+
+test:do_execsql_test(
+ "msgpack-1.2",
+ [[
+ SELECT CAST([123, 234, 356, 467] AS ANY)[3];
+ ]], {
+ 356
+ })
+
+test:do_execsql_test(
+ "msgpack-1.3",
+ [[
+ SELECT CAST([123, 234, 356, 467] AS ANY)['3'];
+ ]], {
+ ""
+ })
+
+test:do_execsql_test(
+ "msgpack-1.4",
+ [[
+ SELECT {'one' : 123, 3 : 'two', '123' : true}[3];
+ ]], {
+ 'two'
+ })
+
+test:do_execsql_test(
+ "msgpack-1.5",
+ [[
+ SELECT {'one' : 123, 3 : 'two', '123' : true}['one'];
+ ]], {
+ 123
+ })
+
+test:do_execsql_test(
+ "msgpack-1.6",
+ [[
+ SELECT {'one' : 123, 3 : 'two', '123' : true}['three'];
+ ]], {
+ ""
+ })
+
+test:do_execsql_test(
+ "msgpack-1.7",
+ [[
+ SELECT CAST({'one' : 123, 3 : 'two', '123' : true} AS ANY)['123'];
+ ]], {
+ true
+ })
+
+test:do_execsql_test(
+ "msgpack-1.8",
+ [[
+ SELECT CAST(1 AS ANY)[2];
+ ]], {
+ ""
+ })
+
+test:do_execsql_test(
+ "msgpack-1.9",
+ [[
+ SELECT CAST('1' AS ANY)['asd'];
+ ]], {
+ ""
+ })
+
+test:do_execsql_test(
+ "msgpack-1.10",
+ [[
+ SELECT CAST('1' AS ANY)[];
+ ]], {
+ '1'
+ })
+
+--
+-- Make sure that operator [] cannot get elements from values of types other
+-- than MAP, ARRAY and ANY.
+--
+test:do_catchsql_test(
+ "msgpack-2.1",
+ [[
+ SELECT 1[];
+ ]], {
+ 1, "Selecting is only possible from any, map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.2",
+ [[
+ SELECT -1[];
+ ]], {
+ 1, "Selecting is only possible from any, map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.3",
+ [[
+ SELECT 1.1[];
+ ]], {
+ 1, "Selecting is only possible from any, map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.4",
+ [[
+ SELECT 1.2e0[];
+ ]], {
+ 1, "Selecting is only possible from any, map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.5",
+ [[
+ SELECT '1'[];
+ ]], {
+ 1, "Selecting is only possible from any, map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.6",
+ [[
+ SELECT x'31'[];
+ ]], {
+ 1, "Selecting is only possible from any, map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.7",
+ [[
+ SELECT uuid()[];
+ ]], {
+ 1, "Selecting is only possible from any, map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.8",
+ [[
+ SELECT true[];
+ ]], {
+ 1, "Selecting is only possible from any, map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.9",
+ [[
+ SELECT CAST(1 AS NUMBER)[];
+ ]], {
+ 1, "Selecting is only possible from any, map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.10",
+ [[
+ SELECT CAST('a' AS STRING)[];
+ ]], {
+ 1, "Selecting is only possible from any, map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.11",
+ [[
+ SELECT NULL[];
+ ]], {
+ 1, "Selecting is only possible from any, map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.12",
+ [[
+ SELECT CAST(NULL AS ANY)[];
+ ]], {
+ 1, "Type mismatch: can not convert NULL to any, map or array"
+ })
+
+test:finish_test()
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] sql: introduce operator []
2021-11-19 14:44 [Tarantool-patches] [PATCH v1 1/1] sql: introduce operator [] Mergen Imeev via Tarantool-patches
@ 2021-11-19 17:25 ` Konstantin Osipov via Tarantool-patches
2021-11-21 15:53 ` Vladislav Shpilevoy via Tarantool-patches
2021-11-24 11:56 ` Mergen Imeev via Tarantool-patches
0 siblings, 2 replies; 6+ messages in thread
From: Konstantin Osipov via Tarantool-patches @ 2021-11-19 17:25 UTC (permalink / raw)
To: imeevma; +Cc: v.shpilevoy, tarantool-patches
* Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org> [21/11/19 17:49]:
> This patch introduces operator [] that allows to get elements from MAP,
> ARRAY and ANY values.
>
> Part of #4762
> Part of #4763
>
Have you benchmarked this? This is going to be a very hot path.
There is already tuple field access in the box by path, why not reuse it?
> @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;
> 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;
> 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;
> 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.
>
> Examples:
> ```
> tarantool> box.execute([[SELECT {'a' : 12, 3 : 34}[3];]])
> ---
> - metadata:
> - name: COLUMN_1
> type: any
> rows:
> - [34]
> ...
> ```
>
> ```
> tarantool> box.execute([[SELECT [12, [4, 5, 6], 34][2, 2];]])
> ---
> - metadata:
> - name: COLUMN_1
> type: any
> rows:
> - [5]
> ...
> ```
> ---
> https://github.com/tarantool/tarantool/issues/4762
> https://github.com/tarantool/tarantool/issues/4763
> https://github.com/tarantool/tarantool/tree/imeevma/gh-4763-syntax-for-map-array
>
> extra/addopcodes.sh | 1 +
> src/box/sql/expr.c | 28 +++++
> src/box/sql/mem.c | 59 +++++++++++
> src/box/sql/mem.h | 18 ++++
> src/box/sql/parse.y | 17 ++++
> src/box/sql/vdbe.c | 33 ++++++
> test/sql-tap/engine.cfg | 1 +
> test/sql-tap/msgpack.test.lua | 186 ++++++++++++++++++++++++++++++++++
> 8 files changed, 343 insertions(+)
> create mode 100755 test/sql-tap/msgpack.test.lua
>
> diff --git a/extra/addopcodes.sh b/extra/addopcodes.sh
> index 3f8cfdf02..51acfe38e 100755
> --- a/extra/addopcodes.sh
> +++ b/extra/addopcodes.sh
> @@ -53,6 +53,7 @@ extras=" \
> LINEFEED \
> SPACE \
> ILLEGAL \
> + GETITEM \
> "
>
> IFS=" "
> 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)
> +{
> + struct Vdbe *vdbe = parser->pVdbe;
> + struct ExprList *list = expr->x.pList;
> + assert(list != NULL);
> + int count = list->nExpr;
> + struct Expr *value = list->a[count - 1].pExpr;
> +
> + enum field_type type = value->op != TK_NULL ? sql_expr_type(value) :
> + field_type_MAX;
> + if (value->op != TK_VARIABLE && type != FIELD_TYPE_MAP &&
> + type != FIELD_TYPE_ARRAY && type != FIELD_TYPE_ANY) {
> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, "Selecting is "
> + "only possible from any, map and array values");
> + parser->is_aborted = true;
> + return;
> + }
> + int reg_operands = parser->nMem + 1;
> + parser->nMem += count;
> + sqlExprCodeExprList(parser, list, reg_operands, 0, SQL_ECEL_FACTOR);
> + sqlVdbeAddOp3(vdbe, OP_Getitem, count, reg, reg_operands);
> +}
> +
> /*
> * Erase column-cache entry number i
> */
> @@ -3920,6 +3944,10 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
> expr_code_map(pParse, pExpr, target);
> return target;
>
> + case TK_GETITEM:
> + expr_code_getitem(pParse, pExpr, target);
> + return target;
> +
> case TK_LT:
> case TK_LE:
> case TK_GT:
> 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)) {
> + *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;
> + *data += len;
> + assert(!mem_is_map(&mem) && !mem_is_array(&mem));
> + if (mem_cmp_scalar(&mem, key, NULL) == 0)
> + return 0;
> + mp_next(data);
> + }
> + mp_next(&end);
> + if (*data == end)
> + *data = NULL;
> + return 0;
> +}
> +
> +int
> +mem_getitem(const struct Mem *mem, const struct Mem *keys, int count,
> + struct Mem *res)
> +{
> + assert(count > 0);
> + assert(mem_is_map(mem) || mem_is_array(mem));
> + const char *data = mem->z;
> + for (int i = 0; i < count && data != NULL; ++i) {
> + if (mp_getitem(&data, &keys[i]) != 0)
> + return -1;
> + }
> + if (data == NULL) {
> + mem_set_null(res);
> + return 0;
> + }
> + uint32_t len;
> + if (mem_from_mp(res, data, &len) != 0)
> + return -1;
> + res->flags |= MEM_Any;
> + return 0;
> +}
> +
> /**
> * Allocate a sequence of initialized vdbe memory registers
> * on region.
> diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
> index f442d8673..90eb3b137 100644
> --- a/src/box/sql/mem.h
> +++ b/src/box/sql/mem.h
> @@ -135,6 +135,19 @@ mem_is_num(const struct Mem *mem)
> MEM_TYPE_DEC)) != 0;
> }
>
> +static inline bool
> +mem_is_any(const struct Mem *mem)
> +{
> + return (mem->flags & MEM_Any) != 0;
> +}
> +
> +static inline bool
> +mem_is_doc(const struct Mem *mem)
> +{
> + return (mem->type & (MEM_TYPE_MAP | MEM_TYPE_ARRAY)) != 0 ||
> + (mem->flags & MEM_Any) != 0;
> +}
> +
> static inline bool
> mem_is_metatype(const struct Mem *mem)
> {
> @@ -884,3 +897,8 @@ mem_encode_array(const struct Mem *mems, uint32_t count, uint32_t *size,
> char *
> mem_encode_map(const struct Mem *mems, uint32_t count, uint32_t *size,
> struct region *region);
> +
> +/** Return a value from ANY, MAP, or ARRAY MEM using the MEM array as keys. */
> +int
> +mem_getitem(const struct Mem *mem, const struct Mem *keys, int count,
> + struct Mem *res);
> 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
> @@ -155,6 +155,7 @@ cmdx ::= cmd.
> %left CONCAT.
> %left COLLATE.
> %right BITNOT.
> +%left LB.
>
>
> ///////////////////// Begin and end transactions. ////////////////////////////
> @@ -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);
> + 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];
> +}
> +
> expr(A) ::= LB(X) exprlist(Y) RB(E). {
> struct Expr *expr = sql_expr_new_dequoted(pParse->db, TK_ARRAY, NULL);
> if (expr == NULL) {
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index 86de3f98a..6b1810508 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -1458,6 +1458,39 @@ case OP_Map: {
> break;
> }
>
> +/**
> + * Opcode: Getitem P1 P2 P3 * *
> + * Synopsis: r[P2] = value[P3@(P1 - 1)]
> + *
> + * Get an element from the value in register P3[P1 - 1] using values in
> + * registers P3, ... P3 + (P1 - 2).
> + */
> +case OP_Getitem: {
> + int count = pOp->p1 - 1;
> + struct Mem *value = &aMem[pOp->p3 + count];
> + if (!mem_is_doc(value) ) {
> + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(value),
> + "any, map or array");
> + goto abort_due_to_error;
> + }
> +
> + pOut = &aMem[pOp->p2];
> + if (count == 0) {
> + if (mem_copy(pOut, value) != 0)
> + goto abort_due_to_error;
> + break;
> + }
> + if (!mem_is_map(value) && !mem_is_array(value)) {
> + mem_set_null(pOut);
> + break;
> + }
> +
> + struct Mem *keys = &aMem[pOp->p3];
> + if (mem_getitem(value, keys, count, pOut) != 0)
> + goto abort_due_to_error;
> + break;
> +}
> +
> /* Opcode: Eq P1 P2 P3 P4 P5
> * Synopsis: IF r[P3]==r[P1]
> *
> diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg
> index 528212ab6..52ce5a1f6 100644
> --- a/test/sql-tap/engine.cfg
> +++ b/test/sql-tap/engine.cfg
> @@ -35,6 +35,7 @@
> "built-in-functions.test.lua": {
> "memtx": {"engine": "memtx"}
> },
> + "msgpack.test.lua": {},
> "gh-6157-unnecessary-free-on-string.test.lua": {},
> "gh-6376-wrong-double-to-dec-cmp.test.lua": {},
> "gh-4077-iproto-execute-no-bind.test.lua": {},
> diff --git a/test/sql-tap/msgpack.test.lua b/test/sql-tap/msgpack.test.lua
> new file mode 100755
> index 000000000..7872426a5
> --- /dev/null
> +++ b/test/sql-tap/msgpack.test.lua
> @@ -0,0 +1,186 @@
> +#!/usr/bin/env tarantool
> +local test = require("sqltester")
> +test:plan(22)
> +
> +-- Make sure that it is possible to get elements from MAP, ARRAY and ANY.
> +test:do_execsql_test(
> + "msgpack-1.1",
> + [[
> + SELECT [123, 234, 356, 467][2];
> + ]], {
> + 234
> + })
> +
> +test:do_execsql_test(
> + "msgpack-1.2",
> + [[
> + SELECT CAST([123, 234, 356, 467] AS ANY)[3];
> + ]], {
> + 356
> + })
> +
> +test:do_execsql_test(
> + "msgpack-1.3",
> + [[
> + SELECT CAST([123, 234, 356, 467] AS ANY)['3'];
> + ]], {
> + ""
> + })
> +
> +test:do_execsql_test(
> + "msgpack-1.4",
> + [[
> + SELECT {'one' : 123, 3 : 'two', '123' : true}[3];
> + ]], {
> + 'two'
> + })
> +
> +test:do_execsql_test(
> + "msgpack-1.5",
> + [[
> + SELECT {'one' : 123, 3 : 'two', '123' : true}['one'];
> + ]], {
> + 123
> + })
> +
> +test:do_execsql_test(
> + "msgpack-1.6",
> + [[
> + SELECT {'one' : 123, 3 : 'two', '123' : true}['three'];
> + ]], {
> + ""
> + })
> +
> +test:do_execsql_test(
> + "msgpack-1.7",
> + [[
> + SELECT CAST({'one' : 123, 3 : 'two', '123' : true} AS ANY)['123'];
> + ]], {
> + true
> + })
> +
> +test:do_execsql_test(
> + "msgpack-1.8",
> + [[
> + SELECT CAST(1 AS ANY)[2];
> + ]], {
> + ""
> + })
> +
> +test:do_execsql_test(
> + "msgpack-1.9",
> + [[
> + SELECT CAST('1' AS ANY)['asd'];
> + ]], {
> + ""
> + })
> +
> +test:do_execsql_test(
> + "msgpack-1.10",
> + [[
> + SELECT CAST('1' AS ANY)[];
> + ]], {
> + '1'
> + })
> +
> +--
> +-- Make sure that operator [] cannot get elements from values of types other
> +-- than MAP, ARRAY and ANY.
> +--
> +test:do_catchsql_test(
> + "msgpack-2.1",
> + [[
> + SELECT 1[];
> + ]], {
> + 1, "Selecting is only possible from any, map and array values"
> + })
> +
> +test:do_catchsql_test(
> + "msgpack-2.2",
> + [[
> + SELECT -1[];
> + ]], {
> + 1, "Selecting is only possible from any, map and array values"
> + })
> +
> +test:do_catchsql_test(
> + "msgpack-2.3",
> + [[
> + SELECT 1.1[];
> + ]], {
> + 1, "Selecting is only possible from any, map and array values"
> + })
> +
> +test:do_catchsql_test(
> + "msgpack-2.4",
> + [[
> + SELECT 1.2e0[];
> + ]], {
> + 1, "Selecting is only possible from any, map and array values"
> + })
> +
> +test:do_catchsql_test(
> + "msgpack-2.5",
> + [[
> + SELECT '1'[];
> + ]], {
> + 1, "Selecting is only possible from any, map and array values"
> + })
> +
> +test:do_catchsql_test(
> + "msgpack-2.6",
> + [[
> + SELECT x'31'[];
> + ]], {
> + 1, "Selecting is only possible from any, map and array values"
> + })
> +
> +test:do_catchsql_test(
> + "msgpack-2.7",
> + [[
> + SELECT uuid()[];
> + ]], {
> + 1, "Selecting is only possible from any, map and array values"
> + })
> +
> +test:do_catchsql_test(
> + "msgpack-2.8",
> + [[
> + SELECT true[];
> + ]], {
> + 1, "Selecting is only possible from any, map and array values"
> + })
> +
> +test:do_catchsql_test(
> + "msgpack-2.9",
> + [[
> + SELECT CAST(1 AS NUMBER)[];
> + ]], {
> + 1, "Selecting is only possible from any, map and array values"
> + })
> +
> +test:do_catchsql_test(
> + "msgpack-2.10",
> + [[
> + SELECT CAST('a' AS STRING)[];
> + ]], {
> + 1, "Selecting is only possible from any, map and array values"
> + })
> +
> +test:do_catchsql_test(
> + "msgpack-2.11",
> + [[
> + SELECT NULL[];
> + ]], {
> + 1, "Selecting is only possible from any, map and array values"
> + })
> +
> +test:do_catchsql_test(
> + "msgpack-2.12",
> + [[
> + SELECT CAST(NULL AS ANY)[];
> + ]], {
> + 1, "Type mismatch: can not convert NULL to any, map or array"
> + })
> +
> +test:finish_test()
> --
> 2.25.1
--
Konstantin Osipov, Moscow, Russia
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] sql: introduce operator []
2021-11-19 17:25 ` Konstantin Osipov via Tarantool-patches
@ 2021-11-21 15:53 ` Vladislav Shpilevoy via Tarantool-patches
2021-11-30 15:20 ` Mergen Imeev via Tarantool-patches
2021-11-24 11:56 ` Mergen Imeev via Tarantool-patches
1 sibling, 1 reply; 6+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-11-21 15:53 UTC (permalink / raw)
To: Konstantin Osipov, imeevma, tarantool-patches
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] sql: introduce operator []
2021-11-19 17:25 ` Konstantin Osipov via Tarantool-patches
2021-11-21 15:53 ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-11-24 11:56 ` Mergen Imeev via Tarantool-patches
1 sibling, 0 replies; 6+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-11-24 11:56 UTC (permalink / raw)
To: Konstantin Osipov; +Cc: v.shpilevoy, tarantool-patches
Hi! Thank you for the review! My answer below.
On Fri, Nov 19, 2021 at 08:25:02PM +0300, Konstantin Osipov wrote:
> * Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org> [21/11/19 17:49]:
> > This patch introduces operator [] that allows to get elements from MAP,
> > ARRAY and ANY values.
> >
> > Part of #4762
> > Part of #4763
> >
>
> Have you benchmarked this? This is going to be a very hot path.
> There is already tuple field access in the box by path, why not reuse it?
>
No, I have no benchmark for this. However, at the moment we plan to
implement the basic version of the operation and speed it up if it is
very much in demand by users. We already have some ideas on how to speed
this up.
>
>
> > @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;
> > 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;
> > 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;
> > 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.
> >
> > Examples:
> > ```
> > tarantool> box.execute([[SELECT {'a' : 12, 3 : 34}[3];]])
> > ---
> > - metadata:
> > - name: COLUMN_1
> > type: any
> > rows:
> > - [34]
> > ...
> > ```
> >
> > ```
> > tarantool> box.execute([[SELECT [12, [4, 5, 6], 34][2, 2];]])
> > ---
> > - metadata:
> > - name: COLUMN_1
> > type: any
> > rows:
> > - [5]
> > ...
> > ```
> > ---
> > https://github.com/tarantool/tarantool/issues/4762
> > https://github.com/tarantool/tarantool/issues/4763
> > https://github.com/tarantool/tarantool/tree/imeevma/gh-4763-syntax-for-map-array
> >
> > extra/addopcodes.sh | 1 +
> > src/box/sql/expr.c | 28 +++++
> > src/box/sql/mem.c | 59 +++++++++++
> > src/box/sql/mem.h | 18 ++++
> > src/box/sql/parse.y | 17 ++++
> > src/box/sql/vdbe.c | 33 ++++++
> > test/sql-tap/engine.cfg | 1 +
> > test/sql-tap/msgpack.test.lua | 186 ++++++++++++++++++++++++++++++++++
> > 8 files changed, 343 insertions(+)
> > create mode 100755 test/sql-tap/msgpack.test.lua
> >
> > diff --git a/extra/addopcodes.sh b/extra/addopcodes.sh
> > index 3f8cfdf02..51acfe38e 100755
> > --- a/extra/addopcodes.sh
> > +++ b/extra/addopcodes.sh
> > @@ -53,6 +53,7 @@ extras=" \
> > LINEFEED \
> > SPACE \
> > ILLEGAL \
> > + GETITEM \
> > "
> >
> > IFS=" "
> > 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)
> > +{
> > + struct Vdbe *vdbe = parser->pVdbe;
> > + struct ExprList *list = expr->x.pList;
> > + assert(list != NULL);
> > + int count = list->nExpr;
> > + struct Expr *value = list->a[count - 1].pExpr;
> > +
> > + enum field_type type = value->op != TK_NULL ? sql_expr_type(value) :
> > + field_type_MAX;
> > + if (value->op != TK_VARIABLE && type != FIELD_TYPE_MAP &&
> > + type != FIELD_TYPE_ARRAY && type != FIELD_TYPE_ANY) {
> > + diag_set(ClientError, ER_SQL_PARSER_GENERIC, "Selecting is "
> > + "only possible from any, map and array values");
> > + parser->is_aborted = true;
> > + return;
> > + }
> > + int reg_operands = parser->nMem + 1;
> > + parser->nMem += count;
> > + sqlExprCodeExprList(parser, list, reg_operands, 0, SQL_ECEL_FACTOR);
> > + sqlVdbeAddOp3(vdbe, OP_Getitem, count, reg, reg_operands);
> > +}
> > +
> > /*
> > * Erase column-cache entry number i
> > */
> > @@ -3920,6 +3944,10 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
> > expr_code_map(pParse, pExpr, target);
> > return target;
> >
> > + case TK_GETITEM:
> > + expr_code_getitem(pParse, pExpr, target);
> > + return target;
> > +
> > case TK_LT:
> > case TK_LE:
> > case TK_GT:
> > 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)) {
> > + *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;
> > + *data += len;
> > + assert(!mem_is_map(&mem) && !mem_is_array(&mem));
> > + if (mem_cmp_scalar(&mem, key, NULL) == 0)
> > + return 0;
> > + mp_next(data);
> > + }
> > + mp_next(&end);
> > + if (*data == end)
> > + *data = NULL;
> > + return 0;
> > +}
> > +
> > +int
> > +mem_getitem(const struct Mem *mem, const struct Mem *keys, int count,
> > + struct Mem *res)
> > +{
> > + assert(count > 0);
> > + assert(mem_is_map(mem) || mem_is_array(mem));
> > + const char *data = mem->z;
> > + for (int i = 0; i < count && data != NULL; ++i) {
> > + if (mp_getitem(&data, &keys[i]) != 0)
> > + return -1;
> > + }
> > + if (data == NULL) {
> > + mem_set_null(res);
> > + return 0;
> > + }
> > + uint32_t len;
> > + if (mem_from_mp(res, data, &len) != 0)
> > + return -1;
> > + res->flags |= MEM_Any;
> > + return 0;
> > +}
> > +
> > /**
> > * Allocate a sequence of initialized vdbe memory registers
> > * on region.
> > diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
> > index f442d8673..90eb3b137 100644
> > --- a/src/box/sql/mem.h
> > +++ b/src/box/sql/mem.h
> > @@ -135,6 +135,19 @@ mem_is_num(const struct Mem *mem)
> > MEM_TYPE_DEC)) != 0;
> > }
> >
> > +static inline bool
> > +mem_is_any(const struct Mem *mem)
> > +{
> > + return (mem->flags & MEM_Any) != 0;
> > +}
> > +
> > +static inline bool
> > +mem_is_doc(const struct Mem *mem)
> > +{
> > + return (mem->type & (MEM_TYPE_MAP | MEM_TYPE_ARRAY)) != 0 ||
> > + (mem->flags & MEM_Any) != 0;
> > +}
> > +
> > static inline bool
> > mem_is_metatype(const struct Mem *mem)
> > {
> > @@ -884,3 +897,8 @@ mem_encode_array(const struct Mem *mems, uint32_t count, uint32_t *size,
> > char *
> > mem_encode_map(const struct Mem *mems, uint32_t count, uint32_t *size,
> > struct region *region);
> > +
> > +/** Return a value from ANY, MAP, or ARRAY MEM using the MEM array as keys. */
> > +int
> > +mem_getitem(const struct Mem *mem, const struct Mem *keys, int count,
> > + struct Mem *res);
> > 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
> > @@ -155,6 +155,7 @@ cmdx ::= cmd.
> > %left CONCAT.
> > %left COLLATE.
> > %right BITNOT.
> > +%left LB.
> >
> >
> > ///////////////////// Begin and end transactions. ////////////////////////////
> > @@ -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);
> > + 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];
> > +}
> > +
> > expr(A) ::= LB(X) exprlist(Y) RB(E). {
> > struct Expr *expr = sql_expr_new_dequoted(pParse->db, TK_ARRAY, NULL);
> > if (expr == NULL) {
> > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> > index 86de3f98a..6b1810508 100644
> > --- a/src/box/sql/vdbe.c
> > +++ b/src/box/sql/vdbe.c
> > @@ -1458,6 +1458,39 @@ case OP_Map: {
> > break;
> > }
> >
> > +/**
> > + * Opcode: Getitem P1 P2 P3 * *
> > + * Synopsis: r[P2] = value[P3@(P1 - 1)]
> > + *
> > + * Get an element from the value in register P3[P1 - 1] using values in
> > + * registers P3, ... P3 + (P1 - 2).
> > + */
> > +case OP_Getitem: {
> > + int count = pOp->p1 - 1;
> > + struct Mem *value = &aMem[pOp->p3 + count];
> > + if (!mem_is_doc(value) ) {
> > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(value),
> > + "any, map or array");
> > + goto abort_due_to_error;
> > + }
> > +
> > + pOut = &aMem[pOp->p2];
> > + if (count == 0) {
> > + if (mem_copy(pOut, value) != 0)
> > + goto abort_due_to_error;
> > + break;
> > + }
> > + if (!mem_is_map(value) && !mem_is_array(value)) {
> > + mem_set_null(pOut);
> > + break;
> > + }
> > +
> > + struct Mem *keys = &aMem[pOp->p3];
> > + if (mem_getitem(value, keys, count, pOut) != 0)
> > + goto abort_due_to_error;
> > + break;
> > +}
> > +
> > /* Opcode: Eq P1 P2 P3 P4 P5
> > * Synopsis: IF r[P3]==r[P1]
> > *
> > diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg
> > index 528212ab6..52ce5a1f6 100644
> > --- a/test/sql-tap/engine.cfg
> > +++ b/test/sql-tap/engine.cfg
> > @@ -35,6 +35,7 @@
> > "built-in-functions.test.lua": {
> > "memtx": {"engine": "memtx"}
> > },
> > + "msgpack.test.lua": {},
> > "gh-6157-unnecessary-free-on-string.test.lua": {},
> > "gh-6376-wrong-double-to-dec-cmp.test.lua": {},
> > "gh-4077-iproto-execute-no-bind.test.lua": {},
> > diff --git a/test/sql-tap/msgpack.test.lua b/test/sql-tap/msgpack.test.lua
> > new file mode 100755
> > index 000000000..7872426a5
> > --- /dev/null
> > +++ b/test/sql-tap/msgpack.test.lua
> > @@ -0,0 +1,186 @@
> > +#!/usr/bin/env tarantool
> > +local test = require("sqltester")
> > +test:plan(22)
> > +
> > +-- Make sure that it is possible to get elements from MAP, ARRAY and ANY.
> > +test:do_execsql_test(
> > + "msgpack-1.1",
> > + [[
> > + SELECT [123, 234, 356, 467][2];
> > + ]], {
> > + 234
> > + })
> > +
> > +test:do_execsql_test(
> > + "msgpack-1.2",
> > + [[
> > + SELECT CAST([123, 234, 356, 467] AS ANY)[3];
> > + ]], {
> > + 356
> > + })
> > +
> > +test:do_execsql_test(
> > + "msgpack-1.3",
> > + [[
> > + SELECT CAST([123, 234, 356, 467] AS ANY)['3'];
> > + ]], {
> > + ""
> > + })
> > +
> > +test:do_execsql_test(
> > + "msgpack-1.4",
> > + [[
> > + SELECT {'one' : 123, 3 : 'two', '123' : true}[3];
> > + ]], {
> > + 'two'
> > + })
> > +
> > +test:do_execsql_test(
> > + "msgpack-1.5",
> > + [[
> > + SELECT {'one' : 123, 3 : 'two', '123' : true}['one'];
> > + ]], {
> > + 123
> > + })
> > +
> > +test:do_execsql_test(
> > + "msgpack-1.6",
> > + [[
> > + SELECT {'one' : 123, 3 : 'two', '123' : true}['three'];
> > + ]], {
> > + ""
> > + })
> > +
> > +test:do_execsql_test(
> > + "msgpack-1.7",
> > + [[
> > + SELECT CAST({'one' : 123, 3 : 'two', '123' : true} AS ANY)['123'];
> > + ]], {
> > + true
> > + })
> > +
> > +test:do_execsql_test(
> > + "msgpack-1.8",
> > + [[
> > + SELECT CAST(1 AS ANY)[2];
> > + ]], {
> > + ""
> > + })
> > +
> > +test:do_execsql_test(
> > + "msgpack-1.9",
> > + [[
> > + SELECT CAST('1' AS ANY)['asd'];
> > + ]], {
> > + ""
> > + })
> > +
> > +test:do_execsql_test(
> > + "msgpack-1.10",
> > + [[
> > + SELECT CAST('1' AS ANY)[];
> > + ]], {
> > + '1'
> > + })
> > +
> > +--
> > +-- Make sure that operator [] cannot get elements from values of types other
> > +-- than MAP, ARRAY and ANY.
> > +--
> > +test:do_catchsql_test(
> > + "msgpack-2.1",
> > + [[
> > + SELECT 1[];
> > + ]], {
> > + 1, "Selecting is only possible from any, map and array values"
> > + })
> > +
> > +test:do_catchsql_test(
> > + "msgpack-2.2",
> > + [[
> > + SELECT -1[];
> > + ]], {
> > + 1, "Selecting is only possible from any, map and array values"
> > + })
> > +
> > +test:do_catchsql_test(
> > + "msgpack-2.3",
> > + [[
> > + SELECT 1.1[];
> > + ]], {
> > + 1, "Selecting is only possible from any, map and array values"
> > + })
> > +
> > +test:do_catchsql_test(
> > + "msgpack-2.4",
> > + [[
> > + SELECT 1.2e0[];
> > + ]], {
> > + 1, "Selecting is only possible from any, map and array values"
> > + })
> > +
> > +test:do_catchsql_test(
> > + "msgpack-2.5",
> > + [[
> > + SELECT '1'[];
> > + ]], {
> > + 1, "Selecting is only possible from any, map and array values"
> > + })
> > +
> > +test:do_catchsql_test(
> > + "msgpack-2.6",
> > + [[
> > + SELECT x'31'[];
> > + ]], {
> > + 1, "Selecting is only possible from any, map and array values"
> > + })
> > +
> > +test:do_catchsql_test(
> > + "msgpack-2.7",
> > + [[
> > + SELECT uuid()[];
> > + ]], {
> > + 1, "Selecting is only possible from any, map and array values"
> > + })
> > +
> > +test:do_catchsql_test(
> > + "msgpack-2.8",
> > + [[
> > + SELECT true[];
> > + ]], {
> > + 1, "Selecting is only possible from any, map and array values"
> > + })
> > +
> > +test:do_catchsql_test(
> > + "msgpack-2.9",
> > + [[
> > + SELECT CAST(1 AS NUMBER)[];
> > + ]], {
> > + 1, "Selecting is only possible from any, map and array values"
> > + })
> > +
> > +test:do_catchsql_test(
> > + "msgpack-2.10",
> > + [[
> > + SELECT CAST('a' AS STRING)[];
> > + ]], {
> > + 1, "Selecting is only possible from any, map and array values"
> > + })
> > +
> > +test:do_catchsql_test(
> > + "msgpack-2.11",
> > + [[
> > + SELECT NULL[];
> > + ]], {
> > + 1, "Selecting is only possible from any, map and array values"
> > + })
> > +
> > +test:do_catchsql_test(
> > + "msgpack-2.12",
> > + [[
> > + SELECT CAST(NULL AS ANY)[];
> > + ]], {
> > + 1, "Type mismatch: can not convert NULL to any, map or array"
> > + })
> > +
> > +test:finish_test()
> > --
> > 2.25.1
>
> --
> Konstantin Osipov, Moscow, Russia
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] sql: introduce operator []
2021-11-21 15:53 ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-11-30 15:20 ` Mergen Imeev via Tarantool-patches
2021-12-02 23:43 ` Vladislav Shpilevoy via Tarantool-patches
0 siblings, 1 reply; 6+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-11-30 15:20 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
Hi! Thank you for the review! I reworked the patch a bit. Also, after a
discussion we decided to change syntax and behavior of the operation.
My answers, diff and new patch below.
On Sun, Nov 21, 2021 at 04:53:54PM +0100, Vladislav Shpilevoy wrote:
> 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?
>
You understood correctly. I thought it was logical, but for now I have dropped
everything about the variable number of elements between the brackets.
> >> 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.
>
Since there is no good way to catch an error, we try to avoid them as much as
possible.
> >> 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?
>
The result will be NULL.
> >> 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.
>
I still think code_*() or expr_code_*() should be used here as these functions
are static functions.
> >> +{
> >> + 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.
>
Fixed.
> >> + *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.
>
Thanks, added.
> >> + *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().
>
Fixed.
> >> + 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.
I replaced assert with error. I did not write about this either in the rules or
in the commit message, since I am not sure if it is correct. To find a key, I
create a MEM from MP, so looking for MAP or ARRAY key is inconvenient. We can
try to go the other way - from MEM to MP. In this case, we can use memcmp() to
look up the ARRAY and MAP keys, although this would be a fairly shallow
comparison.
Diff:
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 9b15e85be..8a3bab8a4 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -3455,7 +3455,7 @@ expr_code_map(struct Parse *parser, struct Expr *expr, int reg)
enum field_type type = sql_expr_type(expr);
if (expr->op != TK_VARIABLE && type != FIELD_TYPE_INTEGER &&
type != FIELD_TYPE_UNSIGNED && type != FIELD_TYPE_STRING &&
- type != FIELD_TYPE_UUID) {
+ type != FIELD_TYPE_UUID && expr->op != TK_GETITEM) {
diag_set(ClientError, ER_SQL_PARSER_GENERIC, "Only "
"integer, string and uuid can be keys in map");
parser->is_aborted = true;
@@ -3479,13 +3479,24 @@ expr_code_getitem(struct Parse *parser, struct Expr *expr, int reg)
enum field_type type = value->op != TK_NULL ? sql_expr_type(value) :
field_type_MAX;
- if (value->op != TK_VARIABLE && type != FIELD_TYPE_MAP &&
- type != FIELD_TYPE_ARRAY && type != FIELD_TYPE_ANY) {
+ if (value->op != TK_VARIABLE && value->op != TK_GETITEM &&
+ type != FIELD_TYPE_MAP && type != FIELD_TYPE_ARRAY) {
diag_set(ClientError, ER_SQL_PARSER_GENERIC, "Selecting is "
- "only possible from any, map and array values");
+ "only possible from map and array values");
parser->is_aborted = true;
return;
}
+ for (int i = 0; i < count - 1; ++i) {
+ struct Expr *arg = list->a[i].pExpr;
+ enum field_type type = arg->op != TK_NULL ? sql_expr_type(arg) :
+ field_type_MAX;
+ if (type == FIELD_TYPE_MAP || type == FIELD_TYPE_ARRAY) {
+ diag_set(ClientError, ER_SQL_PARSER_GENERIC, "Map and "
+ "array values cannot be keys");
+ parser->is_aborted = true;
+ return;
+ }
+ }
int reg_operands = parser->nMem + 1;
parser->nMem += count;
sqlExprCodeExprList(parser, list, reg_operands, 0, SQL_ECEL_FACTOR);
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 146fd3ffc..f52835982 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1956,7 +1956,7 @@ is_upcast(int op, enum field_type a, enum field_type b)
static inline bool
is_castable(int op, enum field_type a, enum field_type b)
{
- return is_upcast(op, a, b) || op == TK_VARIABLE ||
+ return is_upcast(op, a, b) || op == TK_VARIABLE || op == TK_GETITEM ||
(sql_type_is_numeric(a) && sql_type_is_numeric(b)) ||
b == FIELD_TYPE_ANY;
}
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 042d069b4..32b0c3fb9 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -3114,7 +3114,7 @@ mem_encode_map(const struct Mem *mems, uint32_t count, uint32_t *size,
static int
mp_getitem(const char **data, const struct Mem *key)
{
- if ((mp_typeof(**data) != MP_ARRAY && mp_typeof(**data) != MP_MAP)) {
+ if (mp_typeof(**data) != MP_ARRAY && mp_typeof(**data) != MP_MAP) {
*data = NULL;
return 0;
}
@@ -3136,8 +3136,12 @@ mp_getitem(const char **data, const struct Mem *key)
uint32_t len;
if (mem_from_mp_ephemeral(&mem, *data, &len) != 0)
return -1;
+ assert(mem_is_trivial(&mem) && !mem_is_metatype(&mem));
*data += len;
- assert(!mem_is_map(&mem) && !mem_is_array(&mem));
+ if (mem_is_map(&mem) || mem_is_array(&mem)) {
+ *data = NULL;
+ return 0;
+ }
if (mem_cmp_scalar(&mem, key, NULL) == 0)
return 0;
mp_next(data);
@@ -3166,7 +3170,6 @@ mem_getitem(const struct Mem *mem, const struct Mem *keys, int count,
uint32_t len;
if (mem_from_mp(res, data, &len) != 0)
return -1;
- res->flags |= MEM_Any;
return 0;
}
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index 2d3512f3b..2fbdea3ec 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -142,10 +142,9 @@ mem_is_any(const struct Mem *mem)
}
static inline bool
-mem_is_doc(const struct Mem *mem)
+mem_is_msgpack(const struct Mem *mem)
{
- return (mem->type & (MEM_TYPE_MAP | MEM_TYPE_ARRAY)) != 0 ||
- (mem->flags & MEM_Any) != 0;
+ return (mem->type & (MEM_TYPE_MAP | MEM_TYPE_ARRAY)) != 0;
}
static inline bool
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 805771c69..d49920133 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -155,7 +155,7 @@ cmdx ::= cmd.
%left CONCAT.
%left COLLATE.
%right BITNOT.
-%left LB.
+%right LB.
///////////////////// Begin and end transactions. ////////////////////////////
@@ -1100,8 +1100,8 @@ 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);
+expr(A) ::= expr(X) LB getlist(Y) RB(E). {
+ struct Expr *expr = sql_expr_new_anon(pParse->db, TK_GETITEM);
if (expr == NULL) {
sql_expr_list_delete(pParse->db, Y);
pParse->is_aborted = true;
@@ -1116,6 +1116,16 @@ expr(A) ::= expr(X) LB exprlist(Y) RB(E). {
A.zEnd = &E.z[E.n];
}
+getlist(A) ::= getlist(A) RB LB expr(X). {
+ A = sql_expr_list_append(pParse->db, A, X.pExpr);
+}
+getlist(A) ::= expr(X). {
+ A = sql_expr_list_append(pParse->db, NULL, X.pExpr);
+}
+
+%type getlist {ExprList *}
+%destructor getlist {sql_expr_list_delete(pParse->db, $$);}
+
expr(A) ::= LB(X) exprlist(Y) RB(E). {
struct Expr *expr = sql_expr_new_anon(pParse->db, TK_ARRAY);
if (expr == NULL) {
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 6b1810508..ad8f2b3e9 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1467,24 +1467,20 @@ case OP_Map: {
*/
case OP_Getitem: {
int count = pOp->p1 - 1;
+ assert(count > 0);
struct Mem *value = &aMem[pOp->p3 + count];
- if (!mem_is_doc(value) ) {
+ if (mem_is_null(value)) {
+ diag_set(ClientError, ER_SQL_EXECUTE, "Selecting is not "
+ "possible from NULL");
+ goto abort_due_to_error;
+ }
+ if (mem_is_any(value) || !mem_is_msgpack(value)) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(value),
- "any, map or array");
+ "map or array");
goto abort_due_to_error;
}
pOut = &aMem[pOp->p2];
- if (count == 0) {
- if (mem_copy(pOut, value) != 0)
- goto abort_due_to_error;
- break;
- }
- if (!mem_is_map(value) && !mem_is_array(value)) {
- mem_set_null(pOut);
- break;
- }
-
struct Mem *keys = &aMem[pOp->p3];
if (mem_getitem(value, keys, count, pOut) != 0)
goto abort_due_to_error;
diff --git a/test/sql-tap/msgpack.test.lua b/test/sql-tap/msgpack.test.lua
index 7872426a5..a67da2ead 100755
--- a/test/sql-tap/msgpack.test.lua
+++ b/test/sql-tap/msgpack.test.lua
@@ -1,8 +1,8 @@
#!/usr/bin/env tarantool
local test = require("sqltester")
-test:plan(22)
+test:plan(23)
--- Make sure that it is possible to get elements from MAP, ARRAY and ANY.
+-- Make sure that it is possible to get elements from MAP и ARRAY.
test:do_execsql_test(
"msgpack-1.1",
[[
@@ -14,173 +14,185 @@ test:do_execsql_test(
test:do_execsql_test(
"msgpack-1.2",
[[
- SELECT CAST([123, 234, 356, 467] AS ANY)[3];
+ SELECT {'one' : 123, 3 : 'two', '123' : true}[3];
]], {
- 356
+ 'two'
})
test:do_execsql_test(
"msgpack-1.3",
[[
- SELECT CAST([123, 234, 356, 467] AS ANY)['3'];
+ SELECT {'one' : [11, 22, 33], 3 : 'two', '123' : true}['one'][2];
]], {
- ""
+ 22
})
test:do_execsql_test(
"msgpack-1.4",
[[
- SELECT {'one' : 123, 3 : 'two', '123' : true}[3];
+ SELECT {'one' : 123, 3 : 'two', '123' : true}['three'];
]], {
- 'two'
+ ""
})
-test:do_execsql_test(
- "msgpack-1.5",
+--
+-- Make sure that operator [] cannot get elements from values of types other
+-- than MAP and ARRAY.
+--
+test:do_catchsql_test(
+ "msgpack-2.1",
[[
- SELECT {'one' : 123, 3 : 'two', '123' : true}['one'];
+ SELECT 1[1];
]], {
- 123
+ 1, "Selecting is only possible from map and array values"
})
-test:do_execsql_test(
- "msgpack-1.6",
+test:do_catchsql_test(
+ "msgpack-2.2",
[[
- SELECT {'one' : 123, 3 : 'two', '123' : true}['three'];
+ SELECT -1[1];
]], {
- ""
+ 1, "Selecting is only possible from map and array values"
})
-test:do_execsql_test(
- "msgpack-1.7",
+test:do_catchsql_test(
+ "msgpack-2.3",
[[
- SELECT CAST({'one' : 123, 3 : 'two', '123' : true} AS ANY)['123'];
+ SELECT 1.1[1];
]], {
- true
+ 1, "Selecting is only possible from map and array values"
})
-test:do_execsql_test(
- "msgpack-1.8",
+test:do_catchsql_test(
+ "msgpack-2.4",
[[
- SELECT CAST(1 AS ANY)[2];
+ SELECT 1.2e0[1];
]], {
- ""
+ 1, "Selecting is only possible from map and array values"
})
-test:do_execsql_test(
- "msgpack-1.9",
+test:do_catchsql_test(
+ "msgpack-2.5",
[[
- SELECT CAST('1' AS ANY)['asd'];
+ SELECT '1'[1];
]], {
- ""
+ 1, "Selecting is only possible from map and array values"
})
-test:do_execsql_test(
- "msgpack-1.10",
+test:do_catchsql_test(
+ "msgpack-2.6",
[[
- SELECT CAST('1' AS ANY)[];
+ SELECT x'31'[1];
]], {
- '1'
+ 1, "Selecting is only possible from map and array values"
})
---
--- Make sure that operator [] cannot get elements from values of types other
--- than MAP, ARRAY and ANY.
---
test:do_catchsql_test(
- "msgpack-2.1",
+ "msgpack-2.7",
[[
- SELECT 1[];
+ SELECT uuid()[1];
]], {
- 1, "Selecting is only possible from any, map and array values"
+ 1, "Selecting is only possible from map and array values"
})
test:do_catchsql_test(
- "msgpack-2.2",
+ "msgpack-2.8",
[[
- SELECT -1[];
+ SELECT true[1];
]], {
- 1, "Selecting is only possible from any, map and array values"
+ 1, "Selecting is only possible from map and array values"
})
test:do_catchsql_test(
- "msgpack-2.3",
+ "msgpack-2.9",
[[
- SELECT 1.1[];
+ SELECT CAST(1 AS NUMBER)[1];
]], {
- 1, "Selecting is only possible from any, map and array values"
+ 1, "Selecting is only possible from map and array values"
})
test:do_catchsql_test(
- "msgpack-2.4",
+ "msgpack-2.10",
[[
- SELECT 1.2e0[];
+ SELECT CAST('a' AS STRING)[1];
]], {
- 1, "Selecting is only possible from any, map and array values"
+ 1, "Selecting is only possible from map and array values"
})
test:do_catchsql_test(
- "msgpack-2.5",
+ "msgpack-2.11",
[[
- SELECT '1'[];
+ SELECT NULL[1];
]], {
- 1, "Selecting is only possible from any, map and array values"
+ 1, "Selecting is only possible from map and array values"
})
test:do_catchsql_test(
- "msgpack-2.6",
+ "msgpack-2.12",
[[
- SELECT x'31'[];
+ SELECT CAST(NULL AS ANY)[1];
]], {
- 1, "Selecting is only possible from any, map and array values"
+ 1, "Selecting is only possible from map and array values"
})
-test:do_catchsql_test(
- "msgpack-2.7",
+-- Make sure that the second and the following brackets do not throw type error.
+test:do_execsql_test(
+ "msgpack-3.1",
+ [[
+ SELECT [1, 2, 3][1][2];
+ ]], {
+ ""
+ })
+
+test:do_execsql_test(
+ "msgpack-3.2",
[[
- SELECT uuid()[];
+ SELECT [1, 2, 3][1][2][3][4][5][6][7];
]], {
- 1, "Selecting is only possible from any, map and array values"
+ ""
})
test:do_catchsql_test(
- "msgpack-2.8",
+ "msgpack-3.3",
[[
- SELECT true[];
+ SELECT ([1, 2, 3][1])[2];
]], {
- 1, "Selecting is only possible from any, map and array values"
+ 1, "Type mismatch: can not convert integer(1) to map or array"
})
-test:do_catchsql_test(
- "msgpack-2.9",
+-- Make sure that type of result of [] is checked in runtime.
+test:do_execsql_test(
+ "msgpack-4.1",
[[
- SELECT CAST(1 AS NUMBER)[];
+ SELECT [123, 23, 1][2] + 7;
]], {
- 1, "Selecting is only possible from any, map and array values"
+ 30
})
test:do_catchsql_test(
- "msgpack-2.10",
+ "msgpack-4.2",
[[
- SELECT CAST('a' AS STRING)[];
+ SELECT ['a', 2, true][1] + 1;
]], {
- 1, "Selecting is only possible from any, map and array values"
+ 1, "Type mismatch: can not convert string('a') to integer, decimal "..
+ "or double"
})
-test:do_catchsql_test(
- "msgpack-2.11",
+test:do_execsql_test(
+ "msgpack-4.3",
[[
- SELECT NULL[];
+ SELECT {[11, 22, 33][3] : 'asd'};
]], {
- 1, "Selecting is only possible from any, map and array values"
+ {[33] = 'asd'}
})
-test:do_catchsql_test(
- "msgpack-2.12",
+-- Built-in functions in this case will use default types of arguments.
+test:do_execsql_test(
+ "msgpack-4.4",
[[
- SELECT CAST(NULL AS ANY)[];
+ SELECT typeof(ABS([123, 23, 1][1]));
]], {
- 1, "Type mismatch: can not convert NULL to any, map or array"
+ "decimal"
})
test:finish_test()
New patch:
commit 5442c79ab86f469d2c2997416b765167c619c5fc
Author: Mergen Imeev <imeevma@gmail.com>
Date: Wed Nov 17 10:21:41 2021 +0300
sql: introduce operator []
This patch introduces operator [] that allows to get elements from MAP
and ARRAY values.
Closes #4762
Closes #4763
Part of #6251
@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 expression to the left of `[` ("container");
2) between `[` and `]` should be one expression ("key");
3) if container is NULL or not MAP or ARRAY:
a) If there is only one such operator, or if there is a sequence of []
operators, and this is the first of them, an error is thrown;
b) otherwise the result is NULL;
4) if container is ARRAY, then:
a) if key is INTEGER and its value is greater than 0 and not more than
the number of elements in the container, the result will be the
value in the container with key as the index;
b) otherwise the result will be NULL;
5) if container is MAP, then:
a) if container contains a key as one of its keys, the result is the
value corresponding to this key;
b) otherwise the result will be NULL;
Examples:
```
tarantool> box.execute([[SELECT {'a' : 12, 3 : 34}[3];]])
---
- metadata:
- name: COLUMN_1
type: any
rows:
- [34]
...
```
```
tarantool> box.execute([[SELECT [12, [4, 5, 6], 34][2][2];]])
---
- metadata:
- name: COLUMN_1
type: any
rows:
- [5]
...
```
diff --git a/extra/addopcodes.sh b/extra/addopcodes.sh
index 3f8cfdf02..51acfe38e 100755
--- a/extra/addopcodes.sh
+++ b/extra/addopcodes.sh
@@ -53,6 +53,7 @@ extras=" \
LINEFEED \
SPACE \
ILLEGAL \
+ GETITEM \
"
IFS=" "
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index e4c1dcff1..8a3bab8a4 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -3455,7 +3455,7 @@ expr_code_map(struct Parse *parser, struct Expr *expr, int reg)
enum field_type type = sql_expr_type(expr);
if (expr->op != TK_VARIABLE && type != FIELD_TYPE_INTEGER &&
type != FIELD_TYPE_UNSIGNED && type != FIELD_TYPE_STRING &&
- type != FIELD_TYPE_UUID) {
+ type != FIELD_TYPE_UUID && expr->op != TK_GETITEM) {
diag_set(ClientError, ER_SQL_PARSER_GENERIC, "Only "
"integer, string and uuid can be keys in map");
parser->is_aborted = true;
@@ -3468,6 +3468,41 @@ 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)
+{
+ struct Vdbe *vdbe = parser->pVdbe;
+ struct ExprList *list = expr->x.pList;
+ assert(list != NULL);
+ int count = list->nExpr;
+ struct Expr *value = list->a[count - 1].pExpr;
+
+ enum field_type type = value->op != TK_NULL ? sql_expr_type(value) :
+ field_type_MAX;
+ if (value->op != TK_VARIABLE && value->op != TK_GETITEM &&
+ type != FIELD_TYPE_MAP && type != FIELD_TYPE_ARRAY) {
+ diag_set(ClientError, ER_SQL_PARSER_GENERIC, "Selecting is "
+ "only possible from map and array values");
+ parser->is_aborted = true;
+ return;
+ }
+ for (int i = 0; i < count - 1; ++i) {
+ struct Expr *arg = list->a[i].pExpr;
+ enum field_type type = arg->op != TK_NULL ? sql_expr_type(arg) :
+ field_type_MAX;
+ if (type == FIELD_TYPE_MAP || type == FIELD_TYPE_ARRAY) {
+ diag_set(ClientError, ER_SQL_PARSER_GENERIC, "Map and "
+ "array values cannot be keys");
+ parser->is_aborted = true;
+ return;
+ }
+ }
+ int reg_operands = parser->nMem + 1;
+ parser->nMem += count;
+ sqlExprCodeExprList(parser, list, reg_operands, 0, SQL_ECEL_FACTOR);
+ sqlVdbeAddOp3(vdbe, OP_Getitem, count, reg, reg_operands);
+}
+
/*
* Erase column-cache entry number i
*/
@@ -3927,6 +3962,10 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
expr_code_map(pParse, pExpr, target);
return target;
+ case TK_GETITEM:
+ expr_code_getitem(pParse, pExpr, target);
+ return target;
+
case TK_LT:
case TK_LE:
case TK_GT:
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 146fd3ffc..f52835982 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1956,7 +1956,7 @@ is_upcast(int op, enum field_type a, enum field_type b)
static inline bool
is_castable(int op, enum field_type a, enum field_type b)
{
- return is_upcast(op, a, b) || op == TK_VARIABLE ||
+ return is_upcast(op, a, b) || op == TK_VARIABLE || op == TK_GETITEM ||
(sql_type_is_numeric(a) && sql_type_is_numeric(b)) ||
b == FIELD_TYPE_ANY;
}
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 7411b8f67..32b0c3fb9 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -3111,6 +3111,68 @@ 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) {
+ *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;
+ assert(mem_is_trivial(&mem) && !mem_is_metatype(&mem));
+ *data += len;
+ if (mem_is_map(&mem) || mem_is_array(&mem)) {
+ *data = NULL;
+ return 0;
+ }
+ if (mem_cmp_scalar(&mem, key, NULL) == 0)
+ return 0;
+ mp_next(data);
+ }
+ mp_next(&end);
+ if (*data == end)
+ *data = NULL;
+ return 0;
+}
+
+int
+mem_getitem(const struct Mem *mem, const struct Mem *keys, int count,
+ struct Mem *res)
+{
+ assert(count > 0);
+ assert(mem_is_map(mem) || mem_is_array(mem));
+ const char *data = mem->z;
+ for (int i = 0; i < count && data != NULL; ++i) {
+ if (mp_getitem(&data, &keys[i]) != 0)
+ return -1;
+ }
+ if (data == NULL) {
+ mem_set_null(res);
+ return 0;
+ }
+ uint32_t len;
+ if (mem_from_mp(res, data, &len) != 0)
+ return -1;
+ return 0;
+}
+
/**
* Allocate a sequence of initialized vdbe memory registers
* on region.
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index 7e35123ca..2fbdea3ec 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -135,6 +135,18 @@ mem_is_num(const struct Mem *mem)
MEM_TYPE_DEC)) != 0;
}
+static inline bool
+mem_is_any(const struct Mem *mem)
+{
+ return (mem->flags & MEM_Any) != 0;
+}
+
+static inline bool
+mem_is_msgpack(const struct Mem *mem)
+{
+ return (mem->type & (MEM_TYPE_MAP | MEM_TYPE_ARRAY)) != 0;
+}
+
static inline bool
mem_is_metatype(const struct Mem *mem)
{
@@ -890,3 +902,8 @@ mem_encode_array(const struct Mem *mems, uint32_t count, uint32_t *size,
char *
mem_encode_map(const struct Mem *mems, uint32_t count, uint32_t *size,
struct region *region);
+
+/** Return a value from ANY, MAP, or ARRAY MEM using the MEM array as keys. */
+int
+mem_getitem(const struct Mem *mem, const struct Mem *keys, int count,
+ struct Mem *res);
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 998acadea..d49920133 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -155,6 +155,7 @@ cmdx ::= cmd.
%left CONCAT.
%left COLLATE.
%right BITNOT.
+%right LB.
///////////////////// Begin and end transactions. ////////////////////////////
@@ -1099,6 +1100,32 @@ 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 getlist(Y) RB(E). {
+ struct Expr *expr = sql_expr_new_anon(pParse->db, TK_GETITEM);
+ 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];
+}
+
+getlist(A) ::= getlist(A) RB LB expr(X). {
+ A = sql_expr_list_append(pParse->db, A, X.pExpr);
+}
+getlist(A) ::= expr(X). {
+ A = sql_expr_list_append(pParse->db, NULL, X.pExpr);
+}
+
+%type getlist {ExprList *}
+%destructor getlist {sql_expr_list_delete(pParse->db, $$);}
+
expr(A) ::= LB(X) exprlist(Y) RB(E). {
struct Expr *expr = sql_expr_new_anon(pParse->db, TK_ARRAY);
if (expr == NULL) {
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 86de3f98a..ad8f2b3e9 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1458,6 +1458,35 @@ case OP_Map: {
break;
}
+/**
+ * Opcode: Getitem P1 P2 P3 * *
+ * Synopsis: r[P2] = value[P3@(P1 - 1)]
+ *
+ * Get an element from the value in register P3[P1 - 1] using values in
+ * registers P3, ... P3 + (P1 - 2).
+ */
+case OP_Getitem: {
+ int count = pOp->p1 - 1;
+ assert(count > 0);
+ struct Mem *value = &aMem[pOp->p3 + count];
+ if (mem_is_null(value)) {
+ diag_set(ClientError, ER_SQL_EXECUTE, "Selecting is not "
+ "possible from NULL");
+ goto abort_due_to_error;
+ }
+ if (mem_is_any(value) || !mem_is_msgpack(value)) {
+ diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(value),
+ "map or array");
+ goto abort_due_to_error;
+ }
+
+ pOut = &aMem[pOp->p2];
+ struct Mem *keys = &aMem[pOp->p3];
+ if (mem_getitem(value, keys, count, pOut) != 0)
+ goto abort_due_to_error;
+ break;
+}
+
/* Opcode: Eq P1 P2 P3 P4 P5
* Synopsis: IF r[P3]==r[P1]
*
diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg
index 528212ab6..52ce5a1f6 100644
--- a/test/sql-tap/engine.cfg
+++ b/test/sql-tap/engine.cfg
@@ -35,6 +35,7 @@
"built-in-functions.test.lua": {
"memtx": {"engine": "memtx"}
},
+ "msgpack.test.lua": {},
"gh-6157-unnecessary-free-on-string.test.lua": {},
"gh-6376-wrong-double-to-dec-cmp.test.lua": {},
"gh-4077-iproto-execute-no-bind.test.lua": {},
diff --git a/test/sql-tap/msgpack.test.lua b/test/sql-tap/msgpack.test.lua
new file mode 100755
index 000000000..a67da2ead
--- /dev/null
+++ b/test/sql-tap/msgpack.test.lua
@@ -0,0 +1,198 @@
+#!/usr/bin/env tarantool
+local test = require("sqltester")
+test:plan(23)
+
+-- Make sure that it is possible to get elements from MAP и ARRAY.
+test:do_execsql_test(
+ "msgpack-1.1",
+ [[
+ SELECT [123, 234, 356, 467][2];
+ ]], {
+ 234
+ })
+
+test:do_execsql_test(
+ "msgpack-1.2",
+ [[
+ SELECT {'one' : 123, 3 : 'two', '123' : true}[3];
+ ]], {
+ 'two'
+ })
+
+test:do_execsql_test(
+ "msgpack-1.3",
+ [[
+ SELECT {'one' : [11, 22, 33], 3 : 'two', '123' : true}['one'][2];
+ ]], {
+ 22
+ })
+
+test:do_execsql_test(
+ "msgpack-1.4",
+ [[
+ SELECT {'one' : 123, 3 : 'two', '123' : true}['three'];
+ ]], {
+ ""
+ })
+
+--
+-- Make sure that operator [] cannot get elements from values of types other
+-- than MAP and ARRAY.
+--
+test:do_catchsql_test(
+ "msgpack-2.1",
+ [[
+ SELECT 1[1];
+ ]], {
+ 1, "Selecting is only possible from map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.2",
+ [[
+ SELECT -1[1];
+ ]], {
+ 1, "Selecting is only possible from map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.3",
+ [[
+ SELECT 1.1[1];
+ ]], {
+ 1, "Selecting is only possible from map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.4",
+ [[
+ SELECT 1.2e0[1];
+ ]], {
+ 1, "Selecting is only possible from map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.5",
+ [[
+ SELECT '1'[1];
+ ]], {
+ 1, "Selecting is only possible from map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.6",
+ [[
+ SELECT x'31'[1];
+ ]], {
+ 1, "Selecting is only possible from map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.7",
+ [[
+ SELECT uuid()[1];
+ ]], {
+ 1, "Selecting is only possible from map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.8",
+ [[
+ SELECT true[1];
+ ]], {
+ 1, "Selecting is only possible from map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.9",
+ [[
+ SELECT CAST(1 AS NUMBER)[1];
+ ]], {
+ 1, "Selecting is only possible from map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.10",
+ [[
+ SELECT CAST('a' AS STRING)[1];
+ ]], {
+ 1, "Selecting is only possible from map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.11",
+ [[
+ SELECT NULL[1];
+ ]], {
+ 1, "Selecting is only possible from map and array values"
+ })
+
+test:do_catchsql_test(
+ "msgpack-2.12",
+ [[
+ SELECT CAST(NULL AS ANY)[1];
+ ]], {
+ 1, "Selecting is only possible from map and array values"
+ })
+
+-- Make sure that the second and the following brackets do not throw type error.
+test:do_execsql_test(
+ "msgpack-3.1",
+ [[
+ SELECT [1, 2, 3][1][2];
+ ]], {
+ ""
+ })
+
+test:do_execsql_test(
+ "msgpack-3.2",
+ [[
+ SELECT [1, 2, 3][1][2][3][4][5][6][7];
+ ]], {
+ ""
+ })
+
+test:do_catchsql_test(
+ "msgpack-3.3",
+ [[
+ SELECT ([1, 2, 3][1])[2];
+ ]], {
+ 1, "Type mismatch: can not convert integer(1) to map or array"
+ })
+
+-- Make sure that type of result of [] is checked in runtime.
+test:do_execsql_test(
+ "msgpack-4.1",
+ [[
+ SELECT [123, 23, 1][2] + 7;
+ ]], {
+ 30
+ })
+
+test:do_catchsql_test(
+ "msgpack-4.2",
+ [[
+ SELECT ['a', 2, true][1] + 1;
+ ]], {
+ 1, "Type mismatch: can not convert string('a') to integer, decimal "..
+ "or double"
+ })
+
+test:do_execsql_test(
+ "msgpack-4.3",
+ [[
+ SELECT {[11, 22, 33][3] : 'asd'};
+ ]], {
+ {[33] = 'asd'}
+ })
+
+-- Built-in functions in this case will use default types of arguments.
+test:do_execsql_test(
+ "msgpack-4.4",
+ [[
+ SELECT typeof(ABS([123, 23, 1][1]));
+ ]], {
+ "decimal"
+ })
+
+test:finish_test()
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] sql: introduce operator []
2021-11-30 15:20 ` Mergen Imeev via Tarantool-patches
@ 2021-12-02 23:43 ` Vladislav Shpilevoy via Tarantool-patches
0 siblings, 0 replies; 6+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-12-02 23:43 UTC (permalink / raw)
To: Mergen Imeev; +Cc: tarantool-patches
Thanks for the patch!
>> 2. Why not error? This looks a bit controversial with how hard the rules
>> for explicit and implict casts are.
>>
> Since there is no good way to catch an error, we try to avoid them as much as
> possible.
But for implicit cast violations we raise a lot of errors.
>>>> + 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.
> I replaced assert with error. I did not write about this either in the rules or
> in the commit message, since I am not sure if it is correct. To find a key, I
> create a MEM from MP, so looking for MAP or ARRAY key is inconvenient. We can
> try to go the other way - from MEM to MP. In this case, we can use memcmp() to
> look up the ARRAY and MAP keys, although this would be a fairly shallow
> comparison.
I would just forget about it. Comparing arrays and maps might be
possible for == and !=, but it is not possible for other comparison
operators. Also comparing maps for == would be very expensive because
order of fields in 2 equal maps can be different. Hence you have
N^2 complexity.
See 4 comments below. Nothing serious.
1. Lets add a changelog file.
> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> index 7411b8f67..32b0c3fb9 100644
> --- a/src/box/sql/mem.c
> +++ b/src/box/sql/mem.c
> @@ -3111,6 +3111,68 @@ 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) {
> + *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)
2. This might be nitpicking but when you use an expression
in the loop condition, you risk to force is recalculation on
each iteration. Because when mp_next(data) is called, compiler
can't be sure data != &key. So it will recalculate key->u.u - 1
on the next iteration.
To avoid that it usually helps to save the value somewhere. For
instance:
for (uint32_t i = 0, end = key->u.u - 1; i < end; ++i)
mp_next(data);
or here it can be even simpler:
for (uint32_t i = 1, end = key->u.u; i < end; ++i)
mp_next(data);
Up to you.
> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
> index 998acadea..d49920133 100644
> --- a/src/box/sql/parse.y
> +++ b/src/box/sql/parse.y
> @@ -1099,6 +1100,32 @@ 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 getlist(Y) RB(E). {
> + struct Expr *expr = sql_expr_new_anon(pParse->db, TK_GETITEM);
> + 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);
3. It might be better to introduce a function sql_expr_list_prepend.
Otherwise it is very counter-intuitive that the first expression
is on the last place, while the others are in correct places.
Or add a comment in expr_code_getitem().
Or save the first expression into expr->pLeft/pRight instead
of the index list.
> + 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];
> +}
> +
> +getlist(A) ::= getlist(A) RB LB expr(X). {
> + A = sql_expr_list_append(pParse->db, A, X.pExpr);
> +}
> +getlist(A) ::= expr(X). {
> + A = sql_expr_list_append(pParse->db, NULL, X.pExpr);
> +}
> +
> +%type getlist {ExprList *}
> +%destructor getlist {sql_expr_list_delete(pParse->db, $$);}
> +
> expr(A) ::= LB(X) exprlist(Y) RB(E). {
> struct Expr *expr = sql_expr_new_anon(pParse->db, TK_ARRAY);
> if (expr == NULL) {
4. In SQL NULL != anything else. But I managed to achieve this:
box.cfg{}
_ = box.schema.create_space('TEST', {format = {{name = 'F1', type = 'unsigned'}, {name = 'F2', type = 'map'}}}):create_index('pk')
box.space.TEST:replace{1, {[box.NULL] = 1}}
tarantool> box.execute('SELECT f2[NULL] FROM test')
---
- metadata:
- name: COLUMN_1
type: any
rows:
- [1]
...
Here NULL index = NULL key. Maybe it is correct. But it is worth
discussing with somebody to be sure. I can't tell how would we
get NULL keys if NULL = NULL wouldn't work here TBH. IS NULL
won't help.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-02 23:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-19 14:44 [Tarantool-patches] [PATCH v1 1/1] sql: introduce operator [] Mergen Imeev via Tarantool-patches
2021-11-19 17:25 ` Konstantin Osipov via Tarantool-patches
2021-11-21 15:53 ` Vladislav Shpilevoy via Tarantool-patches
2021-11-30 15:20 ` Mergen Imeev via Tarantool-patches
2021-12-02 23:43 ` Vladislav Shpilevoy via Tarantool-patches
2021-11-24 11:56 ` Mergen Imeev via Tarantool-patches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox