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

fix bug traverseByLabel() not set page every loop #805

Merged
merged 4 commits into from
Jan 7, 2020
Merged

Conversation

zhoney
Copy link
Contributor

@zhoney zhoney commented Dec 30, 2019

fixed: #802 #814

Change-Id: I586032e221ebfdc3141fb9374bca23a4d5af229c

fixed: #802

Change-Id: I586032e221ebfdc3141fb9374bca23a4d5af229c
@codecov
Copy link

codecov bot commented Dec 30, 2019

Codecov Report

Merging #805 into master will increase coverage by 0.62%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #805      +/-   ##
============================================
+ Coverage     71.13%   71.75%   +0.62%     
- Complexity     4331     4381      +50     
============================================
  Files           283      283              
  Lines         20957    20964       +7     
  Branches       2957     2958       +1     
============================================
+ Hits          14908    15043     +135     
+ Misses         4568     4443     -125     
+ Partials       1481     1478       -3
Impacted Files Coverage Δ Complexity Δ
...idu/hugegraph/job/schema/RebuildIndexCallable.java 86.95% <ø> (-0.19%) 20 <0> (ø)
...baidu/hugegraph/backend/store/BackendMutation.java 85.71% <100%> (+0.27%) 32 <0> (ø) ⬇️
...m/baidu/hugegraph/backend/tx/GraphTransaction.java 82.77% <100%> (+1.09%) 235 <0> (+3) ⬆️
.../java/com/baidu/hugegraph/backend/query/Query.java 86.89% <0%> (-0.69%) 68% <0%> (ø)
...du/hugegraph/backend/tx/GraphIndexTransaction.java 81.1% <0%> (+0.26%) 180% <0%> (+2%) ⬆️
...hugegraph/backend/serializer/BinarySerializer.java 83.47% <0%> (+0.69%) 104% <0%> (+1%) ⬆️
...a/com/baidu/hugegraph/backend/query/Condition.java 78.83% <0%> (+0.82%) 31% <0%> (ø) ⬇️
.../baidu/hugegraph/backend/query/ConditionQuery.java 85.26% <0%> (+1.78%) 96% <0%> (+2%) ⬆️
.../hugegraph/backend/store/rocksdb/RocksDBStore.java 70.6% <0%> (+2.87%) 56% <0%> (+3%) ⬆️
... and 5 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 ef68289...349a919. Read the comment docs.

@@ -127,6 +127,7 @@ public GraphTransaction(HugeGraph graph, BackendStore store) {
CoreOptions.VERTEX_CHECK_CUSTOMIZED_ID_EXIST);
this.verticesCapacity = conf.get(CoreOptions.VERTEX_TX_CAPACITY);
this.edgesCapacity = conf.get(CoreOptions.EDGE_TX_CAPACITY);
this.traverseBatch = conf.get(CoreOptions.QUERY_TRAVERSE_BATCH);
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 use page_size

Assume.assumeFalse("Support query by label",
storeFeatures().supportsQueryByLabel());

initDataWithoutLabelIndex();
Copy link
Contributor

Choose a reason for hiding this comment

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

add case graph().traversal().E().hasLabel(xx)

Assume.assumeFalse("Support query by label",
storeFeatures().supportsQueryByLabel());

initDataWithoutLabelIndex();
Copy link
Contributor

Choose a reason for hiding this comment

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

add case graph().traversal().V().hasLabel(xx)

Change-Id: Ie88d6ad5633f477c87ae21878f93a0abf5727bc1
@@ -1538,10 +1539,14 @@ public void traverseEdgesByLabel(EdgeLabel label, Consumer<Edge> consumer,
SchemaLabel elemLabel = ((HugeElement) e).schemaLabel();
if (label.equals(elemLabel)) {
consumer.accept(e);
if (deleting) {
this.commit();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

call commitIf()

}
}
if (query.paging()) {
page = PageInfo.pageState(iter).toString();
query.page(page);
Copy link
Contributor

Choose a reason for hiding this comment

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

this.commit();

@@ -127,6 +127,7 @@ public GraphTransaction(HugeGraph graph, BackendStore store) {
CoreOptions.VERTEX_CHECK_CUSTOMIZED_ID_EXIST);
this.verticesCapacity = conf.get(CoreOptions.VERTEX_TX_CAPACITY);
this.edgesCapacity = conf.get(CoreOptions.EDGE_TX_CAPACITY);
this.traverseBatch = conf.get(CoreOptions.QUERY_PAGE_SIZE);
Copy link
Contributor

Choose a reason for hiding this comment

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

rename this.traverseBatch to pageSize

Change-Id: I1406c003f1f52a691f6ef018f66c738c0102d921
Change-Id: I789db8f69f76895155be6db97a11f91ce6675971
@zhoney
Copy link
Contributor Author

zhoney commented Jan 6, 2020

Also fixed #814

graph().traversal().E().hasLabel("read").toList();
}, e -> {
Assert.assertTrue(
e.getMessage().startsWith("Don't accept query by label") &&
Copy link
Contributor

Choose a reason for hiding this comment

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

use assertContains() instead and rebase on apache/incubator-hugegraph-commons#43

}, e -> {
Assert.assertTrue(
e.getMessage().startsWith("Don't accept query by label") &&
e.getMessage().endsWith("it disables label index"));
Copy link
Contributor

Choose a reason for hiding this comment

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

use assertContains() instead and rebase on apache/incubator-hugegraph-commons#43

@Linary Linary merged commit 455c8d9 into master Jan 7, 2020
@Linary Linary deleted the traverse-by-label branch January 7, 2020 03:11
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.

bug traverseByLabel() with LabelIndex disable
3 participants