support nested containers as attribute type #658

swilly22 · 2024-04-21T10:53:01Z

Resolves #657, #31

Summary by CodeRabbit

New Features
- Added functionality to check for a specific type in nested maps within arrays.
- Introduced new functions to create maps from keys and values arrays, from a binary stream, and to check if a map contains a key with a specific type.
- Added functions to handle writing map data to effects buffers and saving maps in a specific format within serializers.
Bug Fixes
- Updated error message related to property values, specifying valid types as primitive types, arrays, or maps.
- Updated handling of NULL values in property maps.
- Refined validation for array and map property values to exclude invalid types.
Documentation
- Enhanced comments for clarity and consistency in various files.
Tests
- Added and adjusted test cases to reflect new features and changes in handling properties and map types.

coderabbitai · 2024-04-21T10:53:24Z

Walkthrough

The recent updates introduce enhanced support for nested data structures within graph entities, specifically focusing on nested arrays and maps. This includes new functionalities for creating, updating, and serializing maps, as well as improved error handling and validation for entity attributes that utilize these complex types.

Changes

Files	Change Summary
`src/datatypes/map.c`, `src/datatypes/map.h`	Added functions to create maps from arrays, binary streams, and check for specific key types.
`src/errors/error_msgs.h`	Updated error messages related to shortestPath traversals and SET expressions to mention FalkorDB.
`tests/flow/test_all_shortest_paths.py`	Updated error messages related to shortest paths to mention FalkorDB.
`tests/flow/test_entity_update.py`	Adjusted property assignments in Cypher queries and updated error messages and test method names.
`tests/flow/test_graph_create.py`	Modified queries for creating invalid complex types in arrays, added a test for creating entities with arrays and maps as attributes.
`tests/flow/test_persistency.py`	Added a test to verify the persistence of nested containers in a graph database.
`tests/flow/test_shortest_path.py`	Updated assertion message related to shortest paths to mention FalkorDB.

Assessment against linked issues

Objective	Addressed	Explanation
Add support for nested datatypes, specifically maps, in properties. (#657)	✅
Enable the usage of maps as property values within the system. (#657)	✅
Maintain backward compatibility with existing property structures. (#657)	✅
Ensure that the system can handle nested data structures for properties. (#657)	✅
Provide clear documentation and examples for utilizing nested map datatypes in properties. (#657)	❌	Documentation and examples are not covered in the PR changes.

Possibly related issues

Add support for Nested datatypes (Maps) in properties #31: The changes directly support nested maps and arrays as entity attributes, aligning with the objectives to handle nested data structures.

🐇🎉
A map and array nested deep,
In code where data structures sleep.
Now they stir, in graph's embrace,
Crafted with elegance and grace.
Cheers to the coders, their wisdom we reap!
🎉🐇

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 1

Actionable comments outside the diff hunks (1)

tests/flow/test_effects.py (1)
Line range hint 36-36: Avoid using a bare except statement.
- except:
+ except Exception as e:
+     # Handle or log the exception e appropriately
Using a bare except can catch unexpected events such as memory errors, interrupts, system exit, and so on. It's better to catch specific exceptions to avoid masking other errors.

coderabbitai · 2024-04-21T11:00:56Z

tests/flow/test_effects.py

+                v: vecf32([-1.1, 2.2, -3.3]),
+                m: {a:1, b:{c:'string_val'}}
+                }"""


Tip
Codebase Verification

Several files in the codebase still use the old function signature.

The search did not reveal specific handling for the attributes v and m in serialization or deserialization processes.

The only related mention was a generic comment in tests/flow/test_persistency.py about property handling, which does not confirm specific management for v and m.

Analysis chain

Ensure the new attributes v and m are correctly handled in the test.

Check the serialization and deserialization processes for the updated attributes in edge updates using map overwrite.

Scripts Executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash # Verify that the attributes 'v' and 'm' are being correctly serialized and deserialized. rg --type py "serialize_v_attribute" rg --type py "serialize_m_attribute" rg --type py "deserialize_v_attribute" rg --type py "deserialize_m_attribute"

Length of output: 151

Script:

#!/bin/bash # Broaden the search to check for any serialization or deserialization related to 'v' and 'm' in Python files rg --type py -A 3 -B 3 "v" | grep -i "serialize\|deserialize" rg --type py -A 3 -B 3 "m" | grep -i "serialize\|deserialize"

Length of output: 397

swilly22

See comments

swilly22 · 2024-05-01T07:36:24Z

tests/flow/test_persistency.py

+                   "CREATE ({v: []})",      # empty array
+                   "CREATE ({v: [[]]})",    # nested array
+                   "CREATE ({v: [{}]})",    # map within array
+                   "CREATE ({v: {x:{}}})",  # nested map


Add additional values to nested maps.

swilly22 · 2024-05-01T07:38:38Z

tests/flow/test_graph_create.py


        for query in queries:
            try:
                self.graph.query(query)
                self.env.assertTrue(False)
            except redis.exceptions.ResponseError as e:
-                self.env.assertContains("Property values can only be of primitive types or arrays of primitive types", str(e))
+                self.env.assertContains("Property values can only be of primitive types arrays or maps", str(e))


Rephrase,
e.g. "properties can't contain Graph entities nor Paths."

swilly22 · 2024-05-01T07:40:14Z

tests/flow/test_graph_create.py

+                ("CREATE (a {v: {s:1}}) RETURN a.v", {'s': 1}),
+
+                # map with NULL element
+                ("CREATE (a {v: {s: null}}) RETURN a.v", {'s': None}),


Include a scenario which tests map containing strings

swilly22 · 2024-05-01T07:53:22Z

tests/flow/test_entity_update.py


        try:
            self.graph.query("MERGE (n:N) ON MATCH SET n = {v: 1}, n.a.b=3 RETURN n")
            self.env.assertTrue(False)
        except ResponseError as e:
-            self.env.assertContains("RedisGraph does not currently support non-alias references on the left-hand side of SET expressions", str(e))
+            self.env.assertContains("FalkorDB does not currently support non-alias references on the left-hand side of SET expressions", str(e))

    # Fail when a property is a complex type nested within an array type
    def test13_invalid_complex_type_in_array(self):
        # Test combinations of invalid types with nested and top-level arrays
        # Invalid types are NULL, maps, nodes, edges, and paths


update comment

swilly22 · 2024-05-02T09:36:37Z

tests/flow/test_entity_update.py

+        try:
+            res = self.graph.query(q)
+        except ResponseError as e:
+            self.env.assertContains("", str(e))


fill in exception error message

swilly22 · 2024-05-02T09:40:56Z

src/serializers/encoder/v14/encode_graph_entities.c

@@ -60,6 +60,29 @@ static void _RdbSaveSIVector
 		SerializerIO_WriteFloat(rdb, values[i]);
 	}
 }
+static void _RdBSaveMap


Talk to Dudi regarding update procedure.

swilly22 · 2024-05-02T09:42:36Z

src/serializers/decoders/current/v14/decode_graph_entities.c

@@ -9,6 +9,7 @@
 // forward declarations
 static SIValue _RdbLoadPoint(SerializerIO rdb);
 static SIValue _RdbLoadSIArray(SerializerIO rdb);
+static SIValue _RdbLoadMap (SerializerIO rdb, SIType t);


Remove extra space!

swilly22 · 2024-05-02T09:52:33Z

src/execution_plan/ops/shared/create_functions.c

+		// a map containing an invalid type
+		if(SI_TYPE(val) == T_MAP) {
+			SIType invalid_types = ~SI_VALID_PROPERTY_VALUE & ~T_NULL;
+			bool res = SIArray_ContainsType(val, invalid_types);


Wrong function call, should be SIMap_ContainsType!

swilly22 · 2024-05-02T09:56:00Z

Performance test for Saving / Loading graphs with plenty of maps, some of which are small others might be quite large.

support nested containers as attribute type

70de395

swilly22 requested a review from AviAvni April 21, 2024 10:53

coderabbitai bot reviewed Apr 21, 2024

View reviewed changes

skip TCK test checking map assignment to an attribute

9e67ebc

swilly22 force-pushed the nested-attributes branch from fd627f7 to 9e67ebc Compare April 21, 2024 11:44

swilly22 and others added 2 commits April 21, 2024 14:44

Merge branch 'master' into nested-attributes

6880e52

additonal tests

f1e61a1

flowt-au mentioned this pull request Apr 28, 2024

Support for FalkorDB vert-x3/vertx-redis-client#446

Open

swilly22 commented May 2, 2024

View reviewed changes

Merge branch 'master' into nested-attributes

9a1ffe0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support nested containers as attribute type #658

support nested containers as attribute type #658

swilly22 commented Apr 21, 2024 •

edited by coderabbitai bot

coderabbitai bot commented Apr 21, 2024 •

edited

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

coderabbitai bot left a comment

coderabbitai bot Apr 21, 2024

swilly22 left a comment

swilly22 May 1, 2024

swilly22 May 1, 2024

swilly22 May 1, 2024

swilly22 May 1, 2024

swilly22 May 2, 2024

swilly22 May 2, 2024

swilly22 May 2, 2024

swilly22 May 2, 2024

swilly22 commented May 2, 2024

support nested containers as attribute type #658

Are you sure you want to change the base?

support nested containers as attribute type #658

Conversation

swilly22 commented Apr 21, 2024 • edited by coderabbitai bot

Summary by CodeRabbit

Summary by CodeRabbit

coderabbitai bot commented Apr 21, 2024 • edited

Walkthrough

Changes

Assessment against linked issues

Possibly related issues

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot Apr 21, 2024

Choose a reason for hiding this comment

swilly22 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swilly22 commented May 2, 2024

swilly22 commented Apr 21, 2024 •

edited by coderabbitai bot

coderabbitai bot commented Apr 21, 2024 •

edited

CodeRabbit Configration File (`.coderabbit.yaml`)