fix(explore): Add int and bool regex pattern #13621
Conversation
superset/db_engine_specs/postgres.py
Outdated
| ( | ||
| re.compile(r"^int.*", re.IGNORECASE), | ||
| INTEGER(), | ||
| utils.GenericDataType.NUMERIC, | ||
| ), | ||
| ( | ||
| re.compile(r"^bool.*", re.IGNORECASE), | ||
| BOOLEAN(), | ||
| utils.GenericDataType.BOOLEAN, | ||
| ), |
There was a problem hiding this comment.
I think these should be put in BaseEngineSpec (to replace integer and boolean respectively), as they're both fairly commonly used type names.
There was a problem hiding this comment.
This is specific to Postgres Dialect https://github.com/apache/superset/pull/13621/files#diff-8cec2642bdfebf02492bfb132c78133c229f409661a4e56c8ea96868a1afd63fR37
so keeping them separate like this might be better idea unless you think it's not needed?
There was a problem hiding this comment.
I know INT is at least used in SQL Server and a synonym on Snowflake, and I don't know of any commonly used type names that would cause a collision with the shorter regexes, so I still feel like these should be in the base engine spec.
There was a problem hiding this comment.
Makes sense, will make a change
tests/db_engine_specs/crate_tests.py
Outdated
| assert CrateEngineSpec.convert_dttm("TIMESTAMP", dttm) == "1546398245678.9" | ||
| assert CrateEngineSpec.convert_dttm("TIMESTAMP", dttm) == "1546394645678.9" |
There was a problem hiding this comment.
This needs to be changed back to UTC (I need to look into how to make this always render the timestamp as UTC despite local timezone).
Codecov Report
@@ Coverage Diff @@
## master #13621 +/- ##
==========================================
- Coverage 77.05% 71.12% -5.94%
==========================================
Files 911 829 -82
Lines 46380 41512 -4868
Branches 5615 4332 -1283
==========================================
- Hits 35740 29524 -6216
- Misses 10500 11988 +1488
+ Partials 140 0 -140
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
nikolagigic
force-pushed
the
fix/postgres_regex_strings
branch
from
bdef61d to
11c8766
Compare
villebro
changed the title
* fix postgres regex string for bool and int * move regex from postgres to bae * move regex from postgres to base
mistercrunch
added
🏷️ bot
SUMMARY
Adding postgres regex pattern for integer and boolean type columns.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
ADDITIONAL INFORMATION