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

Fixed unary minus integer bug and support to take function call as vid #429

Merged
merged 8 commits into from
Jul 2, 2019
Merged

Fixed unary minus integer bug and support to take function call as vid #429

merged 8 commits into from
Jul 2, 2019

Conversation

dutor
Copy link
Contributor

@dutor dutor commented May 24, 2019

As title.

Now, you can use function call expression where a vertex id is required, like INSERT VERTEX player(name, age) VALUES hash("Tim Duncan"): ("Tim Duncan", 42)
close #563

@dutor dutor added the ready-for-testing PR: ready for the CI test label Jun 5, 2019
@nebula-community-bot
Copy link
Member

Unit testing failed.

@nebula-community-bot
Copy link
Member

Unit testing passed.

@nebula-community-bot
Copy link
Member

Unit testing failed.

@whitewum
Copy link
Contributor

remember add to nGQL.md after check in

whitewum
whitewum previously approved these changes Jun 20, 2019
@@ -403,7 +410,7 @@ TEST(Parser, InsertEdge) {
{
GQLParser parser;
std::string query = "INSERT EDGE NO OVERWRITE transfer(amount, time) "
"VALUES 12345->54321@1537408527:(3.75, 1537408527)";
"VALUES 12345->-54321@1537408527:(3.75, 1537408527)";
auto result = parser.parse(query);
ASSERT_TRUE(result.ok()) << result.status();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

try -1->-2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's your point?

Copy link
Contributor

Choose a reason for hiding this comment

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

a negtive vid to another negtive vid. recommend to cover this simple situation.

laura-ding
laura-ding previously approved these changes Jun 26, 2019
Copy link
Contributor

@laura-ding laura-ding 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 dismissed stale reviews from laura-ding and whitewum via 2d8ae80 June 26, 2019 09:41
@nebula-community-bot
Copy link
Member

Unit testing passed.

1 similar comment
@nebula-community-bot
Copy link
Member

Unit testing passed.

laura-ding
laura-ding previously approved these changes Jun 27, 2019
Copy link
Contributor

@laura-ding laura-ding 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 removed the ready-for-testing PR: ready for the CI test label Jul 1, 2019
laura-ding
laura-ding previously approved these changes Jul 1, 2019
Copy link
Contributor

@laura-ding laura-ding left a comment

Choose a reason for hiding this comment

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

well done

@laura-ding
Copy link
Contributor

close #563

// (Even though we already implicitly add `YIELD edge_type._dst AS id`)
// In such case, the input ref could be simply written as `$-'.
// TODO(dutor) In future, we should remove the implicitly added YIELD clause.
static const std::string defaultCol("id");
Copy link
Contributor

Choose a reason for hiding this comment

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

How about giving this default in parser, like this:

| INPUT_REF {
    auto *label = new std::string("id");
    $$ = new InputPropertyExpression(label);
}

@@ -138,7 +138,8 @@ Expression::decode(folly::StringPiece buffer) noexcept {
std::string InputPropertyExpression::toString() const {
std::string buf;
buf.reserve(64);
buf += "$-.";
buf += "$-";
buf += ".";
Copy link
Contributor

Choose a reason for hiding this comment

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

Check out this.

@dutor dutor added the ready-for-testing PR: ready for the CI test label Jul 2, 2019
@dutor
Copy link
Contributor Author

dutor commented Jul 2, 2019

Jenkins, go!

@nebula-community-bot
Copy link
Member

Unit testing passed.

@dutor dutor merged commit 4fd48c4 into vesoft-inc:master Jul 2, 2019
@dutor dutor deleted the unary-minus-integer branch July 2, 2019 06:00
dangleptr pushed a commit to dangleptr/nebula that referenced this pull request Jul 5, 2019
vesoft-inc#429)

* Fixed unary minus integer bug and support to take function call as vid

* Fixed crash when a non-existing column referenced in PIPE

* Addressed @laura-ding's comment

* minor fixes

* add test for unary minus

* Address comments

* Addressed @CPWstatic's comments
yixinglu pushed a commit to yixinglu/nebula that referenced this pull request Feb 16, 2020
vesoft-inc#429)

* Fixed unary minus integer bug and support to take function call as vid

* Fixed crash when a non-existing column referenced in PIPE

* Addressed @laura-ding's comment

* minor fixes

* add test for unary minus

* Address comments

* Addressed @CPWstatic's comments
tong-hao pushed a commit to tong-hao/nebula that referenced this pull request Jun 1, 2021
vesoft-inc#429)

* Fixed unary minus integer bug and support to take function call as vid

* Fixed crash when a non-existing column referenced in PIPE

* Addressed @laura-ding's comment

* minor fixes

* add test for unary minus

* Address comments

* Addressed @CPWstatic's comments
yixinglu pushed a commit to yixinglu/nebula that referenced this pull request Mar 21, 2022
* add db-upgrade V3

* use ingest

* modify upgrade args

* write data version key

* address wenhao's comment

* address some comment

* address some comment

* format

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

Co-authored-by: hs.zhang <22708345+cangfengzhs@users.noreply.github.com>
Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>
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.

[nGQL] parser bug for minus 1
5 participants