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.

Sign up for GitHub

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

Jump to bottom

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
Merged

Support for SIMILAR TO syntax, change Like and ILike to Expr variants, allow escape char for like/ilike #569

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 •
edited
Loading

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
This was referenced Aug 10, 2022
Closed
Merged
@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
alamb
alamb reviewed Aug 11, 2022
View reviewed changes
Copy link
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.

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
@@ -438,6 +461,72 @@ impl fmt::Display for Expr {
high
),
Expr::BinaryOp { left, op, right } => write!(f, "{} {} {}", left, op, right),
Expr::Like {
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems to match https://www.postgresql.org/docs/current/functions-matching.html so 👍

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

Choose a reason for hiding this comment

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

👍

tests/sqlparser_common.rs
BinaryOperator::Like
},
right: Box::new(Expr::Value(Value::SingleQuotedString("%a".to_string()))),
Expr::Like {
Copy link
Contributor

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?

tests/sqlparser_common.rs Outdated
select.selection.unwrap()
);

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

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

@alamb
Copy link
Contributor

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
alamb
alamb approved these changes Aug 11, 2022
View reviewed changes
Copy link
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.

LGTM -- thanks again @ayushdg !

@alamb
Copy link
Contributor

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 apache:main Aug 11, 2022
@andygrove andygrove mentioned this pull request Aug 13, 2022
Closed
@alamb alamb mentioned this pull request Aug 15, 2022
Closed
@andygrove andygrove mentioned this pull request Aug 17, 2022
Merged
MazterQyou pushed a commit to cube-js/sqlparser-rs that referenced this pull request Dec 1, 2022
@ayushdg @MazterQyou
Support for SIMILAR TO syntax, change Like and ILike to Expr …
53716f1 
…variants, allow escape char for like/ilike (apache#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
@ayushdg @MazterQyou
Support for SIMILAR TO syntax, change Like and ILike to Expr …
512a18b 
…variants, allow escape char for like/ilike (apache#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
@TennyZhuang TennyZhuang mentioned this pull request Mar 22, 2024
Merged
9 tasks
mcheshkov pushed a commit to cube-js/sqlparser-rs that referenced this pull request Sep 2, 2024
@ayushdg @mcheshkov
Support for SIMILAR TO syntax, change Like and ILike to Expr …
21f076c 
…variants, allow escape char for like/ilike (apache#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

Can drop this after rebase on commit f07063f " Support for SIMILAR TO syntax, change Like and ILike to Expr variants, allow escape char for like/ilike (apache#569)", first released in 0.21.0
mcheshkov pushed a commit to cube-js/sqlparser-rs that referenced this pull request Sep 3, 2024
@ayushdg @mcheshkov
Support for SIMILAR TO syntax, change Like and ILike to Expr …
9c3d6a2 
…variants, allow escape char for like/ilike (apache#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

Can drop this after rebase on commit f07063f " Support for SIMILAR TO syntax, change Like and ILike to Expr variants, allow escape char for like/ilike (apache#569)", first released in 0.21.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alamb alamb alamb approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ayushdg @coveralls @alamb