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 giving wrong column names when union has only right result. #786

Open
wants to merge 5 commits into
base: master
from

Conversation

@CPWstatic
Copy link
Contributor

commented Aug 15, 2019

Fix #774

@CPWstatic

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2019

Jenkins go!

@nebula-community-bot

This comment has been minimized.

Copy link
Member

commented Aug 15, 2019

Unit testing failed.

@jude-zhu jude-zhu requested review from dutor, monadbobo, whitewum and laura-ding and removed request for dutor and monadbobo Aug 15, 2019

@CPWstatic

This comment has been minimized.

Copy link
Contributor Author

commented Aug 16, 2019

Jenkins go!

@nebula-community-bot

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

Unit testing failed.

1 similar comment
@nebula-community-bot

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

Unit testing failed.

@nebula-community-bot

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

Unit testing passed.

src/graph/GoExecutor.cpp Outdated Show resolved Hide resolved
src/graph/InterimResult.cpp Outdated Show resolved Hide resolved
}
}
index->rows_.emplace_back(std::move(row));
++rowIter;
}

return index;
return std::move(index);

This comment has been minimized.

Copy link
@wadeliuyi

wadeliuyi Aug 16, 2019

Contributor

is there any compile error when not add std::move? take care about return with std::move what like it say:
https://www.ibm.com/developerworks/community/blogs/5894415f-be62-4bc0-81c5-3956e82276f3/entry/RVO_V_S_std_move?lang=en

This comment has been minimized.

Copy link
@CPWstatic

CPWstatic Aug 19, 2019

Author Contributor

This is a known compile error in Jenkins when we use Status with unique_ptr but it works well on our offline machine when there is no std::move. This might cause by the compiler.

}
auto result = std::make_unique<InterimResult>(std::move(colNames));
result->setInterim(std::move(rsWriter));
return std::move(result);

This comment has been minimized.

Copy link
@wadeliuyi

wadeliuyi Aug 16, 2019

Contributor

ditto

src/graph/InterimResult.h Outdated Show resolved Hide resolved
@nebula-community-bot

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

Unit testing passed.

@jude-zhu jude-zhu requested a review from wadeliuyi Aug 29, 2019

@@ -76,8 +76,9 @@ void FetchExecutor::setupResponse(cpp2::ExecutionResponse &resp) {
}

void FetchExecutor::onEmptyInputs() {
auto outputs = std::make_unique<InterimResult>(std::move(resultColNames_));

This comment has been minimized.

Copy link
@laura-ding

laura-ding Aug 30, 2019

Contributor

put this sentence in if (onResult_) { }

@jude-zhu jude-zhu requested a review from laura-ding Sep 3, 2019

@CPWstatic CPWstatic force-pushed the CPWstatic:fix_union branch from 05b6d4b to 308e49e Sep 10, 2019

@nebula-community-bot

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

Unit testing passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.