superset-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From maximebeauche...@apache.org
Subject [incubator-superset] branch master updated: Force quotes on non-expression time grains on Postgres (#6897)
Date Thu, 21 Feb 2019 21:16:37 GMT
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 ea9d22b  Force quotes on non-expression time grains on Postgres (#6897)
ea9d22b is described below

commit ea9d22b2eca6cf823380adc96d2040a5c955edd8
Author: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
AuthorDate: Thu Feb 21 23:16:28 2019 +0200

    Force quotes on non-expression time grains on Postgres (#6897)
    
    * Force quotes on non-expression time grains on postgres
    
    * Change to or semantics
    
    * Return lower case column name as unmutated
    
    * Add testcases for postgres time grains
    
    * Make expression MixedCase
---
 superset/connectors/sqla/models.py |  5 ++---
 superset/db_engine_specs.py        | 17 +++++++++++++++++
 tests/model_tests.py               | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index 8183d7c..ca0db02 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -156,9 +156,8 @@ class TableColumn(Model, BaseColumn):
             if not grain:
                 raise NotImplementedError(
                     f'No grain spec for {time_grain} for database {db.database_name}')
-        expr = db.db_engine_spec.get_time_expr(
-            self.expression or self.column_name,
-            pdf, time_grain, grain)
+        col = db.db_engine_spec.get_timestamp_column(self.expression, self.column_name)
+        expr = db.db_engine_spec.get_time_expr(col, pdf, time_grain, grain)
         sqla_col = literal_column(expr, type_=DateTime)
         return self.table.make_sqla_column_compatible(sqla_col, label)
 
diff --git a/superset/db_engine_specs.py b/superset/db_engine_specs.py
index 0785082..ad92fa9 100644
--- a/superset/db_engine_specs.py
+++ b/superset/db_engine_specs.py
@@ -463,6 +463,13 @@ class BaseEngineSpec(object):
             label = label[:cls.max_column_name_length]
         return label
 
+    @staticmethod
+    def get_timestamp_column(expression, column_name):
+        """Return the expression if defined, otherwise return column_name. Some
+        engines require forcing quotes around column name, in which case this method
+        can be overridden."""
+        return expression or column_name
+
 
 class PostgresBaseEngineSpec(BaseEngineSpec):
     """ Abstract class for Postgres 'like' databases """
@@ -509,6 +516,16 @@ class PostgresEngineSpec(PostgresBaseEngineSpec):
         tables.extend(inspector.get_foreign_table_names(schema))
         return sorted(tables)
 
+    @staticmethod
+    def get_timestamp_column(expression, column_name):
+        """Postgres is unable to identify mixed case column names unless they
+        are quoted."""
+        if expression:
+            return expression
+        elif column_name.lower() != column_name:
+            return f'"{column_name}"'
+        return column_name
+
 
 class SnowflakeEngineSpec(PostgresBaseEngineSpec):
     engine = 'snowflake'
diff --git a/tests/model_tests.py b/tests/model_tests.py
index cc2b30c..0fe03de 100644
--- a/tests/model_tests.py
+++ b/tests/model_tests.py
@@ -117,6 +117,39 @@ class DatabaseModelTestCase(SupersetTestCase):
         self.assertEquals(d.get('P1D').function, 'DATE({col})')
         self.assertEquals(d.get('Time Column').function, '{col}')
 
+    def test_postgres_expression_time_grain(self):
+        uri = 'postgresql+psycopg2://uid:pwd@localhost:5432/superset'
+        database = Database(sqlalchemy_uri=uri)
+        pdf, time_grain = '', 'P1D'
+        expression, column_name = 'COALESCE(lowercase_col, "MixedCaseCol")', ''
+        grain = database.grains_dict().get(time_grain)
+        col = database.db_engine_spec.get_timestamp_column(expression, column_name)
+        grain_expr = database.db_engine_spec.get_time_expr(col, pdf, time_grain, grain)
+        grain_expr_expected = grain.function.replace('{col}', expression)
+        self.assertEqual(grain_expr, grain_expr_expected)
+
+    def test_postgres_lowercase_col_time_grain(self):
+        uri = 'postgresql+psycopg2://uid:pwd@localhost:5432/superset'
+        database = Database(sqlalchemy_uri=uri)
+        pdf, time_grain = '', 'P1D'
+        expression, column_name = '', 'lowercase_col'
+        grain = database.grains_dict().get(time_grain)
+        col = database.db_engine_spec.get_timestamp_column(expression, column_name)
+        grain_expr = database.db_engine_spec.get_time_expr(col, pdf, time_grain, grain)
+        grain_expr_expected = grain.function.replace('{col}', column_name)
+        self.assertEqual(grain_expr, grain_expr_expected)
+
+    def test_postgres_mixedcase_col_time_grain(self):
+        uri = 'postgresql+psycopg2://uid:pwd@localhost:5432/superset'
+        database = Database(sqlalchemy_uri=uri)
+        pdf, time_grain = '', 'P1D'
+        expression, column_name = '', 'MixedCaseCol'
+        grain = database.grains_dict().get(time_grain)
+        col = database.db_engine_spec.get_timestamp_column(expression, column_name)
+        grain_expr = database.db_engine_spec.get_time_expr(col, pdf, time_grain, grain)
+        grain_expr_expected = grain.function.replace('{col}', f'"{column_name}"')
+        self.assertEqual(grain_expr, grain_expr_expected)
+
     def test_single_statement(self):
         main_db = get_main_database(db.session)
 


Mime
View raw message