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

Refactor fulltext index plan #5077

Merged
merged 8 commits into from
Dec 23, 2022
Merged

Refactor fulltext index plan #5077

merged 8 commits into from
Dec 23, 2022

Conversation

cangfengzhs
Copy link
Contributor

@cangfengzhs cangfengzhs commented Dec 19, 2022 •
edited
Loading

add FulltextIndexScan

modify fulltext index plan

support add listeener in tck

add ft index test demo in tck

fmt

fix standalone

fix standalone

fix standalone

  1. access es on create index
  2. fix host address

fix bug

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

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

@cangfengzhs cangfengzhs changed the title allow multi fulltext index on a tag/edge Refactor fulltext index plan Dec 19, 2022
@cangfengzhs
allow multi fulltext index on a tag/edge
bdb7830 
add FulltextIndexScan

modify fulltext index plan

support add listeener in tck

add ft index test demo in tck

fmt

fix standalone

fix standalone

fix standalone

1. access es on create index
2. fix host address

fix bug
@cangfengzhs
fix bug
768e899
@cangfengzhs
fix bug
ba39487
@cangfengzhs cangfengzhs added the ready-for-testing PR: ready for the CI test label Dec 23, 2022
@yixinglu
Copy link
Contributor

Good job! I really like this refactoring.

critical27
critical27 reviewed Dec 23, 2022
View reviewed changes
src/meta/processors/index/FTIndexProcessor.cpp
const auto& serviceKey = MetaKeyUtils::serviceKey(cpp2::ExternalServiceType::ELASTICSEARCH);
auto getRet = doGet(serviceKey);
if (!nebula::ok(getRet)) {
auto retCode = nebula::error(getRet);
Copy link
Contributor

Choose a reason for hiding this comment

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

Add some specific error code~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will fix it in next PR

critical27
critical27 approved these changes Dec 23, 2022
View reviewed changes
Copy link
Contributor

@critical27 critical27 left a comment

Choose a reason for hiding this comment

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

Generally LGTM

yixinglu
yixinglu approved these changes Dec 23, 2022
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.

LGTM

@yixinglu yixinglu merged commit 8e7aa68 into vesoft-inc:master Dec 23, 2022
@wey-gu wey-gu mentioned this pull request Dec 24, 2022
Closed
@cangfengzhs cangfengzhs mentioned this pull request Dec 27, 2022
Closed
@cangfengzhs cangfengzhs added the doc affected PR: improvements or additions to documentation label Dec 30, 2022
@abby-cyber abby-cyber self-assigned this Jan 17, 2023
@foesa-yang foesa-yang assigned foesa-yang and unassigned abby-cyber Jan 17, 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

@critical27 critical27 critical27 approved these changes

@jievince jievince Awaiting requested review from jievince

Assignees

@foesa-yang foesa-yang

Labels
doc affected PR: improvements or additions to documentation 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.

None yet
5 participants
@cangfengzhs @yixinglu @critical27 @foesa-yang @abby-cyber