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

RC4 Cherrypick #2031 #2042 #2025 #2013 #2038 #2051

Merged
merged 5 commits into from
Apr 2, 2020

Conversation

jude-zhu
Copy link
Contributor

@jude-zhu jude-zhu commented Apr 2, 2020

Cherrypick #2031 #2042 #2025 #2013 #2038

knightXun and others added 4 commits April 2, 2020 16:28
Co-authored-by: dutor <440396+dutor@users.noreply.github.com>
(cherry picked from commit 3423df1)
Co-authored-by: dutor <440396+dutor@users.noreply.github.com>
(cherry picked from commit eea7407)
* Sampling from all edges.

* Update test for sampling multi edges.

* Testing both inbound and outbound.

* Fix clang compiling problem.

* Update the sampling.

Co-authored-by: dutor <440396+dutor@users.noreply.github.com>
(cherry picked from commit 8810ecc)
* Fix the error when vertex/edge not exist if update.

* Optimize the atomic error logical.

* Fix one alignment.

Co-authored-by: dutor <440396+dutor@users.noreply.github.com>
(cherry picked from commit 9fc4e7f)
@Shylock-Hg Shylock-Hg added the ready-for-testing PR: ready for the CI test label Apr 2, 2020
@jude-zhu jude-zhu requested review from darionyaphet and removed request for boshengchen April 2, 2020 08:58
@@ -142,7 +142,7 @@ nebula::cpp2::SupportedType TraverseExecutor::calculateExprType(Expression* exp)
case Expression::kInputProp: {
auto* propExp = static_cast<const AliasPropertyExpression*>(exp);
const auto* propName = propExp->prop();
if (inputs_ == nullptr) {
if (inputs_ == nullptr || !inputs_->hasData()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if inputs_ == nullptr , Is it will leads to crash by inputs_->hasData() ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think won't because the short circuit ||

Copy link
Contributor

@bright-starry-sky bright-starry-sky left a comment

Choose a reason for hiding this comment

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

LGTM.

…at. (vesoft-inc#2038)

* Correct issue 2009 which the timestamp default value for edge not treat.

* Add the cover cases.

* Fix one typo.

Co-authored-by: dutor <440396+dutor@users.noreply.github.com>
(cherry picked from commit 4aa9217)
@jude-zhu jude-zhu changed the title RC4 Cherrypick #2031 #2042 #2025 #2013 RC4 Cherrypick #2031 #2042 #2025 #2013 #2038 Apr 2, 2020
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (v1.0.0-rc4@2ea95f3). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             v1.0.0-rc4    #2051   +/-   ##
=============================================
  Coverage              ?   86.73%           
=============================================
  Files                 ?      641           
  Lines                 ?    61104           
  Branches              ?        0           
=============================================
  Hits                  ?    52998           
  Misses                ?     8106           
  Partials              ?        0           

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 2ea95f3...4df895a. Read the comment docs.

Copy link
Contributor

@Shylock-Hg Shylock-Hg left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@dutor dutor left a comment

Choose a reason for hiding this comment

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

LGTM.

@dutor dutor merged commit d7422b6 into vesoft-inc:v1.0.0-rc4 Apr 2, 2020
@jude-zhu jude-zhu deleted the cherrymaster branch June 10, 2020 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants