Skip to content

Multi label node - #1561

Closed
jeffreylovitz wants to merge 77 commits into
masterfrom
multi-label-node
Closed

Multi label node#1561
jeffreylovitz wants to merge 77 commits into
masterfrom
multi-label-node

Conversation

@jeffreylovitz

Copy link
Copy Markdown
Contributor

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.

@swilly22 swilly22 left a comment

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.

reviewed 50/64 files

Comment thread tests/flow/test_multi_label.py Outdated
Comment thread tests/flow/test_multi_label.py Outdated
Comment thread tests/flow/test_multi_label.py Outdated
Comment thread tests/flow/test_multi_label.py
Comment thread src/value.h
Comment thread src/arithmetic/entity_funcs/entity_funcs.c
Comment thread src/arithmetic/entity_funcs/entity_funcs.c Outdated
Comment thread src/arithmetic/entity_funcs/entity_funcs.c
Comment thread src/execution_plan/optimizations/reduce_traversal.c Outdated
Comment thread src/execution_plan/optimizations/traverse_order.c Outdated

@swilly22 swilly22 left a comment

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.

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);

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 a much nicer solution, good idea!

@swilly22 swilly22 left a comment

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.

great addition

@@ -5,21 +5,21 @@
*/

#include "bulk_insert.h"

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.

Please add tests for bulk loading multi-label nodes

swilly22
swilly22 previously approved these changes Oct 12, 2021
@AviAvni

AviAvni commented Oct 13, 2021

Copy link
Copy Markdown
Contributor

Closed for #1988

@AviAvni AviAvni closed this Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants