villebro commented on a change in pull request #13294:
URL: https://github.com/apache/superset/pull/13294#discussion_r592803714
##########
File path: tests/db_engine_specs/mysql_tests.py
##########
@@ -70,7 +70,7 @@ def test_is_db_column_type_match(self):
("TINYINT", GenericDataType.NUMERIC),
("SMALLINT", GenericDataType.NUMERIC),
("MEDIUMINT", GenericDataType.NUMERIC),
- ("INT", GenericDataType.NUMERIC),
+ ("INTEGER", GenericDataType.NUMERIC),
Review comment:
Isn't `INT` an official datatype in Mysql?
https://dev.mysql.com/doc/refman/8.0/en/integer-types.html I think we need to
keep this here.
##########
File path: tests/db_engine_specs/mysql_tests.py
##########
@@ -89,18 +89,10 @@ def test_is_db_column_type_match(self):
("TIME", GenericDataType.TEMPORAL),
)
- for type_expectation in type_expectations:
- type_str = type_expectation[0]
- col_type = type_expectation[1]
- assert MySQLEngineSpec.is_db_column_type_match(
- type_str, GenericDataType.NUMERIC
- ) is (col_type == GenericDataType.NUMERIC)
- assert MySQLEngineSpec.is_db_column_type_match(
- type_str, GenericDataType.STRING
- ) is (col_type == GenericDataType.STRING)
- assert MySQLEngineSpec.is_db_column_type_match(
- type_str, GenericDataType.TEMPORAL
- ) is (col_type == GenericDataType.TEMPORAL)
+ for type_str, col_type in type_expectations:
+ print(">>> ", type_str)
Review comment:
assuming a leftover:
```suggestion
print(">>> ", type_str)
```
##########
File path: superset/db_engine_specs/base.py
##########
@@ -145,8 +146,87 @@ class BaseEngineSpec: # pylint:
disable=too-many-public-methods
_date_trunc_functions: Dict[str, str] = {}
_time_grain_expressions: Dict[Optional[str], str] = {}
column_type_mappings: Tuple[
- Tuple[Pattern[str], Union[TypeEngine, Callable[[Match[str]],
TypeEngine]]], ...,
- ] = ()
+ Tuple[
+ Pattern[str],
+ Union[TypeEngine, Callable[[Match[str]], TypeEngine]],
+ GenericDataType,
+ ],
+ ...,
+ ] = (
+ (
+ re.compile(r"^smallint", re.IGNORECASE),
+ types.SmallInteger(),
+ GenericDataType.NUMERIC,
+ ),
+ (
+ re.compile(r"^integer", re.IGNORECASE),
Review comment:
To make sure we match Mysql `INT` type, we could just change this to
`^INT` to match both `INT` and `INTEGER`, unless there are any known
incompatible types that could cause a collision.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
