From 9c9ddc60def33dce0984b56a85111dadae428312 Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Thu, 31 Oct 2019 07:13:41 -0700 Subject: [PATCH] [sip-15] Adding database level python-date-format (#8474) --- docs/installation.rst | 16 ++++++++-- .../src/datasource/DatasourceEditor.jsx | 4 ++- superset/config.py | 5 +++- superset/connectors/sqla/models.py | 30 ++++++++++++++++--- superset/connectors/sqla/views.py | 3 ++ 5 files changed, 49 insertions(+), 9 deletions(-) diff --git a/docs/installation.rst b/docs/installation.rst index f30199bb10795..d50a8655fb74c 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -954,8 +954,8 @@ cache store when upgrading an existing environment. entire setup. If not, background jobs can get scheduled multiple times resulting in weird behaviors like duplicate delivery of reports, higher than expected load / traffic etc. - -* SQL Lab will only run your queries asynchronously if you enable + +* SQL Lab will only run your queries asynchronously if you enable "Asynchronous Query Execution" in your database settings. @@ -1346,7 +1346,17 @@ SIP-15 `SIP-15 `_ aims to ensure that time intervals are handled in a consistent and transparent manner for both the Druid and SQLAlchemy connectors. -Prior to SIP-15 SQLAlchemy used inclusive endpoints however these may behave like exclusive depending on the time column (refer to the SIP for details) and thus the endpoint behavior could be unknown. To aid with transparency the current endpoint behavior is explicitly called out in the chart time range (post SIP-15 this will be [start, end) for all connectors and databases). One can override the defaults on a per database level via the ``extra`` +Prior to SIP-15 SQLAlchemy used inclusive endpoints however these may behave like exclusive for string columns (due to lexicographical ordering) if no formatting was defined and the column formatting did not conform to an ISO 8601 date-time (refer to the SIP for details). + +To remedy this rather than having to define the date/time format for every non-IS0 8601 date-time column, once can define a default column mapping on a per database level via the ``extra`` parameter :: + + { + "python_date_format_by_column_name": { + "ds": "%Y-%m-%d" + } + } + +Additionally to aid with transparency the current endpoint behavior is explicitly called out in the chart time range (post SIP-15 this will be [start, end) for all connectors and databases). One can override the defaults on a per database level via the ``extra`` parameter :: { diff --git a/superset/assets/src/datasource/DatasourceEditor.jsx b/superset/assets/src/datasource/DatasourceEditor.jsx index 71fa9af49bd0c..4ef034c6f19c6 100644 --- a/superset/assets/src/datasource/DatasourceEditor.jsx +++ b/superset/assets/src/datasource/DatasourceEditor.jsx @@ -113,7 +113,9 @@ function ColumnCollectionTable({ you will need to define an expression and type for transforming the string into a date or timestamp. Note currently time zones are not supported. If time is stored - in epoch format, put \`epoch_s\` or \`epoch_ms\`.`)} + in epoch format, put \`epoch_s\` or \`epoch_ms\`. If no pattern + is specified we fall back to using the optional defaults on a per + database/column name level via the extra parameter.`)} } control={} diff --git a/superset/config.py b/superset/config.py index b6b2b1fa33423..b0852892a23ac 100644 --- a/superset/config.py +++ b/superset/config.py @@ -694,7 +694,10 @@ class CeleryConfig(object): # SQLALCHEMY_DATABASE_URI by default if set to `None` SQLALCHEMY_EXAMPLES_URI = None -# Note currently SIP-15 feature is WIP and should not be enabled. +# SIP-15 should be enabled for all new Superset deployments which ensures that the time +# range endpoints adhere to [start, end). For existing deployments admins should provide +# a dedicated period of time to allow chart producers to update their charts before +# mass migrating all charts to use the [start, end) interval. SIP_15_ENABLED = False SIP_15_DEFAULT_TIME_RANGE_ENDPOINTS = ["unknown", "inclusive"] SIP_15_TOAST_MESSAGE = 'Action Required: Preview then save your chart using the new time range endpoints here.' diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index ef054bcf6762f..c1ab44e5111a6 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -169,15 +169,19 @@ def get_time_filter( col = self.get_sqla_col(label="__time") l = [] if start_dttm: - l.append(col >= text(self.dttm_sql_literal(start_dttm))) + l.append( + col >= text(self.dttm_sql_literal(start_dttm, time_range_endpoints)) + ) if end_dttm: if ( time_range_endpoints and time_range_endpoints[1] == utils.TimeRangeEndpoint.EXCLUSIVE ): - l.append(col < text(self.dttm_sql_literal(end_dttm))) + l.append( + col < text(self.dttm_sql_literal(end_dttm, time_range_endpoints)) + ) else: - l.append(col <= text(self.dttm_sql_literal(end_dttm))) + l.append(col <= text(self.dttm_sql_literal(end_dttm, None))) return and_(*l) def get_timestamp_expression( @@ -218,7 +222,13 @@ def lookup_obj(lookup_column): return import_datasource.import_simple_obj(db.session, i_column, lookup_obj) - def dttm_sql_literal(self, dttm: DateTime) -> str: + def dttm_sql_literal( + self, + dttm: DateTime, + time_range_endpoints: Optional[ + Tuple[utils.TimeRangeEndpoint, utils.TimeRangeEndpoint] + ], + ) -> str: """Convert datetime object to a SQL expression string""" sql = ( self.table.database.db_engine_spec.convert_dttm(self.type, dttm) @@ -231,6 +241,18 @@ def dttm_sql_literal(self, dttm: DateTime) -> str: tf = self.python_date_format + # Fallback to the default format (if defined) only if the SIP-15 time range + # endpoints, i.e., [start, end) are enabled. + if not tf and time_range_endpoints == ( + utils.TimeRangeEndpoint.INCLUSIVE, + utils.TimeRangeEndpoint.EXCLUSIVE, + ): + tf = ( + self.table.database.get_extra() + .get("python_date_format_by_column_name", {}) + .get(self.column_name) + ) + if tf: if tf in ["epoch_ms", "epoch_s"]: seconds_since_epoch = int(dttm.timestamp()) diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index 6af0a3799c99f..c508aa9a66f03 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -112,6 +112,9 @@ class TableColumnInlineView(CompactCRUDMixin, SupersetModelView): "define an expression and type for transforming the string into a " "date or timestamp. Note currently time zones are not supported. " "If time is stored in epoch format, put `epoch_s` or `epoch_ms`." + "If no pattern is specified we fall back to using the optional " + "defaults on a per database/column name level via the extra parameter." + "" ), True, ),