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 for SIMILAR TO syntax, change Like and ILike to Expr variants, allow escape char for like/ilike #569

Merged
merged 7 commits into from Aug 11, 2022

Conversation

ayushdg
Copy link
Contributor

@ayushdg ayushdg commented Aug 10, 2022

This pr adds support for the similar to sql operator syntax and also adds support to provide escape char to the [Not] like/ilike operators.

@coveralls
Copy link

coveralls commented Aug 10, 2022

Pull Request Test Coverage Report for Build 2842050083

  • 106 of 147 (72.11%) changed or added relevant lines in 3 files are covered.
  • 664 unchanged lines in 5 files lost coverage.
  • Overall coverage decreased (-4.6%) to 85.372%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/parser.rs 22 30 73.33%
tests/sqlparser_common.rs 57 66 86.36%
src/ast/mod.rs 27 51 52.94%
Files with Coverage Reduction New Missed Lines %
tests/sqlparser_postgres.rs 1 97.96%
tests/sqlparser_mysql.rs 2 99.65%
tests/sqlparser_common.rs 58 97.1%
src/ast/mod.rs 182 77.93%
src/parser.rs 421 83.24%
Totals Coverage Status
Change from base Build 2805421159: -4.6%
Covered Lines: 9437
Relevant Lines: 11054

💛 - Coveralls

@andygrove andygrove requested a review from alamb August 10, 2022 18:32
@alamb alamb changed the title Add support for SIMILAR TO syntax, allow escape char for like/ilike Support for SIMILAR TO syntax, change Like and ILike to Expr variants, allow escape char for like/ilike Aug 11, 2022
Copy link
Collaborator

@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.

Thanks @ayushdg -- this code looks great. Thank you for the contribution.

I think the copyright comments need to be addressed and a few more test cases added, but otherwise this PR is looking ready to go!

src/ast/mod.rs Outdated Show resolved Hide resolved
@@ -438,6 +461,72 @@ impl fmt::Display for Expr {
high
),
Expr::BinaryOp { left, op, right } => write!(f, "{} {} {}", left, op, right),
Expr::Like {
Copy link
Collaborator

Choose a reason for hiding this comment

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

src/ast/operator.rs Outdated Show resolved Hide resolved
@@ -76,10 +78,6 @@ pub enum BinaryOperator {
And,
Or,
Xor,
Like,
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

BinaryOperator::Like
},
right: Box::new(Expr::Value(Value::SingleQuotedString("%a".to_string()))),
Expr::Like {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please also extend the parse_like and parse_ilike tests to include coverage of escape_char for both LIKE and ILIKE?

select.selection.unwrap()
);

// This statement tests that LIKE and NOT LIKE have the same precedence.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment appears to be out of date

tests/sqlparser_common.rs Show resolved Hide resolved
@alamb
Copy link
Collaborator

alamb commented Aug 11, 2022

Marking this as a draft to show it is waiting some changes so I can easily filter out which PRs need review. Please mark it as "ready for review" when it is next ready.

@alamb alamb marked this pull request as draft August 11, 2022 10:47
@ayushdg ayushdg marked this pull request as ready for review August 11, 2022 18:43
Copy link
Collaborator

@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.

LGTM -- thanks again @ayushdg !

@alamb
Copy link
Collaborator

alamb commented Aug 11, 2022

I believe a new version of rust has been released so we need to update lint on master (unrelated to this change)

@alamb alamb merged commit f07063f into sqlparser-rs:main Aug 11, 2022
MazterQyou pushed a commit to cube-js/sqlparser-rs that referenced this pull request Dec 1, 2022
…variants, allow escape char for like/ilike (sqlparser-rs#569)

* Remove [not]like,[not]ilike from binary operator list

* Add like, ilike and similar as an expr variant. Also adds support for escape char to like/ilike

* Add parsing logic for similar to, update parsing logic for like/ilike

* Add tests for similar to, update tests for like/ilike

* Fix linter warnings

* remove additional copyright license from files

* Add more coverage w/wo escape char for like,ilike,similar to
MazterQyou pushed a commit to cube-js/sqlparser-rs that referenced this pull request Dec 1, 2022
…variants, allow escape char for like/ilike (sqlparser-rs#569)

* Remove [not]like,[not]ilike from binary operator list

* Add like, ilike and similar as an expr variant. Also adds support for escape char to like/ilike

* Add parsing logic for similar to, update parsing logic for like/ilike

* Add tests for similar to, update tests for like/ilike

* Fix linter warnings

* remove additional copyright license from files

* Add more coverage w/wo escape char for like,ilike,similar to
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.

None yet

3 participants