Skip to content

Conversation

@raviks789
Copy link
Contributor

@raviks789 raviks789 commented May 9, 2022

@cla-bot cla-bot bot added the cla/signed label May 9, 2022
@raviks789 raviks789 force-pushed the fix/similar-unlike-filters branch from 2b4734a to ae75246 Compare May 9, 2022 09:25
@nilmerg
Copy link
Member

nilmerg commented May 9, 2022

Since you're adjusting many usages of Filter::equal here, I think most of them are not necessary.

Consider the query foo=bar. This is an equal comparison. There's no wildcard, so why should it be handled as a Similar condition? ipl-sql already renders Similar conditions the same as Equal conditions if there's no wildcard.

The filter renderer/parser should handle this the same I think. If there's a wildcard, it can only be a Similar or Unlike condition. If there's no wildcard, it's an Equal or Unequal condition.

This way the search bar and other components need to support all types. And can still use Equal in places where Similar is not required.

And don't forget the tests here 😉

@raviks789 raviks789 force-pushed the fix/similar-unlike-filters branch 2 times, most recently from 2807647 to 4ef4b38 Compare May 10, 2022 13:14
@raviks789 raviks789 requested a review from nilmerg May 10, 2022 13:30
@raviks789 raviks789 force-pushed the fix/similar-unlike-filters branch from 4ef4b38 to 8510953 Compare May 12, 2022 09:11
@raviks789 raviks789 requested a review from nilmerg May 12, 2022 09:18
Copy link
Member

@nilmerg nilmerg left a comment

Choose a reason for hiding this comment

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

Just some minor nitpicks, thanks so far! :)

@raviks789 raviks789 force-pushed the fix/similar-unlike-filters branch from 8510953 to 0557743 Compare May 12, 2022 10:18
@raviks789 raviks789 requested a review from nilmerg May 12, 2022 10:19
@nilmerg nilmerg added this to the v0.5.0 milestone May 20, 2022
@nilmerg nilmerg merged commit f96585e into master May 20, 2022
@nilmerg nilmerg deleted the fix/similar-unlike-filters branch May 20, 2022 06:14
@nilmerg nilmerg changed the title Replace Filter::equal and Filter::unequal with Filter::similar and Filter::unlike correspondingly Replace Filter::equal and Filter::unequal with Filter::like and Filter::unlike correspondingly May 30, 2022
@nilmerg nilmerg added the enhancement New feature or request label May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla/signed enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants