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.

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

allow node bound to include 0 [*0..4] #2041

Merged
merged 1 commit into from
Sep 18, 2023
Merged

allow node bound to include 0 [*0..4] #2041

merged 1 commit into from
Sep 18, 2023

Conversation

AEsir777
Copy link
Contributor

@AEsir777 AEsir777 commented Sep 15, 2023 •
edited
Loading

  • add feature that allows range literal to include 0 (except SHORTEST/ ALL SHORTEST)
  • add test for bound node 0[*0..4]

@AEsir777 AEsir777 force-pushed the include_zero branch 2 times, most recently from 728272c to 022978f Compare September 15, 2023 18:38
@codecov
Copy link

codecov bot commented Sep 15, 2023 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.05% 🎉

Comparison is base (49850c8) 90.15% compared to head (9d38e1f) 90.21%.
Report is 17 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2041      +/-   ##
==========================================
+ Coverage   90.15%   90.21%   +0.05%     
==========================================
  Files         938      945       +7     
  Lines       33364    33561     +197     
==========================================
+ Hits        30080    30277     +197     
  Misses       3284     3284
Files Changed Coverage Δ
src/binder/bind/bind_graph_pattern.cpp 96.33% <100.00%> (+0.71%) ⬆️
...sor/operator/recursive_extend/frontier_scanner.cpp 100.00% <100.00%> (ø)

... and 182 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AEsir777 AEsir777 force-pushed the include_zero branch 2 times, most recently from 3d560ff to eabd9b9 Compare September 15, 2023 19:29
andyfengHKU
andyfengHKU approved these changes Sep 18, 2023
View reviewed changes
Copy link
Contributor

@andyfengHKU andyfengHKU left a comment

Choose a reason for hiding this comment

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

Don't forget to add some PR description.

src/processor/operator/recursive_extend/frontier_scanner.cpp Outdated
@@ -99,7 +107,7 @@ void PathScanner::initDfs(const frontier::node_rel_id_t& nodeAndRelID, size_t cu
void PathScanner::writePathToVector(RecursiveJoinVectors* vectors, sel_t& vectorPos,
sel_t& nodeIDDataVectorPos, sel_t& relIDDataVectorPos) {
assert(vectorPos < DEFAULT_VECTOR_CAPACITY);
auto nodeEntry = ListVector::addList(vectors->pathNodesVector, k - 1);
auto nodeEntry = ListVector::addList(vectors->pathNodesVector, k ? k - 1 : 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

either say k > 0 ? k - 1 : 0 or std::min(k-1, 0)

@AEsir777 AEsir777 merged commit 314934a into master Sep 18, 2023
11 checks passed
@AEsir777 AEsir777 deleted the include_zero branch September 18, 2023 16:06
@andyfengHKU andyfengHKU mentioned this pull request Oct 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andyfengHKU andyfengHKU andyfengHKU approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@AEsir777 @andyfengHKU