fix: support null value for gremlin test #2061

z7658329 · 2022-12-27T06:06:01Z

fix sub-issue 4 #2058

imbajin · 2022-12-27T11:12:54Z

should handle the correspond tests together

z7658329 · 2022-12-28T03:28:30Z

should handle the correspond tests together

ok

javeme · 2022-12-28T15:00:36Z

hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeElement.java

@@ -430,9 +430,6 @@ public static final ElementKeys classifyKeys(Object... keyValues) {
                throw Element.Exceptions
                      .providedKeyValuesMustHaveALegalKeyOnEvenIndices();
            }
-            if (val == null) {
-                throw Property.Exceptions.propertyDoesNotExist();


seems need to skip the null value here with continue, also add the comment for why

javeme · 2022-12-28T15:01:11Z

hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeProperty.java


        this.owner = owner;
        this.pkey = pkey;
-        this.value = pkey.validValueOrThrow(value);
+        this.value = pkey.validValue(value);


we can keep the origin code if the null value is skipped?

hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeElement.java

javeme · 2022-12-28T16:26:36Z

hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeProperty.java


        this.owner = owner;
        this.pkey = pkey;
-        this.value = pkey.validValueOrThrow(value);
+        this.value = pkey.validValue(value);


note also need to update HugeEdge.property(String key, V value) to add null value check (the same as HugeVertex.property(String key, V value)):

if (value == null) { this.removeProperty(propertyKey.id()); return EmptyProperty.instance(); }

codecov · 2022-12-30T03:44:27Z

Codecov Report

Merging #2061 (cba6559) into master (f49b2b8) will decrease coverage by 0.67%.
The diff coverage is 35.29%.

@@             Coverage Diff              @@
##             master    #2061      +/-   ##
============================================
- Coverage     68.67%   68.00%   -0.68%     
+ Complexity      979      676     -303     
============================================
  Files           479      479              
  Lines         39652    39655       +3     
  Branches       5576     5577       +1     
============================================
- Hits          27231    26967     -264     
- Misses         9778    10045     +267     
  Partials       2643     2643

Impacted Files	Coverage Δ
.../java/org/apache/hugegraph/schema/PropertyKey.java	`76.97% <ø> (ø)`
...va/org/apache/hugegraph/structure/HugeElement.java	`73.63% <0.00%> (+0.33%)`	⬆️
.../java/org/apache/hugegraph/structure/HugeEdge.java	`75.66% <16.66%> (-0.68%)`	⬇️
...ava/org/apache/hugegraph/structure/HugeVertex.java	`78.67% <50.00%> (-0.84%)`	⬇️
...a/org/apache/hugegraph/structure/HugeProperty.java	`45.83% <100.00%> (-2.17%)`	⬇️
...raph/backend/store/postgresql/PostgresqlStore.java	`0.00% <0.00%> (-100.00%)`	⬇️
...raph/backend/store/postgresql/PostgresqlTable.java	`0.00% <0.00%> (-97.11%)`	⬇️
...kend/store/postgresql/PostgresqlStoreProvider.java	`0.00% <0.00%> (-94.92%)`	⬇️
...aph/backend/store/postgresql/PostgresqlTables.java	`0.00% <0.00%> (-87.37%)`	⬇️
...backend/store/postgresql/PostgresqlSerializer.java	`0.00% <0.00%> (-80.00%)`	⬇️
... and 13 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

z7658329 · 2022-12-30T03:47:46Z

hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeEdge.java

@@ -202,6 +204,11 @@ public <V> Property<V> property(String key, V value) {
        E.checkArgument(this.label.properties().contains(propertyKey.id()),
                        "Invalid property '%s' for edge label '%s'",
                        key, this.label());
+        if (value == null) {
+            this.removeProperty(propertyKey.id());


if remove null value's property, may cause tinkerpop ut failed ?

follow the tinkerpop ci action tests with release-1.0.0

imbajin · 2022-12-30T05:24:57Z

gremlin test still failed in:

g.V(new Object[0]).hasLabel("person", new String[0]).property("name", (Object)null, new Object[0]);

fix it in release-1.0.0 branch now

javeme · 2022-12-30T08:12:39Z

hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeEdge.java

@@ -364,6 +369,7 @@ public void vertices(HugeVertex owner, HugeVertex other) {
        if (ownerLabel.equals(this.label.sourceLabel())) {
            this.vertices(true, owner, other);
        } else {
+            // TODO: why when compare the label but ignore the result?


maybe missing assert

javeme · 2022-12-30T08:16:35Z

hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeEdge.java

@@ -202,6 +204,11 @@ public <V> Property<V> property(String key, V value) {
        E.checkArgument(this.label.properties().contains(propertyKey.id()),
                        "Invalid property '%s' for edge label '%s'",
                        key, this.label());
+        if (value == null) {


also update HugeVertex.property(String key, V value)

javeme · 2023-01-14T14:48:10Z

@imbajin please cherry pick changes from the release branch?

imbajin · 2023-01-14T14:52:12Z

@imbajin please cherry pick changes from the release branch?

I'm on it, soon

Update: done

hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeEdge.java

hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeVertex.java

support null value

2cf8981

z7658329 changed the title ~~support null value~~ fix TinkerPop unit test : support null value Dec 27, 2022

Merge branch 'master' into feat/fix_tinkerpop_ut

e68b4d1

imbajin changed the title ~~fix TinkerPop unit test : support null value~~ fix: support null value for gremlin test Dec 27, 2022

javeme reviewed Dec 28, 2022

View reviewed changes

imbajin added 2 commits December 29, 2022 17:11

Merge branch 'master' into feat/fix_tinkerpop_ut

ef0c949

address comment

deefd9d

imbajin self-assigned this Dec 30, 2022

imbajin added bug Something isn't working ci-cd Build or deploy labels Dec 30, 2022

imbajin added this to the 1.0.0 milestone Dec 30, 2022

z7658329 commented Dec 30, 2022

View reviewed changes

Merge branch 'master' into feat/fix_tinkerpop_ut

2987c12

javeme reviewed Dec 30, 2022

View reviewed changes

zyxxoo previously approved these changes Jan 14, 2023

View reviewed changes

fix(core): adapt HugeVertex.property() with null values

87108b4

imbajin dismissed zyxxoo’s stale review via 87108b4 January 14, 2023 15:48

imbajin added 2 commits January 14, 2023 23:49

Merge branch 'master' into feat/fix_tinkerpop_ut

dcd846b

Merge branch 'master' into feat/fix_tinkerpop_ut

ebaa542

imbajin mentioned this pull request Jan 14, 2023

refact: update with release branch #2091

Merged

javeme reviewed Jan 14, 2023

View reviewed changes

hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeEdge.java Show resolved Hide resolved

imbajin reviewed Jan 15, 2023

View reviewed changes

hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeVertex.java Show resolved Hide resolved

javeme approved these changes Jan 15, 2023

View reviewed changes

Merge branch 'master' into feat/fix_tinkerpop_ut

cba6559

imbajin approved these changes Jan 16, 2023

View reviewed changes

imbajin merged commit 91a616b into apache:master Jan 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: support null value for gremlin test #2061

fix: support null value for gremlin test #2061

z7658329 commented Dec 27, 2022 •

edited by javeme

Loading

imbajin commented Dec 27, 2022

z7658329 commented Dec 28, 2022

javeme Dec 28, 2022

javeme Dec 28, 2022

javeme Dec 28, 2022

codecov bot commented Dec 30, 2022 •

edited

Loading

z7658329 Dec 30, 2022

imbajin Dec 30, 2022 •

edited

Loading

imbajin commented Dec 30, 2022 •

edited

Loading

javeme Dec 30, 2022

javeme Dec 30, 2022

javeme commented Jan 14, 2023

imbajin commented Jan 14, 2023 •

edited

Loading

fix: support null value for gremlin test #2061

fix: support null value for gremlin test #2061

Conversation

z7658329 commented Dec 27, 2022 • edited by javeme Loading

imbajin commented Dec 27, 2022

z7658329 commented Dec 28, 2022

javeme Dec 28, 2022

Choose a reason for hiding this comment

javeme Dec 28, 2022

Choose a reason for hiding this comment

javeme Dec 28, 2022

Choose a reason for hiding this comment

codecov bot commented Dec 30, 2022 • edited Loading

Codecov Report

z7658329 Dec 30, 2022

Choose a reason for hiding this comment

imbajin Dec 30, 2022 • edited Loading

Choose a reason for hiding this comment

imbajin commented Dec 30, 2022 • edited Loading

javeme Dec 30, 2022

Choose a reason for hiding this comment

javeme Dec 30, 2022

Choose a reason for hiding this comment

javeme commented Jan 14, 2023

imbajin commented Jan 14, 2023 • edited Loading

z7658329 commented Dec 27, 2022 •

edited by javeme

Loading

codecov bot commented Dec 30, 2022 •

edited

Loading

imbajin Dec 30, 2022 •

edited

Loading

imbajin commented Dec 30, 2022 •

edited

Loading

imbajin commented Jan 14, 2023 •

edited

Loading