Skip to content

fix: Native filter dynamic numeric search#24506

Merged
john-bodley merged 1 commit into
apache:masterfrom
john-bodley:john-bodley--fix-24418
Jun 23, 2023
Merged

fix: Native filter dynamic numeric search#24506
john-bodley merged 1 commit into
apache:masterfrom
john-bodley:john-bodley--fix-24418

Conversation

@john-bodley
Copy link
Copy Markdown
Member

SUMMARY

Addresses an issue with #24418 with handling LIKE vs. ILIKE.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

CI.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • 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

elif op in {
utils.FilterOperator.ILIKE,
utils.FilterOperator.LIKE,
utils.FilterOperator.ILIKE.value,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The "value" option is not required but I added this for consistency.

sqla_col = sa.cast(sqla_col, sa.String)

if utils.FilterOperator.LIKE.value:
if op == utils.FilterOperator.LIKE.value:
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oops. Previously both LIKE and ILIKE were being materialized as LIKE.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 23, 2023

Codecov Report

Merging #24506 (bb2904f) into master (e6f7c73) will not change coverage.
The diff coverage is 0.00%.

❗ Current head bb2904f differs from pull request most recent head 6df1ead. Consider uploading reports for the commit 6df1ead to get more accurate results

@@           Coverage Diff           @@
##           master   #24506   +/-   ##
=======================================
  Coverage   69.06%   69.06%           
=======================================
  Files        1901     1901           
  Lines       74019    74019           
  Branches     8116     8116           
=======================================
  Hits        51121    51121           
  Misses      20787    20787           
  Partials     2111     2111           
Flag Coverage Δ
hive 53.89% <0.00%> (ø)
mysql 79.41% <0.00%> (ø)
postgres 79.50% <0.00%> (ø)
presto 53.79% <0.00%> (ø)
python 83.49% <0.00%> (ø)
sqlite 78.00% <0.00%> (ø)
unit 54.65% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset/models/helpers.py 70.95% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@john-bodley john-bodley merged commit b9824d6 into apache:master Jun 23, 2023
john-bodley added a commit to airbnb/superset-fork that referenced this pull request Jun 23, 2023
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.0.0 First shipped in 3.0.0 labels Mar 8, 2024
vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 2024
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS 🚢 3.0.0 First shipped in 3.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants