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

Support Time-to-live in query(syntax) #330

Closed
wants to merge 5 commits into from
Closed

Support Time-to-live in query(syntax) #330

wants to merge 5 commits into from

Conversation

ayyt
Copy link
Contributor

@ayyt ayyt commented May 7, 2019

  1. support Time-to-live in syntax
  2. also extend to support other schema options
  3. improve create/alter edge for using TTL

@ayyt ayyt changed the title Support Time-to-live in query Support Time-to-live in query(syntax) May 7, 2019
@nebula-community-bot
Copy link
Member

Can one of the admins verify this patch?

items_.emplace_back(item);
}

std::vector<std::unique_ptr<SchemaOptItem>> getOpt() {
Copy link
Contributor

@zlcook zlcook May 7, 2019

Choose a reason for hiding this comment

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

How about getOpts() or opts()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch!

items_.emplace_back(item);
}

std::vector<std::unique_ptr<SchemaOptItem>> getOpt() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Returning std::vector<SchemaOptItem*> is more reasonable. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, You are right.

@@ -93,18 +244,25 @@ class CreateTagSentence final : public Sentence {
return columns_->columnSpecs();
}

std::vector<std::unique_ptr<SchemaOptItem>> getSchemaOpts() {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@@ -197,17 +363,25 @@ class AlterTagSentence final : public Sentence {
return opts_->alterTagItems();
}

std::vector<std::unique_ptr<SchemaOptItem>> getSchemaOpts() {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

}
;

alter_tag_sentence
: KW_ALTER KW_TAG LABEL alter_tag_opt_list {
$$ = new AlterTagSentence($3, $4);
: KW_ALTER KW_TAG LABEL alter_schema_opt_list alter_tag_opt_list {
Copy link
Contributor

@zlcook zlcook May 7, 2019

Choose a reason for hiding this comment

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

putting alter_tag_opt_list in front of alter_schema_opt_list seems more reasonable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

refer mysql , the order of alter_tag_opt_list and alter_schema_opt_list doesn't matter

}
{
GQLParser parser;
std::string query = "alter TAG womann ttl_duration = 50, ttl_col = age";
Copy link
Contributor

Choose a reason for hiding this comment

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

The ttl_col property should be time typed property. In order to avoid misunderstanding, using an appropriate property instead of age in the test case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. 64bit type can set ttl_col

@ayyt
Copy link
Contributor Author

ayyt commented May 8, 2019

Fixed and rebased master, please review, thx.

@ayyt
Copy link
Contributor Author

ayyt commented May 13, 2019

Dependent on PR #364, hold on before merging PR #364.

@ayyt
Copy link
Contributor Author

ayyt commented May 20, 2019

Close this PR, use new PR #403.

@ayyt ayyt closed this May 20, 2019
yixinglu added a commit to yixinglu/nebula that referenced this pull request Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants