Fix reorder expression by nafraf · Pull Request #3078 · RedisGraph/RedisGraph · GitHub
Skip to content

Fix reorder expression - #3078

Open
nafraf wants to merge 6 commits into
masterfrom
fix-reorder-exp
Open

nafraf wants to merge 6 commits into
masterfrom
fix-reorder-exp

Conversation

@nafraf

@nafraf nafraf commented May 3, 2023

Copy link
Copy Markdown
Contributor

Hi, this patch is to fix #3058.
The idea is to don't score expressions with GhostEdge, because the reorder expression function was removing conditional variable length traverse operations.

Before applying the patch, the reordering of the expression removed the Conditional Variable Length Traverse operation and the module crashes:

127.0.0.1:6379> GRAPH.explain g "MATCH ()-[*0]->(:A)--() RETURN 0"
1) "Results"
2) "    Project"
3) "        Conditional Traverse | (@anon_1:A)->(@anon_2)"
4) "            Node By Label Scan | (@anon_0)"

But for the same pattern written backwards, there was not problem:

127.0.0.1:6379> GRAPH.explain g "MATCH ()--(:A)<-[*0]-() RETURN 0"
1) "Results"
2) "    Project"
3) "        Conditional Variable Length Traverse | (@anon_1)-[@anon_4*0..0]->(@anon_2)"
4) "            Conditional Traverse | (@anon_1)->(@anon_0)"
5) "                Node By Label Scan | (@anon_1:A)"

@nafraf
nafraf marked this pull request as ready for review May 3, 2023 20:15
@nafraf nafraf mentioned this pull request May 4, 2023
@filipecosta90

filipecosta90 commented May 12, 2023 •

Copy link
Copy Markdown
Collaborator

@codecov

codecov Bot commented Jul 20, 2023 •

Copy link
Copy Markdown

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 🎉

Comparison is base (86d6c12) 90.85% compared to head (1bffce1) 90.88%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3078      +/-   ##
==========================================
+ Coverage   90.85%   90.88%   +0.02%     
==========================================
  Files         294      294              
  Lines       30269    30269              
==========================================
+ Hits        27502    27509       +7     
+ Misses       2767     2760       -7     
Impacted Files Coverage Δ
...xecution_plan/optimizations/traverse_order_utils.c 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash due to untrue assertion AlgebraicExpression_DiagonalOperand

2 participants