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

fix match step range #5216

Merged
merged 5 commits into from
Jan 11, 2023
Merged

Conversation

nevermore3
Copy link
Contributor

@nevermore3 nevermore3 commented Jan 6, 2023

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

close https://github.com/vesoft-inc/nebula-ent/issues/2045

Description:

match (v:player)-[e:like*2]->(b) where id(v) == 'Tony Parker' AND (b)-[*1..2]->(v) return distinct e

before

|  8 | Traverse       | 7            |                | outputVar: {                                        |
|    |                |              |                |   "colNames": [                                     |
|    |                |              |                |     "b",                                            |
|    |                |              |                |     "__VAR_1"                                       |
|    |                |              |                |   ],                                                |
|    |                |              |                |   "type": "DATASET",                                |
|    |                |              |                |   "name": "__Traverse_8"                            |
|    |                |              |                | }                                                   |
|    |                |              |                | inputVar: __Argument_7                              |
|    |                |              |                | space: 6                                            |
|    |                |              |                | dedup: true                                         |
|    |                |              |                | limit: -1                                           |
|    |                |              |                | filter:                                             |
|    |                |              |                | orderBy: []                                         |
|    |                |              |                | src: id($-.b)                                       |
|    |                |              |                | edgeTypes: []                                       |
|    |                |              |                | edgeDirection: OUT_EDGE                             |
|    |                |              |                | vertexProps:                                        |
|    |                |              |                | edgeProps: [                                        |
|    |                |              |                |   {                                                 |
|    |                |              |                |     "props": [                                      |
|    |                |              |                |       "_dst",                                       |
|    |                |              |                |       "_type",                                      |
|    |                |              |                |       "_rank"                                       |
|    |                |              |                |     ],                                              |
|    |                |              |                |     "type": 12                                      |
|    |                |              |                |   },                                                |
|    |                |              |                |   {                                                 |
|    |                |              |                |     "type": 10,                                     |
|    |                |              |                |     "props": [                                      |
|    |                |              |                |       "_dst",                                       |
|    |                |              |                |       "_type",                                      |
|    |                |              |                |       "_rank"                                       |
|    |                |              |                |     ]                                               |
|    |                |              |                |   },                                                |
|    |                |              |                |   {                                                 |
|    |                |              |                |     "type": 11,                                     |
|    |                |              |                |     "props": [                                      |
|    |                |              |                |       "_dst",                                       |
|    |                |              |                |       "_type",                                      |
|    |                |              |                |       "_rank"                                       |
|    |                |              |                |     ]                                               |
|    |                |              |                |   }                                                 |
|    |                |              |                | ]                                                   |
|    |                |              |                | statProps:                                          |
|    |                |              |                | exprs:                                              |
|    |                |              |                | random: false                                       |
|    |                |              |                | steps: 0..140233306976544                           |
|    |                |              |                | vertex filter:                                      |
|    |                |              |                | edge filter:                                        |
|    |                |              |                | if_track_previous_path: false                       |
|    |                |              |                | first step filter:                                  |

After

|  8 | Traverse       | 7            |                | outputVar: {                                        |
|    |                |              |                |   "colNames": [                                     |
|    |                |              |                |     "b",                                            |
|    |                |              |                |     "__VAR_1"                                       |
|    |                |              |                |   ],                                                |
|    |                |              |                |   "type": "DATASET",                                |
|    |                |              |                |   "name": "__Traverse_8"                            |
|    |                |              |                | }                                                   |
|    |                |              |                | inputVar: __Argument_7                              |
|    |                |              |                | space: 11                                           |
|    |                |              |                | dedup: true                                         |
|    |                |              |                | limit: -1                                           |
|    |                |              |                | filter:                                             |
|    |                |              |                | orderBy: []                                         |
|    |                |              |                | src: id($-.b)                                       |
|    |                |              |                | edgeTypes: []                                       |
|    |                |              |                | edgeDirection: OUT_EDGE                             |
|    |                |              |                | vertexProps:                                        |
|    |                |              |                | edgeProps: [                                        |
|    |                |              |                |   {                                                 |
|    |                |              |                |     "type": 17,                                     |
|    |                |              |                |     "props": [                                      |
|    |                |              |                |       "_dst",                                       |
|    |                |              |                |       "_type",                                      |
|    |                |              |                |       "_rank"                                       |
|    |                |              |                |     ]                                               |
|    |                |              |                |   },                                                |
|    |                |              |                |   {                                                 |
|    |                |              |                |     "props": [                                      |
|    |                |              |                |       "_dst",                                       |
|    |                |              |                |       "_type",                                      |
|    |                |              |                |       "_rank"                                       |
|    |                |              |                |     ],                                              |
|    |                |              |                |     "type": 15                                      |
|    |                |              |                |   },                                                |
|    |                |              |                |   {                                                 |
|    |                |              |                |     "props": [                                      |
|    |                |              |                |       "_dst",                                       |
|    |                |              |                |       "_type",                                      |
|    |                |              |                |       "_rank"                                       |
|    |                |              |                |     ],                                              |
|    |                |              |                |     "type": 16                                      |
|    |                |              |                |   }                                                 |
|    |                |              |                | ]                                                   |
|    |                |              |                | statProps:                                          |
|    |                |              |                | exprs:                                              |
|    |                |              |                | random: false                                       |
|    |                |              |                | steps: 1..2                                         |
|    |                |              |                | vertex filter:                                      |
|    |                |              |                | edge filter:                                        |
|    |                |              |                | if_track_previous_path: false                       |
|    |                |              |                | first step filter:                                  |
|    |                |              |                | tag filter:                                         |

The steps in traverse used to be in the pointer format, and it was shallow copied when copying, causing the content pointed to by the steps to be released early

How do you solve it?

Special notes for your reviewer, ex. impact of this fix, design document, etc:

Checklist:

Tests:

  • Unit test(positive and negative cases)
  • Function test
  • Performance test
  • N/A

Affects:

  • Documentation affected (Please add the label if documentation needs to be modified.)
  • Incompatibility (If it breaks the compatibility, please describe it and add the label.)
  • If it's needed to cherry-pick (If cherry-pick to some branches is required, please label the destination version(s).)
  • Performance impacted: Consumes more CPU/Memory

Release notes:

Please confirm whether to be reflected in release notes and how to describe:

ex. Fixed the bug .....

@nevermore3 nevermore3 force-pushed the fix_match_step_range branch from 1f469c0 to dc46934 Compare January 6, 2023 10:46
@nevermore3 nevermore3 added ready-for-testing PR: ready for the CI test ready for review labels Jan 6, 2023
@nevermore3 nevermore3 force-pushed the fix_match_step_range branch 3 times, most recently from 519e2f9 to 81bc281 Compare January 10, 2023 02:42
@@ -46,7 +46,7 @@ struct NodeInfo {

struct EdgeInfo {
bool anonymous{false};
MatchStepRange* range{nullptr};
Copy link
Contributor

Choose a reason for hiding this comment

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

As I known, this AST is released when query executed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't it in the memory pool?

Copy link
Contributor

@Shylock-Hg Shylock-Hg Jan 10, 2023

Choose a reason for hiding this comment

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

All AST will be hold by QueryInstance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AST information will be transferred to Sentence in validate phase. the sentence is saved in the querycontext during the execution phase,. The current problem is the wild pointer caused by the release of the temporary variable

Copy link
Contributor

Choose a reason for hiding this comment

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

Better change

std::vector<MatchPath> pathPreds;
to std::vector<MatchPath*>, we assume lifetime of AST is available before complete query execution.

@codecov-commenter
Copy link

Codecov Report

Base: 76.77% // Head: 78.66% // Increases project coverage by +1.89% 🎉

Coverage data is based on head (81bc281) compared to base (ff4cb8c).
Patch coverage: 71.81% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5216      +/-   ##
==========================================
+ Coverage   76.77%   78.66%   +1.89%     
==========================================
  Files        1109     1110       +1     
  Lines       82965    82984      +19     
==========================================
+ Hits        63694    65277    +1583     
+ Misses      19271    17707    -1564     
Impacted Files Coverage Δ
src/clients/meta/MetaClient.h 92.30% <ø> (ø)
src/codec/RowWriterV2.cpp 84.90% <0.00%> (+0.64%) ⬆️
src/common/base/Arena.cpp 95.00% <0.00%> (+4.52%) ⬆️
src/common/base/Status.cpp 57.83% <0.00%> (-2.17%) ⬇️
src/common/base/Status.h 89.09% <0.00%> (-3.37%) ⬇️
src/common/datatypes/Value.cpp 74.77% <0.00%> (+0.36%) ⬆️
src/common/datatypes/ValueOps-inl.h 60.90% <ø> (ø)
src/common/expression/ArithmeticExpression.cpp 91.66% <0.00%> (+3.66%) ⬆️
src/common/expression/Expression.cpp 52.94% <0.00%> (+0.46%) ⬆️
src/common/expression/LabelAttributeExpression.h 77.41% <0.00%> (+6.83%) ⬆️
... and 355 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

yixinglu
yixinglu previously approved these changes Jan 10, 2023
@Sophie-Xie Sophie-Xie merged commit bb4f6c6 into vesoft-inc:master Jan 11, 2023
@nevermore3 nevermore3 deleted the fix_match_step_range branch January 11, 2023 05:37
Sophie-Xie added a commit that referenced this pull request Jan 28, 2023
* use smart pointer change raw pointer

* fix error

* fix test error

* address comment

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>
Sophie-Xie added a commit that referenced this pull request Jan 29, 2023
* optimize match node label (#5176)

* revert strange return (#5183)

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* fix stderr  save error log (#5188)

* fix processor_test timeout (#5180)

* fix processor_test timeout

* ...

Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com>
Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* fix error code (#5186)

* rename the "test" space to "ngdata". (#5197)

* rename the "test" space to "ngdata".

* add ngdata

* Revise the usages of FATAL, DFATAL, LOG, DLOG. (#5181)

* Revise the usages of FATAL, DFATAL, LOG, DLOG.

* fix.

* revise dfatal.

* Meta upgrade (#5174)

* Meta upgrade
remove all fulltext index when upgrade from V3 to V3_4 because of refacting of
fulltext index

* fix bug

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* Fix pattern expression with same edge variable (#5192)

* Fix pattern expression with same edge variable

add tck

fmt

* add tck

* Fix memory leak, remove toss gflag (#5204)

* remove toss gflag

* fix memory leak

* loose wait job finish time

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* Add max_sessions_per_ip_per_user to default config file (#5207)

* minor bug for adminTaskManager (#5195)

* modify jobmanager ut (#5175)

* modify jobmanager ut

* add expired ut

* avoid recover expired job

* add ut

* address review

* move status

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* Add more match test cases on paths. (#5189)

* improve memtracker, add missed check & remove unnecessary thenError&tryCatch check (#5199)

* [memtracker] check code run with memoery check on all works

refine code

all code memory checked

fix lint

refine code & fix build with gcc+sanitize

* fix build break

* fix lint

* refine code

* remove debug code

* fix test fail build with debug

* fix test fail build with debug

* restore commented test

* minor

* minor

* fix bug (#5214)

* fix bug

* fix bug

Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com>

* handle rpc error task status (#5212)

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* chore: community badges refined (#5202)

* chore: community badges refined

* Update README-CN.md

* Update README-CN.md

* Update README-CN.md

remove sifou and zhihu as aligned with the team

* update linkedin URL

Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com>

* Fix extend whtie space char. (#5213)

* Fix extend whtie space char.

* Format.

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* Add lack tests of no role user. (#5196)

Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com>

* remove memtracker DLOG (#5224)

* Add tck cases for DDL (#5220)

* more TCK tests for variable pattern match clause (#5215)

* cleanup

* same src/dst for variable length pattern

* variable pattern in where clause

* variable scope tests in path pattern

* More tests

* More tests

Co-authored-by: jimingquan <mingquan.ji@vesoft.com>

* Resumed the evaluation fo vertices in AttributeExpression (UTs included) (#5229)

* add memtracker flags to conf (#5231)

* add memtracker flags to conf

* typo

* refine

* refine

* add balance job type to filter when create backup (#5228)

* add more job type to filter when create backup

* log add job

* add log before acquire snapshot lock

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* Fix update sessions when leader change happens (#5225)

* Fix udpate sessions when leader change happens

* Handle errors on the graph side

* Address comments

* Address comments

* fix match step range (#5216)

* use smart pointer change raw pointer

* fix error

* fix test error

* address comment

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* Update response message when adding schema historically existed (#5227)

* update the error code and message for checking history schemas

* update tck

* update comment

* change to log error

* fix ddl tck

* increase wait time in schema.feature

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* fix error code (#5233)

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* print memory stats default to false (#5234)

* print memory stats default to false

* update conf

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* fix bug of extract prop expr visitor (#5238)

* forbid invalid prop expr used in cypher (#5242)

* Fix mistake push down limit with skip. (#5241)

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* fix delete fulltext index (#5239)

* fix delete fulltext index

* fix es delete error
1. remove get Rowreader if op is delete
2. delete es data when value is null

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* Change the default value of session_reclaim_interval_secs to 60 seconds (#5246)

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* Enhance attribute-accessing expression to ensure self-consistency (#5230)

* Revert "Remove all UNKNOWN_PROP as a type of null. (#4907)" (#5149)

This reverts commit aa62416.

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* Enhance attribute-accessing expression to ensure self-consistency

Fix tck

Fix parser

small delete

Fix tck

tck fmt

fix ut

fix ut

Fix ut

Fix tck

Delete v.tag.prop check

Fix tck

Skip some tck cases related ngdata

add test case

Co-authored-by: Cheng Xuntao <7731943+xtcyclist@users.noreply.github.com>
Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* fix ft index of fixed string (#5251)

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* Add tck test (#5253)

* add allpath test

* add shortest path test case

* add subgraph test case

* add go test case

* add go test case

* Add more session tests (#5256)

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* Revert "do not check term for leader info by default para" (#5266)

This reverts commit 593bffc.

* modify ft index default limit size (#5260)

* modify ft index default limit size

* fix test

Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com>

* Test/yield (#5267)

* Add some tests about yield.

* Add more tests.

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* Add another cert to test CA don't match. (#5247)

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* fix baton miss reset in StorageJobExecutor (#5269)

* Report errors on where clauses in optional match queries. (#5273)

* [test case] Check DML cases (#5264)

* Check DML cases

* Add chinses char tests

Add more tests

Add mero delete edge tests

* Revert cases

* fix third party version in package.sh (#5281)

The dump_syms tool path should be match with third party version.

* Test/user (#5139)

* Add some tests about user management.

* Add tests about user roles.

* Format.

* Fix tck fixture name.

* Fix step name.

* Change step name.

---------

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* fix https (#5283)

* fix memtracker bugs during stress test on graphd and storaged (#5276)

* fix memtracker bugs during stress test on graphd and storaged

* fix lint

* fix RocksEngine memory leak of raw pointer iter

* add ENABLE_MEMORY_TRACKER build option & support adaptive limit for MemoryTracker

* delete debug log

* refine log

* refine log

* fix build

* refine error log

* print warning if memtracker is off

* fix rocksdb leak by turn off memcheck

* refine synamic-self-adaptive

* fix cmake check

* minor

* minor

* minor

* minor

* minor

* refine double equel compare

---------

Co-authored-by: jimingquan <mingquan.ji@vesoft.com>
Co-authored-by: jie.wang <38901892+jievince@users.noreply.github.com>
Co-authored-by: Harris.Chu <1726587+HarrisChu@users.noreply.github.com>
Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com>
Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com>
Co-authored-by: canon <87342612+caton-hpg@users.noreply.github.com>
Co-authored-by: Cheng Xuntao <7731943+xtcyclist@users.noreply.github.com>
Co-authored-by: hs.zhang <22708345+cangfengzhs@users.noreply.github.com>
Co-authored-by: kyle.cao <kyle.cao@vesoft.com>
Co-authored-by: Yichen Wang <18348405+Aiee@users.noreply.github.com>
Co-authored-by: liwenhui-soul <38217397+liwenhui-soul@users.noreply.github.com>
Co-authored-by: Alex Xing <90179377+SuperYoko@users.noreply.github.com>
Co-authored-by: codesigner <codesigner.huang@vesoft.com>
Co-authored-by: Wey Gu <weyl.gu@gmail.com>
Co-authored-by: shylock <33566796+Shylock-Hg@users.noreply.github.com>
Co-authored-by: pengwei.song <90180021+pengweisong@users.noreply.github.com>
Co-authored-by: haowen <19355821+wenhaocs@users.noreply.github.com>
Co-authored-by: George <58841610+Shinji-IkariG@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants