You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/08/23 08:07:45 UTC
[GitHub] [superset] zhaoyongjie opened a new pull request #16396: fix: add percent sign in like/ilike clause
zhaoyongjie opened a new pull request #16396:
URL: https://github.com/apache/superset/pull/16396
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
Add percent sign in ends of like/ilike clause
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### After
<img width="1247" alt="image" src="https://user-images.githubusercontent.com/2016594/130412309-f450294d-0245-4d8a-9d14-2ada2125c7dd.png">
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
1. goto explore page
2. select table viz
3. select a column to groupby, select a metric
4. select column to filter control, and select `LIKE` operator in filter modal, input some chars to operand input area.
5. click run button
6. obvious SQL, percent sign in like clause
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16396:
URL: https://github.com/apache/superset/pull/16396#issuecomment-941849023
Ephemeral environment shutdown and build artifacts deleted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mistercrunch commented on pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #16396:
URL: https://github.com/apache/superset/pull/16396#issuecomment-941848758
Let's close this, `LIKE` is used for many more operations than contains! We could clarify/teach about the `LIKE` usage in the modal. Even if we created a "starts with" (and similar), we could use that as an opportunity to teach the user about LIKE and wildcard usage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] junlincc commented on pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #16396:
URL: https://github.com/apache/superset/pull/16396#issuecomment-920195170
/testenv up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #16396:
URL: https://github.com/apache/superset/pull/16396#issuecomment-903556295
# [Codecov](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16396](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ed17426) into [master](https://codecov.io/gh/apache/superset/commit/970d7627797fe2b96f797e413b27f09484b80c7b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (970d762) will **decrease** coverage by `0.21%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head ed17426 differs from pull request most recent head 84d79fe. Consider uploading reports for the commit 84d79fe to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16396 +/- ##
==========================================
- Coverage 76.64% 76.42% -0.22%
==========================================
Files 1000 1000
Lines 53489 53489
Branches 6816 6816
==========================================
- Hits 40996 40880 -116
- Misses 12257 12373 +116
Partials 236 236
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.52% <0.00%> (+0.03%)` | :arrow_up: |
| postgres | `81.55% <0.00%> (ø)` | |
| presto | `?` | |
| python | `81.63% <0.00%> (-0.43%)` | :arrow_down: |
| sqlite | `81.15% <0.00%> (-0.04%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `88.04% <0.00%> (-1.66%)` | :arrow_down: |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `89.86% <ø> (-0.13%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.49%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `88.00% <0.00%> (-0.39%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.14% <0.00%> (-0.26%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [970d762...84d79fe](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] zhaoyongjie commented on pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on pull request #16396:
URL: https://github.com/apache/superset/pull/16396#issuecomment-920640004
@junlincc @jinghua-qa
Thanks for your input!
> 1. It looks to me the LIKE operator is not case sensitive
Operator `like` depend on database spec. Some DB treat like is case insensitive. for instance: sqlite. https://sqlite.org/faq.html#q18
> 2. I think this change makes the return in a wider range than the user expected? For example, i am trying to return any name starting with 'Ma' but then the name contains 'ma' all return. And feel choosing operator 'Like' will be different than writing custom SQL using 'Like'
Actually, a wider range is `like` operator meaning instead of `equal` operator. some BI will introduce more operators like `starts with`, `ends with` and `contains` instead of `like`
<img width="499" alt="image" src="https://user-images.githubusercontent.com/2016594/133567004-868c8c06-afca-4045-bf22-2e6a693c8a77.png">
> 3, when input filter value with string only, user need to scroll to the bottom of the list to find the custom input value, which is inconvinient.
need to improve in `Select component project`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16396:
URL: https://github.com/apache/superset/pull/16396#issuecomment-920198463
@junlincc Ephemeral environment spinning up at http://54.188.236.148:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mistercrunch closed pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
mistercrunch closed pull request #16396:
URL: https://github.com/apache/superset/pull/16396
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16396:
URL: https://github.com/apache/superset/pull/16396#issuecomment-903556295
# [Codecov](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16396](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ed17426) into [master](https://codecov.io/gh/apache/superset/commit/970d7627797fe2b96f797e413b27f09484b80c7b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (970d762) will **decrease** coverage by `0.21%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head ed17426 differs from pull request most recent head 84d79fe. Consider uploading reports for the commit 84d79fe to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16396 +/- ##
==========================================
- Coverage 76.64% 76.42% -0.22%
==========================================
Files 1000 1000
Lines 53489 53489
Branches 6816 6816
==========================================
- Hits 40996 40880 -116
- Misses 12257 12373 +116
Partials 236 236
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.52% <0.00%> (+0.03%)` | :arrow_up: |
| postgres | `81.55% <0.00%> (ø)` | |
| presto | `?` | |
| python | `81.63% <0.00%> (-0.43%)` | :arrow_down: |
| sqlite | `81.15% <0.00%> (-0.04%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `88.04% <0.00%> (-1.66%)` | :arrow_down: |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `89.86% <ø> (-0.13%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.49%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `88.00% <0.00%> (-0.39%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/16396/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.14% <0.00%> (-0.26%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [970d762...84d79fe](https://codecov.io/gh/apache/superset/pull/16396?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] zhaoyongjie commented on a change in pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #16396:
URL: https://github.com/apache/superset/pull/16396#discussion_r693984172
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1232,9 +1232,9 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
elif op == utils.FilterOperator.LESS_THAN_OR_EQUALS.value:
where_clause_and.append(sqla_col <= eq)
elif op == utils.FilterOperator.LIKE.value:
- where_clause_and.append(sqla_col.like(eq))
+ where_clause_and.append(sqla_col.like(f"%{eq}%"))
Review comment:
It seems we need more discussion about filters. Some more design may be needed here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on a change in pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #16396:
URL: https://github.com/apache/superset/pull/16396#discussion_r693882334
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1232,9 +1232,9 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
elif op == utils.FilterOperator.LESS_THAN_OR_EQUALS.value:
where_clause_and.append(sqla_col <= eq)
elif op == utils.FilterOperator.LIKE.value:
- where_clause_and.append(sqla_col.like(eq))
+ where_clause_and.append(sqla_col.like(f"%{eq}%"))
Review comment:
As `LIKE` can do much more than just a "contains" match, and because this would be a breaking change, I'd rather prefer something that allows the user to write their own matching rule and not break backwards compatibility. Do you think it would make sense to only append the leading and trailing `%` if there are no unescaped `%` characters present in the like clause?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mistercrunch closed pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
mistercrunch closed pull request #16396:
URL: https://github.com/apache/superset/pull/16396
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mistercrunch commented on pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #16396:
URL: https://github.com/apache/superset/pull/16396#issuecomment-941848758
Let's close this, `LIKE` is used for many more operations than contains! We could clarify/teach about the `LIKE` usage in the modal. Even if we created a "starts with" (and similar), we could use that as an opportunity to teach the user about LIKE and wildcard usage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] zhaoyongjie commented on a change in pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #16396:
URL: https://github.com/apache/superset/pull/16396#discussion_r693757594
##########
File path: superset/utils/core.py
##########
@@ -472,6 +472,8 @@ def cast_to_boolean(value: Any) -> bool:
False
>>> cast_to_boolean('False')
False
+ >>> cast_to_boolean('True')
Review comment:
bycatch: It doesn't relate to this issue. But it is simple enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] junlincc commented on pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #16396:
URL: https://github.com/apache/superset/pull/16396#issuecomment-920362561
all good points! @jinghua-qa
did some research, it does seem like result match should be case sensitive.
please also optimize the result orders bring the most relevant to the top. thanks!
@zhaoyongjie @villebro
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16396:
URL: https://github.com/apache/superset/pull/16396#issuecomment-941849023
Ephemeral environment shutdown and build artifacts deleted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] jinghua-qa commented on pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on pull request #16396:
URL: https://github.com/apache/superset/pull/16396#issuecomment-920349103
I tested in the test env, and I have the following questions:
1, It looks to me the LIKE operator is not case sensitive
<img width="1790" alt="Screen Shot 2021-09-15 at 1 17 08 PM" src="https://user-images.githubusercontent.com/81597121/133503695-073e45b6-785d-4a5f-838b-a8ebade36176.png">
2, I think this change makes the return in a wider range than the user expected? For example, i am trying to return any name starting with 'Ma' but then the name contains 'ma' all return. And feel choosing operator 'Like' will be different than writing custom SQL using 'Like'
3, when input filter value with string only, user need to scroll to the bottom of the list to find the custom input value, which is inconvinient.
https://user-images.githubusercontent.com/81597121/133503132-7288eca0-b323-4a36-925d-c70b880750cf.mov
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on pull request #16396: fix: add percent sign in like/ilike clause
Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #16396:
URL: https://github.com/apache/superset/pull/16396#issuecomment-920672125
I'm leaning in the direction of adding three new operators for more business-like users. I'd propose the following:
- keep the current `LIKE` and `LIKE (case insensitive)` operators as they are now. The target audience for these are mainly power users who want to write their own SQL based LIKE filters (let's not forget the `_` single character wildcard effect which is also supported in `LIKE`). Keeping these unchanged would also ensure we don't do a breaking change.
- Add new operators: "Contains", "Starts with" and "Ends with" (`'%str%'`, `'str%'` and `'%str'` respectively) that will escape underscores, do **case insensitive** matching when writing all lowercase (e.g. "ma") and **case sensitive** when writing mixed case (e.g. "Ma"). These would be primarily geared towards less technical business users. The reason for deciding when to do case sensitive vs insensitive matching is to keep the basic UX as simple as possible and avoid having to add separate case sensitive and insensitive variants of these operators.
Someone will undoubtedly want to do case sensitive all lowercase matching, but for those cases they should probably just learn to use the full `LIKE` operator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org