Skip to content

Conversation

corgiboygsj
Copy link
Member

@corgiboygsj corgiboygsj commented Dec 20, 2021

  • PointerCombiner no longer implement Combiner interface.
  • The modify scheme for subclasses that implement PropertiesCombiner may not be the best.
@corgiboygsj corgiboygsj marked this pull request as draft December 20, 2021 08:01
Assert.assertEquals(properties2, properties);
}
//@Test
//public void testCombineVertex() {
Copy link
Contributor

Choose a reason for hiding this comment

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

don't need to test?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, OverwriteCombiner only can combine Value now.

@corgiboygsj corgiboygsj marked this pull request as ready for review December 23, 2021 10:25
@corgiboygsj corgiboygsj marked this pull request as draft December 23, 2021 10:35
@corgiboygsj corgiboygsj marked this pull request as ready for review December 23, 2021 10:37
@corgiboygsj corgiboygsj marked this pull request as draft December 23, 2021 10:37
@corgiboygsj corgiboygsj marked this pull request as ready for review December 23, 2021 10:57
@codecov
Copy link

codecov bot commented Dec 23, 2021

Codecov Report

Merging #169 (52ac50f) into master (e378777) will decrease coverage by 0.02%.
The diff coverage is 91.81%.

Impacted file tree graph

@@ Coverage Diff @@ ## master #169 +/- ## ============================================ - Coverage 87.36% 87.33% -0.03%  - Complexity 3000 3057 +57  ============================================ Files 314 324 +10 Lines 11261 11465 +204 Branches 977 995 +18 ============================================ + Hits 9838 10013 +175  - Misses 960 980 +20  - Partials 463 472 +9 
Impacted Files Coverage Δ
...egraph/computer/core/compute/input/EdgesInput.java 86.17% <ø> (-0.12%) ⬇️
...ugegraph/computer/core/config/ComputerOptions.java 100.00% <ø> (ø)
...u/hugegraph/computer/core/io/StreamGraphInput.java 97.43% <ø> (-0.04%) ⬇️
.../hugegraph/computer/core/io/StreamGraphOutput.java 96.96% <ø> (-0.05%) ⬇️
...r/core/sort/flusher/CombineKvInnerSortFlusher.java 100.00% <ø> (ø)
...r/core/sort/flusher/CombineKvOuterSortFlusher.java 100.00% <ø> (ø)
...ore/sort/flusher/CombineSubKvInnerSortFlusher.java 96.92% <ø> (ø)
...ore/sort/flusher/CombineSubKvOuterSortFlusher.java 100.00% <ø> (ø)
...h/computer/core/sort/flusher/InnerSortFlusher.java 0.00% <ø> (ø)
...idu/hugegraph/computer/core/combiner/Combiner.java 71.42% <50.00%> (ø)
... and 44 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 e378777...52ac50f. Read the comment docs.

input1.seek(v1.offset());
input2.seek(v2.offset());
String label1 = StreamGraphInput.readLabel(input1);
String label2 = StreamGraphInput.readLabel(input2);
Copy link
Contributor

Choose a reason for hiding this comment

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

also move label to property class?

writer.writeSubKv(out -> {
this.writeId(out, edge.targetId());
}, out -> {
this.writeLabel(out, edge.label());
Copy link
Contributor

Choose a reason for hiding this comment

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

seems to lose the label value?

@javeme javeme merged commit 8308ffd into master Dec 24, 2021
@javeme javeme deleted the improve-combiner branch December 24, 2021 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants