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

format find path #2957

Merged
merged 11 commits into from
Nov 23, 2021
Merged

format find path #2957

merged 11 commits into from
Nov 23, 2021

Conversation

nevermore3
Copy link
Contributor

@nevermore3 nevermore3 commented Sep 28, 2021

must add YIELD path in findpath sentence

incompatible
KW_PATH changed from non-keyword to keyword, so disallow users to define path as a variable .
some test case :
find path from a to b over like |order by $-.path or. match path = (a)-[]->(b) return path. are not allowed

@nevermore3 nevermore3 added the doc affected PR: improvements or additions to documentation label Sep 28, 2021
@nevermore3 nevermore3 added this to the v2.6.0 milestone Sep 28, 2021
@nevermore3
Copy link
Contributor Author

subjob of #2594

@nevermore3 nevermore3 added the incompatible PR: incompatible with the recently released version label Sep 28, 2021
@nevermore3 nevermore3 added the ready-for-testing PR: ready for the CI test label Sep 28, 2021
@nevermore3 nevermore3 force-pushed the format_path branch 3 times, most recently from e0233df to 76c054e Compare September 30, 2021 06:37
src/parser/parser.yy Outdated Show resolved Hide resolved
yixinglu
yixinglu previously approved these changes Sep 30, 2021
@nevermore3 nevermore3 removed the ready-for-testing PR: ready for the CI test label Oct 1, 2021
@Sophie-Xie Sophie-Xie modified the milestones: v2.6.0, v2.7.0 Oct 9, 2021
@Sophie-Xie Sophie-Xie modified the milestones: v2.7.0, v3.0.0 Oct 15, 2021
@nevermore3 nevermore3 added the ready-for-testing PR: ready for the CI test label Oct 21, 2021
@nevermore3 nevermore3 added do not review PR: not ready for the code review yet and removed ready-for-testing PR: ready for the CI test labels Oct 22, 2021
@codecov-commenter
Copy link

codecov-commenter commented Oct 23, 2021

Codecov Report

Merging #2957 (923536b) into master (80e827f) will decrease coverage by 0.01%.
The diff coverage is 91.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2957      +/-   ##
==========================================
- Coverage   85.24%   85.22%   -0.02%     
==========================================
  Files        1289     1289              
  Lines      120035   120077      +42     
==========================================
+ Hits       102322   102337      +15     
- Misses      17713    17740      +27     
Impacted Files Coverage Δ
src/graph/optimizer/OptGroup.h 100.00% <ø> (ø)
src/graph/optimizer/Optimizer.h 100.00% <ø> (ø)
src/graph/optimizer/OptimizerUtils.cpp 94.00% <ø> (ø)
...h/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp 96.20% <ø> (ø)
src/graph/optimizer/rule/IndexScanRule.cpp 71.98% <ø> (ø)
...timizer/rule/OptimizeEdgeIndexScanByFilterRule.cpp 90.54% <ø> (ø)
...ptimizer/rule/OptimizeTagIndexScanByFilterRule.cpp 90.27% <ø> (ø)
...graph/optimizer/rule/PushFilterDownGetNbrsRule.cpp 75.51% <ø> (ø)
...imizer/rule/PushLimitDownEdgeIndexFullScanRule.cpp 89.65% <ø> (ø)
...izer/rule/PushLimitDownEdgeIndexPrefixScanRule.cpp 89.65% <ø> (ø)
... and 46 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 80e827f...923536b. Read the comment docs.

@nevermore3
Copy link
Contributor Author

subjob of #3135

CPWstatic
CPWstatic previously approved these changes Oct 26, 2021
return Status::SemanticError("Missing yield clause.");
}
if (yield->columns().size() != 1) {
return Status::SemanticError("Only support yield path");
Copy link
Contributor

@Shylock-Hg Shylock-Hg Oct 27, 2021

Choose a reason for hiding this comment

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

`YIELD path`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lower case is also ok

@@ -278,3 +278,60 @@ Feature: Integer Vid All Path
| <("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"})-[:like@0 {likeness: 95}]->("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"})<-[:like@0 {likeness: 95}]-("Tony Parker" :player{age: 36, name: "Tony Parker"})> |
| <("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"})<-[:like@0 {likeness: 80}]-("Boris Diaw" :player{age: 36, name: "Boris Diaw"})-[:like@0 {likeness: 80}]->("Tony Parker" :player{age: 36, name: "Tony Parker"})> |
Then drop the used space

Scenario: Integer Vid ALL PATH YIELD PATH
Copy link
Contributor

Choose a reason for hiding this comment

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

Add some cases to test invalid yield clause.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

alread exists in unit test

CPWstatic
CPWstatic previously approved these changes Nov 18, 2021
@CPWstatic CPWstatic merged commit 6cdca05 into vesoft-inc:master Nov 23, 2021
@nevermore3 nevermore3 deleted the format_path branch November 23, 2021 03:52
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 incompatible PR: incompatible with the recently released version ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants