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.

Sign up for GitHub

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

Jump to bottom

rewrite none_direct_dst(e) to e._dst #5287

Merged
merged 1 commit into from
Jan 30, 2023
Merged

rewrite none_direct_dst(e) to e._dst #5287

merged 1 commit into from
Jan 30, 2023

Conversation

jievince
Copy link
Contributor

@jievince jievince commented Jan 30, 2023 •
edited by wey-gu
Loading

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

Close #5286

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

@jievince jievince added the ready-for-testing PR: ready for the CI test label Jan 30, 2023
yixinglu
yixinglu previously approved these changes Jan 30, 2023
View reviewed changes
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.

Well Done!

Shylock-Hg
Shylock-Hg reviewed Jan 30, 2023
View reviewed changes
tests/tck/features/optimizer/PushFilterDownTraverseRule.feature Outdated
@@ -73,24 +153,25 @@ Feature: Push Filter down Traverse rule
Scenario: push filter down Traverse with complex filter
When profiling query:
"""
MATCH (v:player)-[e:like]->(v2) WHERE v.player.age != 35 and (e.likeness + 100) != 199
Copy link
Contributor

Choose a reason for hiding this comment

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

Why change this?

Copy link
Contributor Author

@jievince jievince Jan 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Adds and none_direct_dst(e) in ["Tony Parker", "Tim Duncan", "Yao Ming"] to make the filter more complex

Copy link
Contributor

@Shylock-Hg Shylock-Hg Jan 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We should avoid to change origin test cases, better to add new one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, fixed.

@wey-gu
Copy link
Contributor

wey-gu commented Jan 30, 2023

This should be doc related, right?

@jievince
Copy link
Contributor Author

This should be doc related, right?

Yes

@yixinglu yixinglu merged commit 0e95568 into vesoft-inc:master Jan 30, 2023
@wey-gu wey-gu added the doc affected PR: improvements or additions to documentation label Jan 30, 2023
@wey-gu
Copy link
Contributor

wey-gu commented Jan 30, 2023

quote @jievince 's description to none_direct_dst for context of those have no idea on it.

dst()函数返回的是考虑边的方向的终点id, none_direct_dst返回的的是不考虑边方向的终点id, 也就是edge 的编码的的四元祖(_src, _dst, _rank, _type)中的_dst字段.

@wey-gu wey-gu mentioned this pull request Feb 4, 2023
Closed
@Sophie-Xie Sophie-Xie removed the doc affected PR: improvements or additions to documentation label Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yixinglu yixinglu yixinglu approved these changes

@Shylock-Hg Shylock-Hg Shylock-Hg approved these changes

@czpmango czpmango Awaiting requested review from czpmango

@codesigner codesigner Awaiting requested review from codesigner

@xtcyclist xtcyclist Awaiting requested review from xtcyclist

@nevermore3 nevermore3 Awaiting requested review from nevermore3

Assignees
No one assigned
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Filter where none_direct_dst(e) in ["Tony Parker", "Tim Duncan"] is not pushed down
5 participants
@jievince @wey-gu @yixinglu @Shylock-Hg @Sophie-Xie