Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

function to parse json format string prop #3513

Closed
wey-gu opened this issue Dec 21, 2021 · 10 comments · Fixed by #4743
Closed

function to parse json format string prop #3513

wey-gu opened this issue Dec 21, 2021 · 10 comments · Fixed by #4743
Labels
community Source: who proposed the issue good first issue Community: perfect as the first pull request type/feature req Type: feature request

Comments

@wey-gu
Copy link
Contributor

wey-gu commented Dec 21, 2021

context: https://discuss.nebula-graph.com.cn/t/topic/6991

@wey-gu wey-gu added the type/feature req Type: feature request label Dec 21, 2021
@jievince
Copy link
Contributor

Return a map?

@Sophie-Xie Sophie-Xie added the community Source: who proposed the issue label Jan 4, 2022
@jievince jievince added the good first issue Community: perfect as the first pull request label Jan 6, 2022
@akshatakulkarni25
Copy link

Hello. I am Akshata Kulkarni. I am interested in working on this issue. Is this issue still open to work on??It would be my first issue. Could you also please guide through this issue.Thanks

@wey-gu
Copy link
Contributor Author

wey-gu commented Sep 5, 2022

Dear @akshatakulkarni25 ,

Welcome to the community, this issue is still open!

I think a function similar to JSON_EXTRACT could be introduced here.

@jievince do you see it as reasonable/feasible?

why

The motivation of this issue was NebulaGraph doesn't support nested datatype, in some cases, user may like to serialize JSON data in a string as properties of Tag/edgeType, thus, this function would enable data extracting in return expression.
(We should consider stopping user from using it in filter expression, as this will lead to a data full scan. @jievince ? )

what

  • It takes a string and returns a map:
YIELD JSON_EXTRACT('{"a": 1, "b": 2, "c": {"d": 4}}') AS map_foo
+-------------------------+
| map_foo                 |
+-------------------------+
| {a: 1, b: 2, c: {d: 4}} |
+-------------------------+

- It takes path as args, too (@jievince do you think this is needed?, this makes the function a lot more complex to be implemented)

YIELD JSON_EXTRACT('{"a": 1, "b": 2, "c": {"d": 4}}', '$.c') AS map_foo_dot_c
+-----------+
| map_foo   |
+-----------+
| {d: 4}    |
+-----------+

@jievince
Copy link
Contributor

jievince commented Sep 5, 2022

Dear @akshatakulkarni25 ,

Welcome to the community, this issue is still open!

I think a function similar to JSON_EXTRACT could be introduced here.

@jievince do you see it as reasonable/feasible?

why

The motivation of this issue was NebulaGraph doesn't support nested datatype, in some cases, user may like to serialize JSON data in a string as properties of Tag/edgeType, thus, this function would enable data extracting in return expression. (We should consider stopping user from using it in filter expression, as this will lead to a data full scan. @jievince ? )

what

  • It takes a string and returns a map:
YIELD JSON_EXTRACT('{"a": 1, "b": 2, "c": {"d": 4}}') AS map_foo
+-------------------------+
| map_foo                 |
+-------------------------+
| {a: 1, b: 2, c: {d: 4}} |
+-------------------------+
  • It takes path as args, too (@jievince do you think this is needed?, this makes the function a lot more complex to be implemented)
YIELD JSON_EXTRACT('{"a": 1, "b": 2, "c": {"d": 4}}', '$.c') AS map_foo_dot_c
+-----------+
| map_foo   |
+-----------+
| {d: 4}    |
+-----------+

That's a good point. But it may be a bit troublesome to implement all the json functions in mysql. I suggest you could first implement a simple function, such as convertToMap, which takes a json string, and returns a map value it represents.

@jievince
Copy link
Contributor

jievince commented Sep 5, 2022

The implementation of the builtin functions is here: https://github.com/vesoft-inc/nebula/blob/master/src/common/function/FunctionManager.cpp

Looking forward to your contribution.

@wey-gu
Copy link
Contributor Author

wey-gu commented Sep 5, 2022

also, one PR on introducing functions here

Let us know your questions any time :) @akshatakulkarni25

@akshatakulkarni25
Copy link

akshatakulkarni25 commented Sep 5, 2022 via email

@wey-gu
Copy link
Contributor Author

wey-gu commented Sep 5, 2022

Dear @akshatakulkarni25 ,
Sure, you could mail/slack me when needed :)

@wey-gu
Copy link
Contributor Author

wey-gu commented Oct 19, 2022

Hi @akshatakulkarni25

May I know your status on this issue? It's ok if you could not make it, I am trying to prepare a PR on it, and in case yours is ready, we could co-work on yours instead.

Thanks!

wey-gu added a commit that referenced this issue Oct 19, 2022
close: #3513
Note, we don't support the path argument in this phase
Sophie-Xie added a commit that referenced this issue Oct 24, 2022
* Introduce JSON_EXTRACT function

close: #3513
Note, we don't support the path argument in this phase

* address jievince's review commit

removed the unecessary interface of construct Map from Value

* Type handling

Only primitive types are supported

* Support depth1 nested

* lint: fmt

* ut: ctest, fixed wrong expression of Map

* fix ut errors

* tck: case for json_extract added

Co-authored-by: Sophie <[email protected]>
Sophie-Xie added a commit that referenced this issue Oct 24, 2022
* Introduce JSON_EXTRACT function

close: #3513
Note, we don't support the path argument in this phase

* address jievince's review commit

removed the unecessary interface of construct Map from Value

* Type handling

Only primitive types are supported

* Support depth1 nested

* lint: fmt

* ut: ctest, fixed wrong expression of Map

* fix ut errors

* tck: case for json_extract added

Co-authored-by: Sophie <[email protected]>
Shinji-IkariG pushed a commit that referenced this issue Oct 25, 2022
* Fix/find start error (#4771)

* Fix find start error.

* Fix test.

Co-authored-by: Sophie <[email protected]>

* Introduce JSON_EXTRACT function (#4743)

* Introduce JSON_EXTRACT function

close: #3513
Note, we don't support the path argument in this phase

* address jievince's review commit

removed the unecessary interface of construct Map from Value

* Type handling

Only primitive types are supported

* Support depth1 nested

* lint: fmt

* ut: ctest, fixed wrong expression of Map

* fix ut errors

* tck: case for json_extract added

Co-authored-by: Sophie <[email protected]>

* Fix bug #1337 from ent (#4740)

* Return an semantic error if no tag is found for a property while validating a match.

* Return an semantic error if no tag is found for a property while validating a match.

* Add a tck case for the fixed bug.

* commented out unused codes.

* add tag in tck cases

* fixing tck

* updated tck cases to add missing cases that are supposed to be there.

* Return an semantic error if no tag is found for a property while validating a match.

* Add a tck case for the fixed bug.

* commented out unused codes.

* add tag in tck cases

* fixing tck

* updated tck cases to add missing cases that are supposed to be there.

* update

* update tck case.

Co-authored-by: Sophie <[email protected]>

* Fix RollUpApplyExecutor (#4778)

Co-authored-by: Sophie <[email protected]>

* Fix mutil-match crash in optimization phase (#4780)

fmt

small fix

small fix

* fix subgraph step (#4776)

* fix subgraph step

* forbid function call in where clause

* fix error

Co-authored-by: shylock <[email protected]>
Co-authored-by: Wey Gu <[email protected]>
Co-authored-by: Cheng Xuntao <[email protected]>
Co-authored-by: Yichen Wang <[email protected]>
Co-authored-by: kyle.cao <[email protected]>
Co-authored-by: jimingquan <[email protected]>
@jinyingsunny
Copy link

jinyingsunny commented Oct 26, 2022

checked on 3.3-rc
checked on 3.3-ent-rc

yixinglu pushed a commit to yixinglu/nebula that referenced this issue Jan 31, 2023
* Introduce JSON_EXTRACT function

close: vesoft-inc#3513
Note, we don't support the path argument in this phase

* address jievince's review commit

removed the unecessary interface of construct Map from Value

* Type handling

Only primitive types are supported

* Support depth1 nested

* lint: fmt

* ut: ctest, fixed wrong expression of Map

* fix ut errors

* tck: case for json_extract added

Co-authored-by: Sophie <[email protected]>

Co-authored-by: Wey Gu <[email protected]>
Co-authored-by: Sophie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Source: who proposed the issue good first issue Community: perfect as the first pull request type/feature req Type: feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants