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

[develop < T0041-GA] Fix label filtering in where queries #103

Merged
merged 29 commits into from Mar 30, 2022

Conversation

katarinasupe
Copy link
Contributor

@katarinasupe katarinasupe commented Mar 14, 2022

In this PR, I wanted to check how to create private properties which won't be saved in the database and how to load a node with partial properties, that is, how to load a node with only required properties provided.

That is related to issues #88 and #79.

Conclusions:

  • A property without Field() will be saved into Memgraph. If you don't want a property to be saved into Memgraph, declare it as private property (e.g. _name). All properties can be used without Field() and schema validation will still work. Field must be used when defining additional fields, such as constraints and indexes.

  • I think it's okay that load works the way it does. If we want to fetch nodes by one property, we can use query builder, that is match().

While working on this, I noticed a couple of more things I added to this PR:

  • WHERE clause and all its versions (and, or, xor) had label filtering added, but this was not tested. I added tests and fixed methods in the query builder accordingly.

  • Changed property to item in xor_where() arguments

  • Fixed WHERE methods in query builder (where, and where, or where, xor where) to work with labels

  • Added tests for label filtering in WHERE methods (where, and where, or where, xor where)

  • Added tests for node loading and properties usage

@katarinasupe katarinasupe added this to In progress in Release v1.2 via automation Mar 14, 2022
@katarinasupe katarinasupe removed this from In progress in Release v1.2 Mar 14, 2022
@katarinasupe katarinasupe changed the base branch from main to develop March 14, 2022 15:59
@katarinasupe katarinasupe added the status: change PR reviewed - needs changes label Mar 14, 2022
@katarinasupe katarinasupe added this to In progress in Release v1.2 via automation Mar 14, 2022
@katarinasupe katarinasupe self-assigned this Mar 14, 2022
@katarinasupe katarinasupe changed the title Add example usage of class properties Fix label filtering in where and add new tests Mar 28, 2022
@katarinasupe katarinasupe changed the title Fix label filtering in where and add new tests Fix label filtering in where queries and add new tests Mar 28, 2022
@katarinasupe katarinasupe added status: ready PR is ready for review and removed status: change PR reviewed - needs changes labels Mar 28, 2022
@katarinasupe katarinasupe marked this pull request as ready for review March 28, 2022 07:38
@katarinasupe katarinasupe changed the title Fix label filtering in where queries and add new tests [develop < T0041-GA] Fix label filtering in where queries Mar 29, 2022
@katarinasupe
Copy link
Contributor Author

Please ignore where() method in query builder here. We need to merge this before where() PR. This PR is just couple of new tests in the end.

Release v1.2 automation moved this from In progress to Review in progress Mar 30, 2022
Copy link
Contributor

@BorisTasevski BorisTasevski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Classes as fixtures should also be discussed here

gqlalchemy/query_builder.py Outdated Show resolved Hide resolved
gqlalchemy/query_builder.py Outdated Show resolved Hide resolved
gqlalchemy/query_builder.py Outdated Show resolved Hide resolved
tests/memgraph/test_query_builder.py Outdated Show resolved Hide resolved
tests/memgraph/test_query_builder.py Outdated Show resolved Hide resolved
tests/memgraph/test_query_builder.py Outdated Show resolved Hide resolved
tests/memgraph/test_query_builder.py Outdated Show resolved Hide resolved
Release v1.2 automation moved this from Review in progress to Reviewer approved Mar 30, 2022
@BorisTasevski BorisTasevski merged commit 4fa9c4b into develop Mar 30, 2022
Release v1.2 automation moved this from Reviewer approved to Done Mar 30, 2022
@BorisTasevski BorisTasevski deleted the test-properties-usage branch March 30, 2022 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready PR is ready for review
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Property without Field() shouldn't be saved into Memgraph Load() doesn't work without default or Optional
5 participants