Skip to content

support create trigger#756

Closed
zidaye wants to merge 2 commits into
apache:mainfrom
zidaye:feature/create_trigger
Closed

support create trigger#756
zidaye wants to merge 2 commits into
apache:mainfrom
zidaye:feature/create_trigger

Conversation

@zidaye
Copy link
Copy Markdown
Contributor

@zidaye zidaye commented Dec 9, 2022

@coveralls
Copy link
Copy Markdown

coveralls commented Dec 9, 2022

Pull Request Test Coverage Report for Build 3662567008

  • 229 of 284 (80.63%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.1%) to 86.161%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/ast/mod.rs 52 66 78.79%
src/parser.rs 84 125 67.2%
Files with Coverage Reduction New Missed Lines %
src/ast/mod.rs 1 77.74%
Totals Coverage Status
Change from base Build 3641238188: -0.1%
Covered Lines: 12763
Relevant Lines: 14813

💛 - Coveralls

Copy link
Copy Markdown
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @zidaye -- this PR appears to have some conflicts. Can you please resolve them?

I also think condition should be an Expr

Comment thread src/parser.rs
Comment on lines +2358 to +2384
let condition = if self.parse_keyword(Keyword::WHEN) {
self.expect_token(&Token::LParen)?;
let mut lparen_count = 1;
let mut condition_str = String::new();
loop {
if lparen_count == 0 {
break;
}
if let Some(next_token) = self.next_token_no_skip() {
match &next_token.token {
Token::LParen => lparen_count += 1,
Token::RParen => lparen_count -= 1,
Token::EOF => {
return self.expected(" `)` ", TokenWithLocation::wrap(Token::EOF));
}
_ => {}
}
if lparen_count == 0 {
break;
}
condition_str.push_str(next_token.token.to_string().as_str());
}
}
Some(condition_str)
} else {
None
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this code would become much simpler when condition is an Expr

Something like let condition = self.parse_expr()

@alamb alamb marked this pull request as draft December 28, 2022 14:02
@alamb
Copy link
Copy Markdown
Contributor

alamb commented Dec 28, 2022

This looks very good @zidaye -- thank you. Please mark this as ready for review when it is ready for another look

@alamb
Copy link
Copy Markdown
Contributor

alamb commented Aug 17, 2023

Closing this PR as it is more than 6 months old without activity. Please reopen or make another PR if you plan to keep working on this.

@alamb alamb closed this Aug 17, 2023
@LucaCappelletti94
Copy link
Copy Markdown
Contributor

Hi - I need this feature, what is the status on this?

@alamb
Copy link
Copy Markdown
Contributor

alamb commented Jul 25, 2024

I don't think there is any update to report

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.

4 participants