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

Feature/sample go #2853

Merged
merged 44 commits into from
Sep 28, 2021
Merged

Feature/sample go #2853

merged 44 commits into from
Sep 28, 2021

Conversation

Shylock-Hg
Copy link
Contributor

@Shylock-Hg Shylock-Hg commented Sep 13, 2021

Sub job of #2602

@Shylock-Hg Shylock-Hg added the ready-for-testing PR: ready for the CI test label Sep 13, 2021
@Shylock-Hg Shylock-Hg requested review from a team September 13, 2021 09:15
@nevermore3
Copy link
Contributor

i think you should write a design document first,
And this design does not sample every step under many conditions

@Shylock-Hg
Copy link
Contributor Author

i think you should write a design document first,
And this design does not sample every step under many conditions

Multiple steps need change some plan nodes, I plan to do it later. Now just one step.

@nevermore3
Copy link
Contributor

i think you should write a design document first,
And this design does not sample every step under many conditions

Multiple steps need change some plan nodes, I plan to do it later. Now just one step.

it only work for go from vid over * yield $^.tag.name, edge.name sample[1],

It won't work with a little change, for example: go from vid over * yield $$.tag.name, go from vid over * yield edge._dst as id | go from $-.id over * yield edge.name sample[1], ....

@Shylock-Hg
Copy link
Contributor Author

i think you should write a design document first,
And this design does not sample every step under many conditions

Multiple steps need change some plan nodes, I plan to do it later. Now just one step.

it only work for go from vid over * yield $^.tag.name, edge.name sample[1],

It won't work with a little change, for example: go from vid over * yield $$.tag.name, go from vid over * yield edge._dst as id | go from $-.id over * yield edge.name sample[1], ....

With input, now sample the result of join. Do you think it's problem?

@Sophie-Xie Sophie-Xie requested review from Aiee and czpmango and removed request for a team September 16, 2021 05:05
@codecov-commenter
Copy link

codecov-commenter commented Sep 26, 2021

Codecov Report

Merging #2853 (69d0277) into master (dddf54e) will decrease coverage by 0.70%.
The diff coverage is 69.87%.

❗ Current head 69d0277 differs from pull request most recent head 93a9bdd. Consider uploading reports for the commit 93a9bdd to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2853      +/-   ##
==========================================
- Coverage   85.47%   84.76%   -0.71%     
==========================================
  Files        1229     1236       +7     
  Lines      110375   111025     +650     
==========================================
- Hits        94341    94114     -227     
- Misses      16034    16911     +877     
Impacted Files Coverage Δ
src/clients/meta/MetaClient.h 92.30% <ø> (ø)
src/common/utils/NebulaKeyUtils.cpp 89.03% <0.00%> (-4.89%) ⬇️
src/common/utils/NebulaKeyUtils.h 78.02% <ø> (ø)
src/graph/planner/SequentialPlanner.cpp 100.00% <ø> (ø)
src/graph/planner/plan/Admin.cpp 54.23% <0.00%> (-2.40%) ⬇️
src/graph/planner/plan/PlanNode.h 80.00% <ø> (ø)
src/graph/service/PermissionCheck.cpp 83.33% <ø> (+1.93%) ⬆️
src/kvstore/KVEngine.h 100.00% <ø> (ø)
src/kvstore/RocksEngine.cpp 81.79% <0.00%> (-1.73%) ⬇️
src/kvstore/RocksEngine.h 66.66% <0.00%> (-25.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 073fb37...93a9bdd. Read the comment docs.

nevermore3
nevermore3 previously approved these changes Sep 27, 2021
czpmango
czpmango previously approved these changes Sep 27, 2021
@Shylock-Hg Shylock-Hg linked an issue Sep 28, 2021 that may be closed by this pull request
tests/tck/features/go/SampleLimit.feature Show resolved Hide resolved
src/graph/planner/ngql/GoPlanner.h Show resolved Hide resolved
src/graph/planner/ngql/GoPlanner.cpp Outdated Show resolved Hide resolved
@Shylock-Hg Shylock-Hg dismissed stale reviews from czpmango and nevermore3 via eed0664 September 28, 2021 02:44
@CPWstatic CPWstatic merged commit f64aa07 into vesoft-inc:master Sep 28, 2021
@Shylock-Hg Shylock-Hg deleted the feature/sample-go branch February 9, 2022 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc affected PR: improvements or additions to documentation ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Sample
6 participants