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 : support collection type in secondary index #1474

Merged
merged 1 commit into from
Jul 7, 2021

Conversation

jadepeng
Copy link
Contributor

@jadepeng jadepeng commented Jun 1, 2021

支持对cardinality 为list或set 的属性建search和secondary索引
fixed #893

索引定义:

        schema.propertyKey("category").asText().valueSet().create();
        schema.propertyKey("country").asText().create();

        schema.vertexLabel("soft").properties("name", "tags", "country", "category")
              .primaryKeys("name").create();

        schema.indexLabel("softByTag").onV("soft").secondary()
              .by("tags").create();
        schema.indexLabel("softByCategory").onV("soft").search()
              .by("category").create();

        graph.addVertex(T.label, "soft", "name", "hugegraph",
                        "country", "china",
                        "category", Arrays.asList("graphdb", "db"),
                        "tags", Arrays.asList("graphdb", "gremlin"));

        graph.addVertex(T.label, "soft", "name", "neo4j",
                        "country", "usa",
                        "category", Arrays.asList("graphdb", "db"),
                        "tags", Arrays.asList("graphdb", "cypher"));

        graph.addVertex(T.label, "soft", "name", "janusgraph",
                        "country", "usa",
                        "category", Arrays.asList("graphdb", "db"),
                        "tags", Arrays.asList("graphdb", "gremlin"));

查询:

        // query by single item
        vertices = graph.traversal().V().has("soft", "tags", "gremlin").toList();
        Assert.assertEquals(2, vertices.size());

        vertices = graph.traversal().V().has("soft", "tags", "graphdb").toList();
        Assert.assertEquals(3, vertices.size());

        // query by contains
        vertices = graph.traversal().V().has("soft", "tags", ConditionP.contains("gremlin")).toList();
        Assert.assertEquals(2, vertices.size());

@imbajin imbajin changed the title feat : support collection index feat : support collection type in secondary index Jun 1, 2021
@codecov
Copy link

codecov bot commented Jun 4, 2021

Codecov Report

Merging #1474 (a8d58a0) into master (a70b4e4) will increase coverage by 1.32%.
The diff coverage is 79.66%.

❗ Current head a8d58a0 differs from pull request most recent head 332ce56. Consider uploading reports for the commit 332ce56 to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1474      +/-   ##
============================================
+ Coverage     59.51%   60.84%   +1.32%     
+ Complexity     6141      527    -5614     
============================================
  Files           410      423      +13     
  Lines         33273    35095    +1822     
  Branches       4590     4982     +392     
============================================
+ Hits          19803    21353    +1550     
- Misses        11400    11597     +197     
- Partials       2070     2145      +75     
Impacted Files Coverage Δ
...in/java/com/baidu/hugegraph/schema/IndexLabel.java 80.21% <ø> (+0.64%) ⬆️
.../baidu/hugegraph/backend/query/ConditionQuery.java 85.01% <63.15%> (-1.39%) ⬇️
...du/hugegraph/backend/tx/GraphIndexTransaction.java 81.81% <85.71%> (+1.24%) ⬆️
...du/hugegraph/schema/builder/IndexLabelBuilder.java 87.46% <100.00%> (+0.11%) ⬆️
...idu/hugegraph/api/filter/AuthenticationFilter.java 44.44% <0.00%> (-10.02%) ⬇️
...om/baidu/hugegraph/auth/StandardAuthenticator.java 35.38% <0.00%> (-6.51%) ⬇️
...m/baidu/hugegraph/backend/cache/AbstractCache.java 71.15% <0.00%> (-5.85%) ⬇️
...a/com/baidu/hugegraph/auth/HugeGraphAuthProxy.java 55.95% <0.00%> (-1.71%) ⬇️
...va/com/baidu/hugegraph/auth/HugeAuthenticator.java 33.70% <0.00%> (-1.09%) ⬇️
.../java/com/baidu/hugegraph/auth/HugePermission.java 78.94% <0.00%> (-1.06%) ⬇️
... and 90 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a70b4e4...332ce56. Read the comment docs.

@imbajin
Copy link
Member

imbajin commented Jun 8, 2021

@jadepeng Seems CI test fail

image

u can also run test in local env firstly

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.

can also add some test cases to schema-test?

"%s's relation must be CONTAINS or " +
"TEXT_CONTAINS, but got %s",
"%s's relation must be CONTAINS, " +
"TEXT_CONTAINS or TEXT_CONTAINS_ANY, but got %s",
Copy link
Contributor

Choose a reason for hiding this comment

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

expect that not allow CONTAINS and TEXT_CONTAINS_ANY for search index, at least not allow TEXT_CONTAINS_ANY

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TEXT_CONTAINS_ANY 移除,search 不包含CONTAINS ,这个这里判断稍微有点麻烦,是否一定要增加?

@imbajin
Copy link
Member

imbajin commented Jun 24, 2021

due to travis's ci blocked, the building suspend for 2days, now is fine (ignore mysql/pg)

seems need rebase master to solve conflict

@jadepeng jadepeng force-pushed the collection-index branch 2 times, most recently from 0aa787a to 6d742e9 Compare June 24, 2021 13:26
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.

seems ci is failed

@jadepeng jadepeng force-pushed the collection-index branch 2 times, most recently from 0e83dcf to 3159a98 Compare June 25, 2021 07:43
javeme
javeme previously approved these changes Jun 25, 2021
imbajin
imbajin previously approved these changes Jun 28, 2021
Copy link
Member

@imbajin imbajin left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your contribution again @jadepeng

And due to some reason for code coverage with github ci (I'll try to fix it as soon as possible), this PR may suspend for a while

image

Update: fix the code coverage problem now.

@imbajin
Copy link
Member

imbajin commented Jun 30, 2021

@jadepeng could u rebase the master branch into yours to fix the code coverage CI problem? (like another pr)

@jadepeng
Copy link
Contributor Author

jadepeng commented Jul 2, 2021

@jadepeng could u rebase the master branch into yours to fix the code coverage CI problem? (like another pr)

rebased

imbajin
imbajin previously approved these changes Jul 5, 2021
@zhoney zhoney merged commit 153fa5e into apache:master Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

如何对cardinality 为list或set 的属性建索引?
4 participants