Added support for the List/Set type #5914
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
flymysql
reviewed
Aug 1, 2024
|
Thanks for your help. You can add some tests for your new features according to the following articles. |
Salieri-004
reviewed
Oct 7, 2024
Salieri-004
reviewed
Oct 14, 2024
Salieri-004
reviewed
Oct 15, 2024
Salieri-004
added
the
doc affected
|
LGTM. We need to add some doc for this feature. cc. @ChrisChen2023 @abby-cyber |
Salieri-004
previously approved these changes
Oct 16, 2024
codesigner
reviewed
Oct 16, 2024
| When try to execute query: | ||
| """ | ||
| INSERT VERTEX player(name, age, hobby, ids, score) VALUES "player100":( | ||
| "Tim Duncan", 42, ["Basketball", "Swimming"], [100, 528], [50.0, 22.0] |
There was a problem hiding this comment.
Is there testcase cover some corner cases, e.g.
- empty list/set, insert or update.
- Fire some error conditions: for example:
- type mismatched, insert List[?] to Set[?] , or insert List[int] to List[string] etc.
- invoke new added function with wrong type, e.g. add string to List[int] etc.
- what happends if try to create index on list/set column?
- very large list/set, according to your implementation, there is seem no limit (or more accurate max of int32) on the size of list or set, right? Should we support that large size list/set?
There was a problem hiding this comment.
- I have added test cases: Insert and update empty list/set
- If the inserted data type is inconsistent, an error will be reported: Storage Error: The data type does not meet the requirements. Use the correct type of data.
I also restricted the creation of list/set indexes and added test cases. - I have added a restriction on the array length
| }; | ||
| } | ||
| { | ||
| auto &attr = functions_["sadd"]; |
There was a problem hiding this comment.
looks like sadd is short for set add, maybe setadd is more readable?
There was a problem hiding this comment.
OK, I have modified it ：setadd
codesigner
approved these changes
Oct 25, 2024
Salieri-004
approved these changes
Oct 25, 2024
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For one-dimensional data of type List/Set:
List_string, List_int, List_float,
Set_string, Set_int, Set_float:
Support attributes defined as List /Set data types.
Use the List /Set data type for storage.
The List /Set data type can be filtered during query.
Attributes of the List /Set data type can be returned when the query is supported.
Added UPDATE statement to modify (REPLACE) and remove (ERASE) specified elements in List/Set types.
Added UPDATE statement to insert new data into List/Set: List = List + value, Set = SADD(Set, value).