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

Disable edgelist join #5268

Merged
merged 8 commits into from
Mar 16, 2023
Merged

Conversation

nevermore3
Copy link
Contributor

@nevermore3 nevermore3 commented Jan 17, 2023

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

close #5209

Description:

How do you solve it?

Special notes for your reviewer, ex. impact of this fix, design document, etc:

Checklist:

Tests:

  • Unit test(positive and negative cases)
  • Function test
  • Performance test
  • N/A

Affects:

  • Documentation affected (Please add the label if documentation needs to be modified.)
  • Incompatibility (If it breaks the compatibility, please describe it and add the label.)
  • If it's needed to cherry-pick (If cherry-pick to some branches is required, please label the destination version(s).)
  • Performance impacted: Consumes more CPU/Memory

Release notes:

Please confirm whether to be reflected in release notes and how to describe:

ex. Fixed the bug .....

@nevermore3 nevermore3 added ready-for-testing PR: ready for the CI test ready for review labels Jan 17, 2023
@nevermore3 nevermore3 added the doc affected PR: improvements or additions to documentation label Jan 17, 2023
@nevermore3 nevermore3 force-pushed the disable_edgelist_join branch from cf65225 to e20aa9a Compare January 17, 2023 08:37
src/parser/MatchPath.cpp Outdated Show resolved Hide resolved
@abby-cyber abby-cyber self-assigned this Jan 29, 2023
@nevermore3 nevermore3 force-pushed the disable_edgelist_join branch from e20aa9a to ad3e221 Compare January 31, 2023 08:07
yixinglu
yixinglu previously approved these changes Jan 31, 2023
Copy link
Contributor

@yixinglu yixinglu left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

Codecov Report

Base: 77.63% // Head: 78.73% // Increases project coverage by +1.10% 🎉

Coverage data is based on head (1a1f222) compared to base (52a177e).
Patch coverage: 59.52% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5268      +/-   ##
==========================================
+ Coverage   77.63%   78.73%   +1.10%     
==========================================
  Files        1110     1110              
  Lines       83382    83443      +61     
==========================================
+ Hits        64736    65702     +966     
+ Misses      18646    17741     -905     
Impacted Files Coverage Δ
src/graph/visitor/PropertyTrackerVisitor.cpp 76.42% <0.00%> (-5.88%) ⬇️
src/graph/visitor/PropertyTrackerVisitor.h 100.00% <ø> (ø)
src/meta/processors/zone/DropHostsProcessor.cpp 72.80% <20.00%> (-4.60%) ⬇️
src/meta/processors/zone/MergeZoneProcessor.cpp 85.00% <20.00%> (-2.10%) ⬇️
src/graph/visitor/PrunePropertiesVisitor.cpp 91.37% <86.66%> (-0.24%) ⬇️
src/graph/executor/algo/SubgraphExecutor.cpp 96.72% <100.00%> (+1.72%) ⬆️
src/graph/executor/query/DataCollectExecutor.cpp 93.54% <100.00%> (+0.07%) ⬆️
src/graph/validator/MatchValidator.cpp 90.18% <100.00%> (+0.01%) ⬆️
src/kvstore/raftex/RaftPart.cpp 73.52% <100.00%> (+3.97%) ⬆️
src/parser/MatchPath.h 98.84% <100.00%> (-0.57%) ⬇️
... and 85 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

src/parser/MatchPath.h Outdated Show resolved Hide resolved
@nevermore3 nevermore3 force-pushed the disable_edgelist_join branch from a571d45 to a8184cd Compare February 6, 2023 03:17
@nevermore3 nevermore3 force-pushed the disable_edgelist_join branch 2 times, most recently from db29b44 to 3744fd6 Compare March 14, 2023 07:34
@nevermore3 nevermore3 force-pushed the disable_edgelist_join branch from 17f086b to 8596974 Compare March 14, 2023 08:22
@nevermore3 nevermore3 force-pushed the disable_edgelist_join branch from 58f7ceb to e57d626 Compare March 15, 2023 02:52
@nevermore3 nevermore3 requested a review from czpmango March 15, 2023 02:55
@Sophie-Xie
Copy link
Contributor

@nevermore3 Did you discuss it with @MuYiYong?

@nevermore3
Copy link
Contributor Author

@nevermore3 Did you discuss it with @MuYiYong?

opencypher has no clear definition of this behavior, so we disable

@yixinglu yixinglu requested review from czpmango and jievince March 15, 2023 06:01
@yixinglu yixinglu merged commit c2ebc02 into vesoft-inc:master Mar 16, 2023
@nevermore3 nevermore3 deleted the disable_edgelist_join branch March 16, 2023 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc affected PR: improvements or additions to documentation ready for review ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

match query's result is incorrect
7 participants