Skip to content

RETURN propertyTypes(n) feature + fast retrieval of property types of an entity from property manager - #519

Open
LucLabarriere wants to merge 12 commits into
mainfrom
query/feature/propertyTypes
Open

RETURN propertyTypes(n) feature + fast retrieval of property types of an entity from property manager#519
LucLabarriere wants to merge 12 commits into
mainfrom
query/feature/propertyTypes

Conversation

@LucLabarriere

@LucLabarriere LucLabarriere commented Mar 17, 2026

Copy link
Copy Markdown
Contributor
  • Loading times from the binary before/after: 2.9/3.2 seconds (reactome)
  • quiet mode MATCH (n) RETURN propertyTypes(n) on reactome: 400 ms
  • quiet mode MATCH (n)-[e]->(m) RETURN propertyTypes(e): 508 ms

@LucLabarriere LucLabarriere self-assigned this Mar 17, 2026
@LucLabarriere
LucLabarriere force-pushed the query/feature/propertyTypes branch from dbb3d4d to cd6a702 Compare March 26, 2026 08:37
@LucLabarriere LucLabarriere added the enhancement New feature or request label Mar 26, 2026
Comment thread storage/columns/Functions.h Outdated
Comment thread storage/properties/PropertyManager.cpp Outdated
Comment thread storage/DataPart.cpp Outdated
@LucLabarriere
LucLabarriere marked this pull request as ready for review March 26, 2026 14:38
@LucLabarriere
LucLabarriere requested a review from rjb32 as a code owner March 26, 2026 14:38
Comment thread storage/properties/PropertyTypeTrie.h Outdated
Comment thread storage/properties/PropertyTypeTrie.h Outdated
Comment thread storage/properties/PropertyTypeTrie.h
Comment thread storage/properties/PropertyTypeTrie.h Outdated
@cyrusknopf

Copy link
Copy Markdown
Contributor

Also any thoughts on how this would work w.r.t SET?

@LucLabarriere

Copy link
Copy Markdown
Contributor Author

Also any thoughts on how this would work w.r.t SET?

That's a good question, I would say:

  • The PropertyTypeSet is the representation of the properties a node has in the current DataPart, it's not related to deletes or updates
  • If a node has a new property in a later datapart, it gets a dedicated PropertyTypeSet in the new datapart (say property type id 5):
    • Datapart 0: set = [0, 1, 2]
    • Datapart 1: set = [5]

For deletions, we will have to have some sort of tombstones on property types, our delete the node itself ?

@LucLabarriere
LucLabarriere force-pushed the query/feature/propertyTypes branch 2 times, most recently from 0042e00 to 8cf6250 Compare March 27, 2026 08:21
Comment thread storage/columns/Functions.h Outdated
Comment thread storage/columns/Functions.h Outdated
Comment thread storage/columns/Functions.h Outdated
class PropertyTypesFunction {
public:
using ResultType = std::string;
std::unordered_set<PropertyTypeID> _seen;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need to even deduplicate with an unordered_set here? Why can't we just give the property types of each NodeID regardless of repetitions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is mostly for the SET command and it depends how it will be implemented. If we just append modifications in newer dataparts, a node will have multiple definitions of the property and the property type ID will appear multiple times.

SET n.displayName = "hello"
COMMIT
SET n.displayName = "world"

displayName is defined twice (in two dataparts), hence the need to deduplicate. That will depend on the actual implementation of SET

Comment thread storage/properties/PropertyTypeSet.h Outdated
Comment thread storage/properties/PropertyTypeTrie.cpp Outdated
Comment thread storage/properties/PropertyTypeTrie.h Outdated
@LucLabarriere
LucLabarriere force-pushed the query/feature/propertyTypes branch 3 times, most recently from 8a3d5a8 to 91dce0e Compare April 2, 2026 09:50
@LucLabarriere
LucLabarriere force-pushed the query/feature/propertyTypes branch from 91dce0e to dd9468f Compare April 2, 2026 12:07
@cyrusknopf

Copy link
Copy Markdown
Contributor

Looks nice, I think it works well with the current SET implementation, currently you can't delete properties anyway, so whilst it might need a rethink to support that, so will the SET itself.

It is worth adding a unit test for testing the rebase behaviour. I have tested the below manually and it appears to work:

change new
change new

checkout change-1
match (n) where n.name = "Cyrus" set n.age = 23, n.new_prop1 = false
change submit

checkout change-0
match (n) where n.name = "Remy" set n.new_prop2 = true
change submit

match (n) return n.name, n.propertyTypes()

i.e. both changes create a new property, and they should be rebased correctly so that Remy has new_prop2 and I have new_prop1. We need this case and other similar examples in ChangeQueriesTest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants