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

Fix optional of multi-match #4159

Merged
merged 3 commits into from
Apr 15, 2022
Merged

Conversation

czpmango
Copy link
Contributor

@czpmango czpmango commented Apr 14, 2022

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

#4154

Description:

How do you solve it?

The calculation of Filter before and after InnerJoin/Cartesian are equivalent if the Filter can be delay.

The calculation of Filter before and after LeftJoin are not equivalent, bcz LeftJoin always keep the left table rows.
So the correct solution is to introduce all visible variables via Argument and computes the Filter before LeftJoin.

This pr currently only fixes the case where Filter not referencing variables outside the definitions in the current match pattern.

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 .....

@czpmango czpmango added ready-for-testing PR: ready for the CI test ready for review labels Apr 14, 2022
@czpmango czpmango requested review from CPWstatic and jievince April 14, 2022 11:53
@czpmango czpmango added the cherry-pick-v3.1 PR: need cherry-pick to this version label Apr 14, 2022
@czpmango czpmango requested a review from Shylock-Hg April 15, 2022 02:44
@@ -83,11 +83,19 @@ Status MatchPlanner::connectMatchPlan(SubPlan& queryPlan, MatchClauseContext* ma
}
if (!intersectedAliases.empty()) {
if (matchCtx->isOptional) {
// connect LeftJoin match filter
if (matchCtx->where != nullptr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What will happen when filter reference variable outside current match cluase if you don't delay the filter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

MATCH (a)--(b)--(c) 
OPTIONAL MATCH (a)--(b)--(d) WHERE d.prop>c.prop
RETURN count(*) AS count

This statement is currently not fixed, count will return 0.

Copy link
Contributor

Choose a reason for hiding this comment

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

If this hotfix introduces a new bug , it's not necessary to merge. WDYT? @CPWstatic

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mean the previous hotfix or this one?

@CPWstatic CPWstatic merged commit 26f8fc8 into vesoft-inc:master Apr 15, 2022
Sophie-Xie added a commit that referenced this pull request Apr 18, 2022
* fix optional of multi-match

* format

Co-authored-by: Sophie <[email protected]>
Sophie-Xie added a commit that referenced this pull request Apr 20, 2022
* fix optional of multi-match

* format

Co-authored-by: Sophie <[email protected]>
Sophie-Xie added a commit that referenced this pull request Apr 20, 2022
* fix issue 4152 (#4158)

* Fix optional of multi-match (#4159)

* fix optional of multi-match

* format

Co-authored-by: Sophie <[email protected]>

* Fix incompatibility imported by #4116 (#4165)

* Add SaveGraphVersionProcessor to separate client version check and version saving

* Update error code

* Update error code

* optimizer path (#4162)

* optimizer multi-shortest path

* new algorithm

* fix error

* skip heartbeat for tool (#4177)

Co-authored-by: Sophie <[email protected]>

* Fix/null pattern expression input (#4180)

* Move input rows of Traverse and AppendVertices.

* Avoid skip validate pattern expression with aggregate.

* Fix case.

* Revert "Move input rows of Traverse and AppendVertices."

This reverts commit 7fd1d38.

Co-authored-by: Sophie <[email protected]>

* fix wrong space key after dropping hosts (#4182)

Co-authored-by: Sophie <[email protected]>

* fix vertex is missing from snapshot (#4189)

Co-authored-by: Sophie <[email protected]>

* Expression is stateful to store the result of evaluation, so we can't… (#4190)

* Expression is stateful to store the result of evaluation, so we can't share it inter threads.

* Fix defef nullptr.

Co-authored-by: jie.wang <[email protected]>
Co-authored-by: kyle.cao <[email protected]>
Co-authored-by: Yichen Wang <[email protected]>
Co-authored-by: jimingquan <[email protected]>
Co-authored-by: Doodle <[email protected]>
Co-authored-by: shylock <[email protected]>
Co-authored-by: liwenhui-soul <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-v3.1 PR: need cherry-pick to this version 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.

Optional match in master(2022.04.13-nightly) became non-optional logcially
5 participants