From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 2A7766E459; Sat, 20 Nov 2021 03:47:01 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2A7766E459 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1637369221; bh=Bl8fextUmN2cSfTWShsI1UdfwNfTMe7sLkEUbZ0qlCs=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=UOqYbMZZKZAOawxqC6QpH0ac31SiiN8PfFiihkRx+LrzSpmrslpBErbpsIzRzjzol ZxEbpcKeTnnD+fYf2HiE3nLuAYwxQwDMtmsTAkCSfXfTytMrKpe21fjchdbxrXuSke +Rs4o/wX0Vvf3RYASCfICfcmOpI7R2eU/c2UXYHI= Received: from smtpng3.i.mail.ru (smtpng3.i.mail.ru [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 99BC36E459 for ; Sat, 20 Nov 2021 03:46:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 99BC36E459 Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1moEWv-0002xa-WA; Sat, 20 Nov 2021 03:46:58 +0300 Message-ID: <662f6b87-f085-ab10-53b8-d087d9598b19@tarantool.org> Date: Sat, 20 Nov 2021 01:46:57 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.3.1 Content-Language: en-US To: imeevma@tarantool.org Cc: tarantool-patches@dev.tarantool.org References: <4ecfb3439688bef76c96270624410dee8822176f.1637244389.git.imeevma@gmail.com> In-Reply-To: <4ecfb3439688bef76c96270624410dee8822176f.1637244389.git.imeevma@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD9731B3922EC063979441C7E49BA1D360DCF2D4FD3FE2A0DCE00894C459B0CD1B9102E23E73602DF7757089AF07AF05397B9415F14626DCF3F9F0FCD7AA15E9C54 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7E4182CE4FE3052C2EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063779089FB2CE4EA2908638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D813A2D9ED095FABB01B2F764DFF45BE08117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF1175FABE1C0F9B6A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18C26CFBAC0749D213D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6300D3B61E77C8D3B089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458834459D11680B505C576550E7231921EB9495A95410DEAD7 X-C1DE0DAB: 0D63561A33F958A5150F7DBA91294F110864F300284AE7BE07210A72E3EA98E0D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7506FE1F977233B9BB410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3467D08F30473A5842DD5D019C54F7A258F6023BC865B8E3B7204ACE4AA1CABAE89FC2E44597703DCD1D7E09C32AA3244CF29455C2346D8295F2F10FFEE58691F3A90944CA99CF22E3729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojUkwcpHt8ZEe8jvGwTZX96g== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5DAB8ACCCC53BA813061FFDDA4260E39413841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E267EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v1 2/2] sql: introduce syntax for MAP values X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thanks for the patch! See 7 comments below. > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index 74a98c550..789d8906c 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -3432,6 +3432,35 @@ expr_code_array(struct Parse *parser, struct Expr *expr, int reg) > sqlVdbeAddOp3(vdbe, OP_Array, count, reg, values_reg); > } > > +static void > +expr_code_map(struct Parse *parser, struct Expr *expr, int reg) 1. I thought the policy was that we name functions, generating VDBE code, using 'emit' suffix. For instance, `vdbe_emit_map()` or `sql_emit_map()`. Don't know about prefix though. I see both vdbe_ and sql_ are used. > diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c > index b598fe5c2..fe7029341 100644 > --- a/src/box/sql/mem.c > +++ b/src/box/sql/mem.c > @@ -3043,6 +3043,45 @@ mem_encode_array(const struct Mem *mems, uint32_t count, uint32_t *size, > return array; > } > > +char * > +mem_encode_map(const struct Mem *mems, uint32_t count, uint32_t *size, > + struct region *region) > +{ > + assert(count % 2 == 0); > + size_t used = region_used(region); > + bool is_error = false; > + struct mpstream stream; > + mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb, > + set_encode_error, &is_error); > + mpstream_encode_map(&stream, (count + 1) / 2); > + for (uint32_t i = 0; i < count / 2; ++i) { > + const struct Mem *key = &mems[2 * i]; > + const struct Mem *value = &mems[2 * i + 1]; > + if (mem_is_metatype(key) || > + (key->type & (MEM_TYPE_UINT | MEM_TYPE_INT | MEM_TYPE_UUID | > + MEM_TYPE_STR)) == 0) { > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, > + mem_str(key), "integer, string or uuid"); > + return NULL; > + } > + mem_to_mpstream(key, &stream); > + mem_to_mpstream(value, &stream); > + } > + mpstream_flush(&stream); > + if (is_error) { 2. The error could happen in the last moment after some allocations were made. Better add a truncate here. > + diag_set(OutOfMemory, stream.pos - stream.buf, > + "mpstream_flush", "stream"); > + return NULL; > + } > + *size = region_used(region) - used; > + char *map = region_join(region, *size); > + if (map == NULL) { 3. Ditto. And the same in mem_encode_array(). > + diag_set(OutOfMemory, *size, "region_join", "map"); > + return NULL; > + } > + return map; > +} > diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y > index 06e6244e3..db7fef71a 100644 > --- a/src/box/sql/parse.y > +++ b/src/box/sql/parse.y > @@ -1140,6 +1140,37 @@ expr(A) ::= LB(X) exprlist(Y) RB(E). { > spanSet(&A, &X, &E); > } > > +expr(A) ::= LCB(X) maplist(Y) RCB(E). { > + struct sql *db = pParse->db; > + struct Expr *expr = sql_expr_new_dequoted(db, TK_MAP, NULL); 4. sql_expr_new_anon(). > + if (expr == NULL) { > + sql_expr_list_delete(db, Y); > + pParse->is_aborted = true; > + return; > + } > + expr->x.pList = Y; > + expr->type = FIELD_TYPE_MAP; > + sqlExprSetHeightAndFlags(pParse, expr); > + A.pExpr = expr; > + spanSet(&A, &X, &E); > +} > + > +maplist(A) ::= nmaplist(A). > +maplist(A) ::= . {A = 0;} 5. Lets remove these extra spaces between . and the code block. > +nmaplist(A) ::= nmaplist(A) COMMA expr(X) COLON expr(Y). { > + A = sql_expr_list_append(pParse->db, A, X.pExpr); > + A = sql_expr_list_append(pParse->db, A, Y.pExpr); > +} > +nmaplist(A) ::= expr(X) COLON expr(Y). { > + A = sql_expr_list_append(pParse->db, NULL, X.pExpr); > + A = sql_expr_list_append(pParse->db, A, Y.pExpr); > +} > + > +%type maplist {ExprList*} > +%destructor maplist {sql_expr_list_delete(pParse->db, $$);} > +%type nmaplist {ExprList*} > +%destructor nmaplist {sql_expr_list_delete(pParse->db, $$);} 6. Could you please add a whitespace between type and '*'? 7. Lets add some more complicated syntax tests: tarantool> box.execute('SELECT {:name}', {{[':name'] = 1}}) --- - null - Syntax error at line 1 near '}' ... tarantool> box.execute('SELECT {:name: 5}', {{[':name'] = 1}}) --- - metadata: - name: COLUMN_1 type: map rows: - [{1: 5}] ... tarantool> box.execute('SELECT {5::name}', {{[':name'] = 1}}) --- - metadata: - name: COLUMN_1 type: map rows: - [{5: 1}] ... To see if the parse is able to handle both map ':' and variable ':'.