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 query adjacent by filtering conditions with limit #1057

Merged
merged 9 commits into from
Jul 2, 2020
Merged

Conversation

javeme
Copy link
Contributor

@javeme javeme commented Jun 24, 2020

fix #1052

Change-Id: I11075c56e89a57cbf8e599a2e5fa6c0122663f6d

@codecov
Copy link

codecov bot commented Jun 24, 2020

Codecov Report

Merging #1057 into master will increase coverage by 9.31%.
The diff coverage is 77.19%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1057      +/-   ##
============================================
+ Coverage     59.92%   69.23%   +9.31%     
- Complexity     4571     5463     +892     
============================================
  Files           328      328              
  Lines         26400    26425      +25     
  Branches       3773     3777       +4     
============================================
+ Hits          15820    18296    +2476     
+ Misses         8857     6349    -2508     
- Partials       1723     1780      +57     
Impacted Files Coverage Δ Complexity Δ
...hugegraph/backend/serializer/TextBackendEntry.java 87.03% <ø> (+77.16%) 54.00 <0.00> (+47.00)
...u/hugegraph/backend/tx/SchemaIndexTransaction.java 79.54% <0.00%> (-3.39%) 13.00 <0.00> (+1.00) ⬇️
...om/baidu/hugegraph/task/StandardTaskScheduler.java 84.96% <0.00%> (+2.15%) 69.00 <2.00> (+8.00)
...aidu/hugegraph/backend/serializer/BytesBuffer.java 87.92% <40.00%> (+12.98%) 175.00 <3.00> (+29.00)
.../java/com/baidu/hugegraph/backend/query/Query.java 85.16% <50.00%> (+5.84%) 95.00 <1.00> (+6.00)
...u/hugegraph/traversal/optimize/HugeVertexStep.java 72.97% <88.88%> (+0.91%) 24.00 <0.00> (+6.00)
...du/hugegraph/backend/tx/GraphIndexTransaction.java 80.50% <93.33%> (+0.88%) 193.00 <4.00> (+4.00)
...egraph/backend/store/cassandra/CassandraStore.java 69.01% <100.00%> (+3.16%) 53.00 <0.00> (+3.00)
...ugegraph/backend/cache/CachedGraphTransaction.java 88.54% <100.00%> (+3.27%) 38.00 <0.00> (+3.00)
...u/hugegraph/backend/serializer/TextSerializer.java 86.96% <100.00%> (+69.76%) 98.00 <2.00> (+86.00)
... and 138 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 ac02492...7b68f90. Read the comment docs.

@javeme
Copy link
Contributor Author

javeme commented Jun 28, 2020

ci error with non rocksdb/hbase backends:

[ERROR] Tests run: 672, Failures: 1, Errors: 0, Skipped: 62, Time elapsed: 239.451 s <<< FAILURE! - in com.baidu.hugegraph.core.CoreTestSuite
[ERROR] testQueryEdgesAdjacentVerticesWithLimitAndFilterProp(com.baidu.hugegraph.core.EdgeCoreTest)  Time elapsed: 0.037 s  <<< FAILURE!
java.lang.AssertionError: expected:<11> but was:<9>
	at com.baidu.hugegraph.core.EdgeCoreTest.testQueryEdgesAdjacentVerticesWithLimitAndFilterProp(EdgeCoreTest.java:2222)

Vertex java = graph.addVertex(T.label, "book", "name", "java-" + i);
james.addEdge("authored", java, "score", i % 2);
james.addEdge("write", java, "time", "2020-6-" + i);
if (i % TX_BATCH == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

first batch only loop 1 time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not affect logic


int txCap = this.superNodeSize();
for (int i = 0; i < txCap; i++) {
Vertex james = graph.addVertex(T.label, "author", "id", 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

add duplicate vertex in every loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, to improve performance to avoid reading every time

vertices = graph.traversal().V(jeff)
.both().has("age", 61)
.limit(1).toList();
Assert.assertEquals(1, vertices.size());
Copy link
Contributor

Choose a reason for hiding this comment

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

what about limit 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

0 means none, and -1 means no limit

Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to add a limit(0) boundary test.

@javeme
Copy link
Contributor Author

javeme commented Jun 29, 2020

g.V().out("write", "look").limit(12) with cassandra order:

e[S206:Louise>611>2020-6-0>S209:java-0][206:Louise-look->209:java-0]
e[S206:Louise>611>2020-6-18>S209:java-0][206:Louise-look->209:java-0]

e[S207:11>610>2020-6-0>S209:java-0][207:11-write->209:java-0]
e[S207:11>610>2020-6-1>S209:java-1][207:11-write->209:java-1]
e[S207:11>610>2020-6-10>S209:java-10][207:11-write->209:java-10]
e[S207:11>610>2020-6-11>S209:java-11][207:11-write->209:java-11]
e[S207:11>610>2020-6-12>S209:java-12][207:11-write->209:java-12]
e[S207:11>610>2020-6-13>S209:java-13][207:11-write->209:java-13]
e[S207:11>610>2020-6-14>S209:java-14][207:11-write->209:java-14]
e[S207:11>610>2020-6-15>S209:java-15][207:11-write->209:java-15]
e[S207:11>610>2020-6-16>S209:java-16][207:11-write->209:java-16]
e[S207:11>610>2020-6-17>S209:java-17][207:11-write->209:java-17]
e[S207:11>610>2020-6-18>S209:java-18][207:11-write->209:java-18]

rocksdb order:

e[S266:Louise>790>2020-6-0>S269:java-0][266:Louise-look->269:java-0]
e[S266:Louise>790>2020-6-18>S269:java-0][266:Louise-look->269:java-0]

e[S267:11>789>2020-6-0>S269:java-0][267:11-write->269:java-0]
e[S267:11>789>2020-6-10>S269:java-10][267:11-write->269:java-10]
e[S267:11>789>2020-6-11>S269:java-11][267:11-write->269:java-11]
e[S267:11>789>2020-6-12>S269:java-12][267:11-write->269:java-12]
e[S267:11>789>2020-6-13>S269:java-13][267:11-write->269:java-13]
e[S267:11>789>2020-6-14>S269:java-14][267:11-write->269:java-14]
e[S267:11>789>2020-6-15>S269:java-15][267:11-write->269:java-15]
e[S267:11>789>2020-6-16>S269:java-16][267:11-write->269:java-16]
e[S267:11>789>2020-6-17>S269:java-17][267:11-write->269:java-17]
e[S267:11>789>2020-6-18>S269:java-18][267:11-write->269:java-18]
e[S267:11>789>2020-6-19>S269:java-19][267:11-write->269:java-19]

rocksdb order:

e[S207:11>610>2020-6-0>S209:java-0][207:11-write->209:java-0]
e[S207:11>610>2020-6-10>S209:java-10][207:11-write->209:java-10]
e[S207:11>610>2020-6-11>S209:java-11][207:11-write->209:java-11]
e[S207:11>610>2020-6-12>S209:java-12][207:11-write->209:java-12]
e[S207:11>610>2020-6-13>S209:java-13][207:11-write->209:java-13]
e[S207:11>610>2020-6-14>S209:java-14][207:11-write->209:java-14]
e[S207:11>610>2020-6-15>S209:java-15][207:11-write->209:java-15]
e[S207:11>610>2020-6-16>S209:java-16][207:11-write->209:java-16]
e[S207:11>610>2020-6-17>S209:java-17][207:11-write->209:java-17]
e[S207:11>610>2020-6-18>S209:java-18][207:11-write->209:java-18]
e[S207:11>610>2020-6-19>S209:java-19][207:11-write->209:java-19]
e[S207:11>610>2020-6-1>S209:java-1][207:11-write->209:java-1]

e[S206:Louise>611>2020-6-0>S209:java-0][206:Louise-look->209:java-0]

@javeme
Copy link
Contributor Author

javeme commented Jun 29, 2020

pgsql error:

[ERROR] Tests run: 673, Failures: 0, Errors: 1, Skipped: 21, Time elapsed: 289.197 s <<< FAILURE! - in com.baidu.hugegraph.core.CoreTestSuite
[ERROR] testAddEdgeWithInvalidSortkey(com.baidu.hugegraph.core.EdgeCoreTest)  Time elapsed: 0.14 s  <<< ERROR!
com.baidu.hugegraph.backend.BackendException: Failed to commit
	at com.baidu.hugegraph.core.EdgeCoreTest.testAddEdgeWithInvalidSortkey(EdgeCoreTest.java:512)
Caused by: java.sql.BatchUpdateException: Batch entry 0 INSERT INTO g_oe (PROPERTIES, OTHER_VERTEX, LABEL, DIRECTION, SORT_VALUES, OWNER_VERTEX) VALUES ('{"5033":"2017-5-27\u0000"}', 'S2237:Test-Book-1', 262, -126, '2017-5-272017-5-27', 'S2235:11') ON CONFLICT (OWNER_VERTEX, DIRECTION, LABEL, SORT_VALUES, OTHER_VERTEX) DO UPDATE SET PROPERTIES = '{"5033":"2017-5-27\u0000"}', OTHER_VERTEX = 'S2237:Test-Book-1', LABEL = 262, DIRECTION = -126, SORT_VALUES = '2017-5-272017-5-27', OWNER_VERTEX = 'S2235:11' was aborted: ERROR: invalid byte sequence for encoding "UTF8": 0x00  Call getNextException to see other errors in the batch.
	at com.baidu.hugegraph.core.EdgeCoreTest.testAddEdgeWithInvalidSortkey(EdgeCoreTest.java:512)
Caused by: org.postgresql.util.PSQLException: ERROR: invalid byte sequence for encoding "UTF8": 0x00
	at com.baidu.hugegraph.core.EdgeCoreTest.testAddEdgeWithInvalidSortkey(EdgeCoreTest.java:512)

* 0x00 is NULL in UTF8(or ASCII) bytes
* 0xFF is not a valid byte in UTF8 bytes
*/
assert !Bytes.contains(bytes, STRING_ENDING_BYTE_FF) :
Copy link
Contributor

Choose a reason for hiding this comment

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

if not use 0xff at all, prefer delete it and no need to assert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to preserve the byte for future usages

@@ -1066,6 +1045,9 @@ private static ConditionQuery constructQuery(ConditionQuery query,
indexType, indexFields);
Object fieldValue = query.userpropValue(indexFields.get(0));
assert fieldValue instanceof String;
// Escape empty String to INDEX_EMPTY_SYM
Copy link
Contributor

Choose a reason for hiding this comment

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

INDEX_EMPTY_SYM -> INDEX_SYM_EMPTY

@@ -1074,11 +1056,9 @@ private static ConditionQuery constructQuery(ConditionQuery query,
case SECONDARY:
List<Id> joinedKeys = indexFields.subList(0, queryKeys.size());
String joinedValues = query.userpropValuesString(joinedKeys);
// Escape empty String to INDEX_EMPTY_SYM
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@javeme javeme force-pushed the outE-limit-bug branch 2 times, most recently from a54a985 to a9a371b Compare June 30, 2020 11:40
zhoney
zhoney previously approved these changes Jul 1, 2020
Linary
Linary previously approved these changes Jul 1, 2020
@@ -1189,7 +1195,9 @@ public static boolean matchEdgeSortKeys(ConditionQuery query,
for (int i = sortKeys.size(); i > 0; i--) {
List<Id> subFields = sortKeys.subList(0, i);
if (queryKeys.containsAll(subFields)) {
return true;
if (queryKeys.size() == subFields.size() || !matchAll) {
Copy link
Contributor

Choose a reason for hiding this comment

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

add a comment later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -124,9 +124,15 @@ public HugeVertexStep(final VertexStep<E> originVertexStep) {
// Query by sort-keys
if (withEdgeCond && edgeLabels.length > 0) {
TraversalUtil.fillConditionQuery(conditions, query, graph);
if (!GraphTransaction.matchEdgeSortKeys(query, graph)) {
if (!GraphTransaction.matchEdgeSortKeys(query, false, graph)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest rename to partialMatchEdgeSortKeys

fix #1052

Change-Id: I11075c56e89a57cbf8e599a2e5fa6c0122663f6d
Change-Id: I34de752d7ba8a0eede3d98d29a8a841803f1fc80
Change-Id: Iba776e18809f77f592283e664e93689f53ab8719
Change-Id: I36096d6723e8ddf121abb1684b6f5b93e2698c78
Change-Id: I63f8c56463314fa3a14b565f2619154951948b45
Change-Id: If28ca2dca496821eb36a1fc678b1827b4aa08588
Change-Id: Iba78468738f25f1c0bc40817f56c9597e66d165a
Change-Id: I81bcb8efb41597860416f92f53eb8ca2b769cd5d
Change-Id: I57af518e3dc79b2afe4f82174c130e5a3a9ffc7a
@zhoney zhoney merged commit 3a7316d into master Jul 2, 2020
@zhoney zhoney deleted the outE-limit-bug branch July 2, 2020 03:11
@javeme
Copy link
Contributor Author

javeme commented Jul 2, 2020

memory ci:

[ERROR] Tests run: 673, Failures: 1, Errors: 5, Skipped: 62, Time elapsed: 442.391 s <<< FAILURE! - in com.baidu.hugegraph.core.CoreTestSuite
[ERROR] testTask(com.baidu.hugegraph.core.TaskCoreTest)  Time elapsed: 2.173 s  <<< FAILURE!
java.lang.AssertionError: Bad exception type java.lang.IllegalArgumentException(expected com.baidu.hugegraph.HugeException)
	at com.baidu.hugegraph.core.TaskCoreTest.testTask(TaskCoreTest.java:85)
[ERROR] testGremlinJobWithError(com.baidu.hugegraph.core.TaskCoreTest)  Time elapsed: 0.009 s  <<< ERROR!
java.lang.IllegalArgumentException: Can't delete incomplete task '88888' in status RUNNING, Please try to cancel the task first
	at com.baidu.hugegraph.core.TaskCoreTest.setup(TaskCoreTest.java:64)
[ERROR] testTaskWithFailure(com.baidu.hugegraph.core.TaskCoreTest)  Time elapsed: 0.006 s  <<< ERROR!
java.lang.IllegalArgumentException: Can't delete incomplete task '88888' in status RUNNING, Please try to cancel the task first
	at com.baidu.hugegraph.core.TaskCoreTest.setup(TaskCoreTest.java:64)
[ERROR] testEphemeralJob(com.baidu.hugegraph.core.TaskCoreTest)  Time elapsed: 0.004 s  <<< ERROR!
java.lang.IllegalArgumentException: Can't delete incomplete task '88888' in status RUNNING, Please try to cancel the task first
	at com.baidu.hugegraph.core.TaskCoreTest.setup(TaskCoreTest.java:64)
[ERROR] testGremlinJobWithFailure(com.baidu.hugegraph.core.TaskCoreTest)  Time elapsed: 0.002 s  <<< ERROR!
java.lang.IllegalArgumentException: Can't delete incomplete task '88888' in status RUNNING, Please try to cancel the task first
	at com.baidu.hugegraph.core.TaskCoreTest.setup(TaskCoreTest.java:64)
[ERROR] testGremlinJob(com.baidu.hugegraph.core.TaskCoreTest)  Time elapsed: 0.004 s  <<< ERROR!
java.lang.IllegalArgumentException: Can't delete incomplete task '88888' in status RUNNING, Please try to cancel the task first
	at com.baidu.hugegraph.core.TaskCoreTest.setup(TaskCoreTest.java:64)

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.

limit语句执行时机有误,导致查询结果不准确。
4 participants