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

feat: improve index hits #2314

Closed
wants to merge 1 commit into from
Closed

feat: improve index hits #2314

wants to merge 1 commit into from

Conversation

zpw-123
Copy link

@zpw-123 zpw-123 commented Sep 17, 2023

EdgeCoreTest.java与VertexCoreTest.java是部分索引的例子!

Purpose of the PR

  • close #xxx

Main Changes

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)
  • Already covered by existing tests, such as (please modify tests here).
  • Need tests and can be verified as follows:
    • xxx

Does this PR potentially affect the following parts?

  • Nope
  • Dependencies (add/update license info)
  • Modify configurations
  • The public API
  • Other affects (typed here)

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

EdgeCoreTest.java与VertexCoreTest.java是部分索引的例子!
@imbajin imbajin requested review from javeme and zyxxoo September 17, 2023 08:27
Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

Thank you very much for your PR.
Could you briefly describe the feature intent and design framework?


public MatchedIndex(SchemaLabel schemaLabel,
Set<IndexLabel> indexLabels) {
this.schemaLabel = schemaLabel;
//this.schemaLabels = new HashSet<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

remove it?

for (ConditionQuery cq : ConditionQueryFlatten.flatten(
(ConditionQuery) query, supportIn)) {
for (ConditionQuery cq: ConditionQueryFlatten.flatten(
(ConditionQuery) query, supportIn)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer to keep the old style

@@ -1405,13 +1407,127 @@ private <R> QueryList<R> optimizeQueries(Query query,
* 2.index-query result(ids after optimization), which may be empty.
*/
if (q == null) {
queries.add(this.indexQuery(cq), this.batchSize);
boolean sys = cq.syspropConditions().size() != 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

sysprop?

@@ -133,7 +133,7 @@ public Number queryNumber(Query query) {
}

@Watched(prefix = "tx")
public QueryResults<BackendEntry> query(Query query) {
protected QueryResults<BackendEntry> query(Query query) {
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure why need to mark protected

}
// Method to reorder conditions
private void excludeOnlyLabelQuery(ConditionQuery cq, QueryList queries) {
// 判断是否命中索引
Copy link
Contributor

Choose a reason for hiding this comment

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

can we translate all Chinese comments?

.collect(Collectors.toSet());

Id label = rawQuery.condition(HugeKeys.LABEL);
if (label == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer return if (label != null)

@zpw-123
Copy link
Author

zpw-123 commented Oct 6, 2023 via email

@simon824 simon824 changed the title 修改了上面的几个文件,来实现部分索引命中! feat: improve index hits Oct 27, 2023
@simon824
Copy link
Member

hi @zpw-123 , is it still in progress?

@zpw-123
Copy link
Author

zpw-123 commented Oct 27, 2023 via email

Copy link

Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label

@javeme
Copy link
Contributor

javeme commented Nov 27, 2023

前辈好 我是#2314 目前好像很多测试都没有通过,我毕竟就修改170+行,为啥这么多测试都通过不了啊! 然后实现的思路大概是:首先需要判断是否命中索引,如果部分命中索引就创建 更加宽泛的条件查询语句,后面再用过滤操作来筛选出来最后结果!

@zpw-123 总体思路是可以的,不过涉及面较广,希望能补充一下详细设计文档。先进行文档Review没问题之后,再进行代码修改。如果有什么问题欢迎通过HugeGraph微信公众号联系我们。

@javeme javeme removed the inactive label Nov 27, 2023
@imbajin imbajin marked this pull request as draft November 28, 2023 03:10
Copy link

Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants