Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@freelists.org, kshcherbatov@tarantool.org
Subject: Re: [tarantool-patches] Re: [PATCH v3 3/3] Lua: implement json path access to tuple fields
Date: Mon, 16 Apr 2018 11:35:49 +0300	[thread overview]
Message-ID: <20180416083549.uyiayz3kcqfgd6kt@esperanza> (raw)
In-Reply-To: <48a37669-c959-1f31-1bee-ecd85787046f@tarantool.org>

On Sat, Apr 14, 2018 at 12:51:30AM +0300, Vladislav Shpilevoy wrote:
> Hello. Thanks for review.
> 
> New commit message:
> 
>     lua: implement json path access to tuple fields
> 
>     Until this moment in Lua a tuple data could be accessed in 3 ways:
>     - get field by field number, decode into Lua,
>     - get field by field name, decode into Lua
>     - decode entire tuple into Lua.
> 
>     It was impossible to decode into Lua only a part of the field to
>     avoid Lua garbage creating. For example, consider the tuple:
>     `{1, 2, {key1 = value1,
>              key2 = {key21 = {key211 = {key2111, key2112}}}}}`
>     with field names `field1`, `field2`, `field3`.
> 
>     To get `key2112` it is necessary to decode into Lua the entire
>     3-th field, including `key1`, `value1`, `key2`, `key21`,
>     `key211`, `key2111` using the syntax
>     `key2112 = tuple.field3.key2.key21.key211[2]`.
> 
>     Now the one can use the following syntax:
>     `key2112 = tuple["field3.key2.key21.key211[2]"]`. The difference
>     is in placing all the path into quotes and brackets `["..."]`.
>     The Tarantool goes through this path and MessagePack tuple body,
>     gets the needed tuple part and decodes into Lua only it.
> 
>     The path must be valid JSON path with one exception - in
>     Tarantool a path can start with `.`. For example:
>     `tuple[".field3..."]` - it is done to lay emphasis that the JSON
>     path here is just a suffix for the tuple.
> 
>     At the same time tuple field names still work: `tuple["field1"]`,
>     `tuple.field2`.
> 
>     If the field name looks like JSON path, for example:
>     `my.field.name`, then it works too. The path at first is checked
>     to be a real JSON path, and if nothing is found, then the entire
>     path is considered as a field name. To combine such names and
>     path elements the one can use `["..."]`. For example:
>     `tuple["['my.field.name'].key1.key2.['another.key.in.map']"]`.
> 
>     Closes #1285
> 
> > 
> > Why did you move handling of integer index from Lua to C? AFAIU having
> > it in Lua should be faster. Besides, this new function is kinda
> > difficult to follow IMO as it does two things now. I'd prefer it to do
> > just one thing - looking up a tuple field by path.
> > 
> 
> Fixed. I returned the tuple field by number implementation as it
> was before JSON path patch.

Thanks. The patch looks good to me now.

There's one thing: the hunk below doesn't belong to this patch.
Obviously, it should be a part of the previous patch. Please move
it there before pushing it to the trunk.

diff --git a/test/unit/json_path.result b/test/unit/json_path.result
index 9b4ea641..a2a2f829 100644
--- a/test/unit/json_path.result
+++ b/test/unit/json_path.result
@@ -1,7 +1,7 @@
 	*** main ***
 1..2
 	*** test_basic ***
-    1..67
+    1..71
     ok 1 - parse <[0]>
     ok 2 - <[0]> is num
     ok 3 - <[0]> is 0
@@ -46,29 +46,33 @@
     ok 42 - <field1> is str
     ok 43 - len is 6
     ok 44 - str is field1
-    ok 45 - parse <[1234]>
-    ok 46 - <[1234]> is num
-    ok 47 - <[1234]> is 1234
-    ok 48 - parse empty path
-    ok 49 - is str
-    ok 50 - parse <field1>
-    ok 51 - <field1> is str
-    ok 52 - len is 6
-    ok 53 - str is field1
-    ok 54 - parse <[2]>
-    ok 55 - <[2]> is num
-    ok 56 - <[2]> is 2
-    ok 57 - parse <[6]>
-    ok 58 - <[6]> is num
-    ok 59 - <[6]> is 6
-    ok 60 - parse <привет中国world>
-    ok 61 - <привет中国world> is str
-    ok 62 - len is 23
-    ok 63 - str is привет中国world
-    ok 64 - parse <中国a>
-    ok 65 - <中国a> is str
-    ok 66 - len is 7
-    ok 67 - str is 中国a
+    ok 45 - parse <field1>
+    ok 46 - <field1> is str
+    ok 47 - len is 6
+    ok 48 - str is field1
+    ok 49 - parse <[1234]>
+    ok 50 - <[1234]> is num
+    ok 51 - <[1234]> is 1234
+    ok 52 - parse empty path
+    ok 53 - is str
+    ok 54 - parse <field1>
+    ok 55 - <field1> is str
+    ok 56 - len is 6
+    ok 57 - str is field1
+    ok 58 - parse <[2]>
+    ok 59 - <[2]> is num
+    ok 60 - <[2]> is 2
+    ok 61 - parse <[6]>
+    ok 62 - <[6]> is num
+    ok 63 - <[6]> is 6
+    ok 64 - parse <привет中国world>
+    ok 65 - <привет中国world> is str
+    ok 66 - len is 23
+    ok 67 - str is привет中国world
+    ok 68 - parse <中国a>
+    ok 69 - <中国a> is str
+    ok 70 - len is 7
+    ok 71 - str is 中国a
 ok 1 - subtests
 	*** test_basic: done ***
 	*** test_errors ***

  reply	other threads:[~2018-04-16  8:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-10  8:31 [PATCH v3 0/3] Implement " Vladislav Shpilevoy
2018-04-10  8:31 ` [PATCH v3 1/3] Allow gcov on Mac Vladislav Shpilevoy
2018-04-10 10:49   ` Vladimir Davydov
     [not found] ` <f1302082b4457a03b5d2b3c44526815428de4a0e.1523349020.git.v.shpilevoy@tarantool.org>
2018-04-10 16:36   ` [PATCH v3 2/3] Introduce json_path_parser with Unicode support Vladimir Davydov
2018-04-10 18:42     ` [tarantool-patches] " Vladislav Shpilevoy
2018-04-11  9:20       ` Vladimir Davydov
     [not found] ` <c3b64b7b4e638970864995c895bbd5f736282560.1523349020.git.v.shpilevoy@tarantool.org>
2018-04-13 11:33   ` [PATCH v3 3/3] Lua: implement json path access to tuple fields Vladimir Davydov
2018-04-13 21:51     ` [tarantool-patches] " Vladislav Shpilevoy
2018-04-16  8:35       ` Vladimir Davydov [this message]
2018-04-16 10:02         ` Vladislav Shpilevoy
2018-04-22 14:21         ` Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180416083549.uyiayz3kcqfgd6kt@esperanza \
    --to=vdavydov.dev@gmail.com \
    --cc=kshcherbatov@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [tarantool-patches] Re: [PATCH v3 3/3] Lua: implement json path access to tuple fields' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox