-
Notifications
You must be signed in to change notification settings - Fork 233
Multi label node #1561
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
Multi label node #1561
Conversation
swilly22
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewed 50/64 files
* Fix flawed assertion in index deletion logic * Reduce KPI for updates_baseline benchmark * Address PR comments * Address PR comments Co-authored-by: Roi Lipman <swilly22@users.noreply.github.com>
* Always report run-time errors as the sole reply * Update test_timeout.py Co-authored-by: Roi Lipman <swilly22@users.noreply.github.com>
a2cd44e to
6199288
Compare
(cherry picked from commit 79c2edf)
swilly22
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure we test QGNode such that it is impossible to create a QGNode with a duplicated label
e.g. MATCH (a:A:B:A) should create a QueryGraphNode with just A and B labels
| AlgebraicExpression *ae_src = AlgebraicExpression_RemoveSource(&exps[0]); | ||
| ASSERT(AlgebraicExpression_DiagonalOperand(ae_src, 0)); | ||
|
|
||
| const char *label = AlgebraicExpression_Label(ae_src); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 This is a much nicer solution, good idea!
swilly22
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great addition
| * This file is available under the Redis Labs Source Available License Agreement | ||
| */ | ||
|
|
||
| #include "bulk_insert.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add tests for bulk loading multi-label nodes
|
Closed for #1988 |
Introduces support for multiple labels on nodes in querying and data persistence.
One element that is absent from this PR is a heuristic for determining which label operand is the best candidate for being used as the LabelScan entry-point. The sensible rule for this would be to defer to usable indexes, and that failing choose the label with the fewest associated nodes, but we don't currently have access to label matrices when these choices are being made.