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

Use vertex key only when use_vertex_key is on #4716

Merged
merged 6 commits into from
Oct 13, 2022

Conversation

critical27
Copy link
Contributor

@critical27 critical27 commented Oct 12, 2022

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

Description:

PRD of vertex key changed again... Here is the new version of logic:

  1. No matter use_vertex_key is on or off, the query result should be consistent with 3.1.
  2. If use_vertex_key is on, we will use the vertex key to detect whether the vertex exists, which is the logic in 3.X version.
  3. If use_vertex_key is off, we will prefix scan by vertex to detect whether the vertex exists, which will bring performance regression
  4. We will insert the vertex only when use_vertex_key is on

By default, use_vertex_key is off

How do you solve it?

This PR reverts all related changes before (#4685 #4680 #4675 #4629), and apply the logic above.

Related changes:

  1. So the upgrade tool is necessary again, so bring it back.
  2. The TCK cases keeps same as before Rollback vertex key #4629

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

Maybe review the last commit is enough

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

@critical27 critical27 added the ready-for-testing PR: ready for the CI test label Oct 12, 2022
@critical27 critical27 changed the title Rollback Use vertex key only when use_vertex_key is on Oct 12, 2022
@Shylock-Hg
Copy link
Contributor

Add test

match (v:player)-[:like]->(t) return v.player.name, t.team.name

and

match (v) return v.bachelor.name limit 3

@critical27
Copy link
Contributor Author

match (v:player)-[:like]->(t) return v.player.name, t.team.name
passed

match (v) return v.bachelor.name limit 3
need to test in ent version

@critical27 critical27 merged commit 3a2912d into vesoft-inc:master Oct 13, 2022
@critical27 critical27 deleted the rollback branch October 13, 2022 02:22
@Shylock-Hg Shylock-Hg added the cherry-pick-v3.3 PR: need cherry-pick to this version label Oct 13, 2022
Sophie-Xie pushed a commit that referenced this pull request Oct 13, 2022
* Revert "insert vertex key when only vertex and flag is set (#4685)"

This reverts commit fff82a6.

* Revert "only write vertex key when flag is set or explictly insert vertex (#4680)"

This reverts commit a5bed33.

* Revert "fix bug (#4675)"

This reverts commit cccc014.

* Revert "Rollback vertex key (#4629)"

This reverts commit 545137a.

* wtf

* tck format
Sophie-Xie added a commit that referenced this pull request Oct 13, 2022
* Use vertex key only when use_vertex_key is on (#4716)

* Revert "insert vertex key when only vertex and flag is set (#4685)"

This reverts commit fff82a6.

* Revert "only write vertex key when flag is set or explictly insert vertex (#4680)"

This reverts commit a5bed33.

* Revert "fix bug (#4675)"

This reverts commit cccc014.

* Revert "Rollback vertex key (#4629)"

This reverts commit 545137a.

* wtf

* tck format

* fix job status when all finished (#4717)

* fix job name from zone balance to data balance

* address wenhui's comment

* fix create index if not exists behavior (#4705)

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>
Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com>

Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com>
Co-authored-by: Alex Xing <90179377+SuperYoko@users.noreply.github.com>
Co-authored-by: kyle.cao <kyle.cao@vesoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-v3.3 PR: need cherry-pick to this version ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants