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

Fix override vertex/edge on some backends #820

Merged

zhoney merged 4 commits into master from override-vertex

Feb 13, 2020

Contributor

Linary commented Jan 8, 2020

Fix #792

Change-Id: Iad461c6742773bd001a1768f83479e1ab429e272

Linary force-pushed the override-vertex branch from 4318b9d to 891d974 Compare

January 9, 2020 09:45

codecov bot commented Jan 9, 2020 •

edited

Codecov Report

Merging #820 into master will increase coverage by 2.09%.
The diff coverage is 86.25%.

@@             Coverage Diff              @@
##             master     #820      +/-   ##
============================================
+ Coverage     69.07%   71.17%   +2.09%     
- Complexity     4276     4366      +90     
============================================
  Files           286      286              
  Lines         21085    21084       -1     
  Branches       2967     2967              
============================================
+ Hits          14565    15006     +441     
+ Misses         5086     4592     -494     
- Partials       1434     1486      +52

Impacted Files	Coverage Δ	Complexity Δ
...graph/backend/store/cassandra/CassandraTables.java	`87.83% <ø> (+0.65%)`	`1 <0> (ø)`	⬇️
...aph/backend/store/cassandra/CassandraFeatures.java	`78.94% <100%> (ø)`	`15 <1> (ø)`	⬇️
...hugegraph/backend/store/rocksdb/RocksDBTables.java	`67.52% <100%> (-0.82%)`	`0 <0> (ø)`
.../backend/store/memory/InMemoryDBStoreProvider.java	`100% <100%> (ø)`	`11 <1> (ø)`	⬇️
...gegraph/backend/store/rocksdb/RocksDBFeatures.java	`84.21% <100%> (ø)`	`16 <2> (ø)`	⬇️
...egraph/backend/store/hbase/HbaseStoreProvider.java	`100% <100%> (+100%)`	`6 <1> (+6)`	⬆️
...ugegraph/backend/serializer/SerializerFactory.java	`89.65% <100%> (ø)`	`10 <0> (ø)`	⬇️
...ugegraph/backend/store/memory/InMemoryDBStore.java	`87.59% <100%> (ø)`	`31 <0> (ø)`	⬇️
.../hugegraph/backend/store/rocksdb/RocksDBTable.java	`77.31% <100%> (-0.95%)`	`33 <2> (+1)`
...u/hugegraph/backend/store/hbase/HbaseFeatures.java	`84.21% <100%> (+84.21%)`	`16 <2> (+16)`	⬆️
... and 28 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 5952dc3...8b5615e. Read the comment docs.

Linary force-pushed the override-vertex branch 3 times, most recently from ed99f47 to e180cd1 Compare

January 9, 2020 13:23

javeme requested changes

View reviewed changes

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TextSerializer.java Outdated Show resolved Hide resolved

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TextSerializer.java Show resolved Hide resolved

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TextSerializer.java Outdated Show resolved Hide resolved

hugegraph-test/src/main/java/com/baidu/hugegraph/unit/serializer/BinarySerializerTest.java Show resolved Hide resolved

Linary force-pushed the override-vertex branch from e180cd1 to 1ea3ffa Compare

January 13, 2020 12:29

javeme requested changes

View reviewed changes

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/BinarySerializer.java Show resolved Hide resolved

javeme reviewed

View reviewed changes

Contributor

javeme left a comment

also upgrade rocksdb/hbase/memory backend version

javeme reviewed

View reviewed changes

hugegraph-rocksdb/src/main/java/com/baidu/hugegraph/backend/store/rocksdb/RocksDBTables.java Show resolved Hide resolved

javeme reviewed

View reviewed changes

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/BinarySerializer.java

+                              // Parse vertex properties
+                              assert entry.columnsSize() == 1 : entry.columnsSize();
+                              this.parseVertex(col.value, vertex);
+                          }

Contributor

javeme Jan 30, 2020

diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/BinaryInlineSerializer.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/BinaryInlineSerializer.java
index b94d0608..b2481aa6 100644
--- a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/BinaryInlineSerializer.java
+++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/BinaryInlineSerializer.java
@@ -72,11 +72,13 @@ public class BinaryInlineSerializer extends BinarySerializer {
 
         // Parse all properties and edges of a Vertex
         for (BackendColumn col : entry.columns()) {
-            if (id.edge()) {
+            // NOTE: the entry id type is vertex even if entry type is edge
+            if (entry.type().isEdge()) {
                 // Parse vertex edges
                 this.parseColumn(col, vertex);
             } else {
                 // Parse vertex properties
+                assert entry.type().isVertex();
                 assert entry.columnsSize() == 1 : entry.columnsSize();
                 this.parseVertex(col.value, vertex);
             }

javeme reviewed

View reviewed changes

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/BinarySerializer.java Outdated

                       // Parse all properties and edges of a Vertex
                       for (BackendColumn col : entry.columns()) {
-                          this.parseColumn(col, vertex);
+                          if (entry.type().isEdge()) {
+                              // NOTE: the id is vertex type even if query edge

Contributor

javeme Jan 30, 2020

This bug was introduced from https://github.com/hugegraph/hugegraph/pull/680/files#diff-e46dfe3b041d128cc0a4657fe901a3b7R75

Contributor

javeme Feb 4, 2020

// NOTE: the entry id type is vertex even if entry type is edge

Linary force-pushed the override-vertex branch from 79d079a to 0c08d92 Compare

February 4, 2020 03:58

javeme requested changes

View reviewed changes

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/BinarySerializer.java

+                              // Parse vertex edges
+                              this.parseColumn(col, vertex);
+                          } else {
+                              // Parse vertex properties

Contributor

javeme Feb 4, 2020

assert entry.type().isVertex();

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/BinarySerializer.java Outdated

                       // Parse all properties and edges of a Vertex
                       for (BackendColumn col : entry.columns()) {
-                          this.parseColumn(col, vertex);
+                          if (entry.type().isEdge()) {
+                              // NOTE: the id is vertex type even if query edge

Contributor

javeme Feb 4, 2020

// NOTE: the entry id type is vertex even if entry type is edge

hugegraph-rocksdb/src/main/java/com/baidu/hugegraph/backend/store/rocksdb/RocksDBTables.java Show resolved Hide resolved

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TextSerializer.java Outdated Show resolved Hide resolved

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TextSerializer.java Show resolved Hide resolved

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TextSerializer.java Outdated Show resolved Hide resolved

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TextSerializer.java Outdated Show resolved Hide resolved

Linary added 4 commits

February 6, 2020 10:40


          Fix override vertex/edge on some backends

40e146c

Fix #792

Change-Id: Iad461c6742773bd001a1768f83479e1ab429e272


          some improve

5d0f6c7

Change-Id: I7458be4ad22bcbd178ed25ecd6805b6b4c2c0ada


          Update vertex get and backend version

4c7bfbd

Change-Id: I16dfd36d8eddedd6ca6abf9199cf32723ad7e268


          some improve

8b5615e

Change-Id: I4c374d557947187f3469d802f8f0d363541fa70a

Linary force-pushed the override-vertex branch from 0c08d92 to 8b5615e Compare

February 6, 2020 03:31

javeme approved these changes

View reviewed changes

zhoney approved these changes

View reviewed changes

zhoney merged commit 5692844 into master

zhoney deleted the override-vertex branch

February 13, 2020 05:46

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment