You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ma...@apache.org on 2019/06/03 16:52:38 UTC

[incubator-superset] branch master updated: fix: SqlaColumn.type overflow on mysql (#7606)

This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 45b9880  fix: SqlaColumn.type overflow on mysql (#7606)
45b9880 is described below

commit 45b988051614bda145add7934af2d310f3a94754
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Mon Jun 3 09:52:25 2019 -0700

    fix: SqlaColumn.type overflow on mysql (#7606)
    
    * fix: SqlaColumn.type overflow on mysql
    
    Hitting a new error when loading examples around a datatype that exceeds
    the 32 chars limit on SqlaColumn.type
    
    Type includes the 'COLLATE utf8mb4_general_ci' suffix which is too
    verbose and not needed in that context.
    
    * fix tests
---
 superset/connectors/sqla/models.py |  2 +-
 superset/db_engine_specs.py        | 15 +++++++++++++++
 tests/base_tests.py                |  7 +++++--
 tests/db_engine_specs_test.py      | 14 ++++++++++++++
 4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index de9f4d1..138b0e5 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -900,7 +900,7 @@ class SqlaTable(Model, BaseDatasource):
 
         for col in table.columns:
             try:
-                datatype = col.type.compile(dialect=db_dialect).upper()
+                datatype = db_engine_spec.column_datatype_to_string(col.type, db_dialect)
             except Exception as e:
                 datatype = 'UNKNOWN'
                 logging.error(
diff --git a/superset/db_engine_specs.py b/superset/db_engine_specs.py
index 5136bca..922e7cc 100644
--- a/superset/db_engine_specs.py
+++ b/superset/db_engine_specs.py
@@ -529,6 +529,10 @@ class BaseEngineSpec(object):
             label = label[:cls.max_column_name_length]
         return label
 
+    @classmethod
+    def column_datatype_to_string(cls, sqla_column_type, dialect):
+        return sqla_column_type.compile(dialect=dialect).upper()
+
 
 class PostgresBaseEngineSpec(BaseEngineSpec):
     """ Abstract class for Postgres 'like' databases """
@@ -864,6 +868,17 @@ class MySQLEngineSpec(BaseEngineSpec):
             pass
         return message
 
+    @classmethod
+    def column_datatype_to_string(cls, sqla_column_type, dialect):
+        datatype = super().column_datatype_to_string(sqla_column_type, dialect)
+        # MySQL dialect started returning long overflowing datatype
+        # as in 'VARCHAR(255) COLLATE UTF8MB4_GENERAL_CI'
+        # and we don't need the verbose collation type
+        str_cutoff = ' COLLATE '
+        if str_cutoff in datatype:
+            datatype = datatype.split(str_cutoff)[0]
+        return datatype
+
 
 class PrestoEngineSpec(BaseEngineSpec):
     engine = 'presto'
diff --git a/tests/base_tests.py b/tests/base_tests.py
index 6de082a..b24193c 100644
--- a/tests/base_tests.py
+++ b/tests/base_tests.py
@@ -174,12 +174,15 @@ class SupersetTestCase(unittest.TestCase):
                     perm.view_menu and table.perm in perm.view_menu.name):
                 security_manager.del_permission_role(public_role, perm)
 
+    def get_main_database(self):
+        return get_main_database(db.session)
+
     def run_sql(self, sql, client_id=None, user_name=None, raise_on_error=False,
                 query_limit=None):
         if user_name:
             self.logout()
             self.login(username=(user_name if user_name else 'admin'))
-        dbid = get_main_database(db.session).id
+        dbid = self.get_main_database().id
         resp = self.get_json_resp(
             '/superset/sql_json/',
             raise_on_error=False,
@@ -195,7 +198,7 @@ class SupersetTestCase(unittest.TestCase):
         if user_name:
             self.logout()
             self.login(username=(user_name if user_name else 'admin'))
-        dbid = get_main_database(db.session).id
+        dbid = self.get_main_database().id
         resp = self.get_json_resp(
             '/superset/validate_sql_json/',
             raise_on_error=False,
diff --git a/tests/db_engine_specs_test.py b/tests/db_engine_specs_test.py
index 4491914..aae7380 100644
--- a/tests/db_engine_specs_test.py
+++ b/tests/db_engine_specs_test.py
@@ -814,3 +814,17 @@ class DbEngineSpecsTestCase(SupersetTestCase):
         expr = PinotEngineSpec.get_timestamp_expr(col, 'epoch_s', 'P1M')
         result = str(expr.compile())
         self.assertEqual(result, 'DATETIMECONVERT(tstamp, "1:SECONDS:EPOCH", "1:SECONDS:EPOCH", "1:MONTHS")')  # noqa
+
+    def test_column_datatype_to_string(self):
+        main_db = self.get_main_database()
+        sqla_table = main_db.get_table('energy_usage')
+        dialect = main_db.get_dialect()
+        col_names = [
+            main_db.db_engine_spec.column_datatype_to_string(c.type, dialect)
+            for c in sqla_table.columns
+        ]
+        if main_db.backend == 'postgresql':
+            expected = ['VARCHAR(255)', 'VARCHAR(255)', 'DOUBLE PRECISION']
+        else:
+            expected = ['VARCHAR(255)', 'VARCHAR(255)', 'FLOAT']
+        self.assertEquals(col_names, expected)