[SIP-15] Adding initial framework

docs/installation.rst

@@ -1242,7 +1242,7 @@ Then we can add this two lines to ``superset_config.py``:
   CUSTOM_SECURITY_MANAGER = CustomSsoSecurityManager
 
 Feature Flags
----------------------------
+-------------
 
 Because of a wide variety of users, Superset has some features that are not enabled by default. For example, some users have stronger security restrictions, while some others may not. So Superset allow users to enable or disable some features by config. For feature owners, you can add optional functionalities in Superset, but will be only affected by a subset of users.
 
@@ -1267,3 +1267,16 @@ Here is a list of flags and descriptions:
 * PRESTO_EXPAND_DATA
 
   * When this feature is enabled, nested types in Presto will be expanded into extra columns and/or arrays. This is experimental, and doesn't work with all nested types.
+
+
+SIP-15
+------
+
+`SIP-15 <https://github.com/apache/incubator-superset/issues/6360>`_ 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``
+parameter ::
+
+    {
+        "time_range_endpoints": ["inclusive", "inclusive"]
+    }

superset/assets/package.json

@@ -101,6 +101,7 @@
     "dompurify": "^1.0.3",
     "geolib": "^2.0.24",
     "immutable": "^3.8.2",
+    "interweave": "^11.2.0",
     "jquery": "^3.4.1",
     "json-bigint": "^0.3.0",
     "lodash": "^4.17.15",

superset/assets/spec/javascripts/messageToasts/components/Toast_spec.jsx

@@ -18,7 +18,7 @@
  */
 import { Alert } from 'react-bootstrap';
 import React from 'react';
-import { shallow } from 'enzyme';
+import { mount } from 'enzyme';
 
 import mockMessageToasts from '../mockMessageToasts';
 import Toast from '../../../../src/messageToasts/components/Toast';
@@ -30,7 +30,7 @@ describe('Toast', () => {
   };
 
   function setup(overrideProps) {
-    const wrapper = shallow(<Toast {...props} {...overrideProps} />);
+    const wrapper = mount(<Toast {...props} {...overrideProps} />);
     return wrapper;
   }
 
@@ -41,9 +41,14 @@ describe('Toast', () => {
 
   it('should render toastText within the alert', () => {
     const wrapper = setup();
-    const alert = wrapper.find(Alert).dive();
+    const alert = wrapper.find(Alert);
 
-    expect(alert.childAt(1).text()).toBe(props.toast.text);
+    expect(
+      alert
+        .childAt(0)
+        .childAt(1)
+        .text(),
+    ).toBe(props.toast.text);
   });
 
   it('should call onCloseToast upon alert dismissal', done => {

superset/assets/src/explore/components/controls/DateFilterControl.jsx

@@ -83,6 +83,7 @@ const FREEFORM_TOOLTIP = t(
 );
 
 const DATE_FILTER_POPOVER_STYLE = { width: '250px' };
+const ISO_8601_REGEX_MATCH = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}$/;
 
 const propTypes = {
   animation: PropTypes.bool,
@@ -94,6 +95,7 @@ const propTypes = {
   height: PropTypes.number,
   onOpenDateFilterControl: PropTypes.func,
   onCloseDateFilterControl: PropTypes.func,
+  endpoints: PropTypes.arrayOf(PropTypes.string),
 };
 
 const defaultProps = {
@@ -359,13 +361,14 @@ export default class DateFilterControl extends React.Component {
       ));
     const timeFrames = COMMON_TIME_FRAMES.map((timeFrame) => {
       const nextState = getStateFromCommonTimeFrame(timeFrame);
+      const endpoints = this.props.endpoints;
       return (
         <OverlayTrigger
           key={timeFrame}
           placement="left"
           overlay={
             <Tooltip id={`tooltip-${timeFrame}`}>
-              {nextState.since}<br />{nextState.until}
+              {nextState.since} {endpoints && `(${endpoints[0]})`}<br />{nextState.until} {endpoints && `(${endpoints[1]})`}
             </Tooltip>
           }
         >
@@ -507,7 +510,15 @@ export default class DateFilterControl extends React.Component {
   }
   render() {
     let value = this.props.value || defaultProps.value;
-    value = value.split(SEPARATOR).map((v, idx) => v.replace('T00:00:00', '') || (idx === 0 ? '-∞' : '∞')).join(SEPARATOR);
+    const endpoints = this.props.endpoints;
+    value = value
+      .split(SEPARATOR)
+      .map((v, idx) =>
+        ISO_8601_REGEX_MATCH.test(v)
+          ? v.replace('T00:00:00', '') + (endpoints ? ` (${endpoints[idx]})` : '')
+          : v || (idx === 0 ? '-∞' : '∞'),
+      )
+      .join(SEPARATOR);
     return (
       <div>
         <ControlHeader {...this.props} />

superset/assets/src/explore/controlPanels/sections.jsx

@@ -35,7 +35,7 @@ export const datasourceAndVizType = {
   controlSetRows: [
     ['datasource'],
     ['viz_type'],
-    ['slice_id', 'cache_timeout', 'url_params'],
+    ['slice_id', 'cache_timeout', 'url_params', 'time_range_endpoints'],
   ],
 };
 

superset/assets/src/explore/controls.jsx

@@ -959,6 +959,9 @@ export const controls = {
       'using the engine\'s local timezone. Note one can explicitly set the timezone ' +
       'per the ISO 8601 format if specifying either the start and/or end time.',
     ),
+    mapStateToProps: state => ({
+      endpoints: state.form_data ? state.form_data.time_range_endpoints : null,
+    }),
   },
 
   max_bubble_size: {
@@ -2034,6 +2037,13 @@ export const controls = {
     description: t('Extra parameters for use in jinja templated queries'),
   },
 
+  time_range_endpoints: {
+    type: 'HiddenControl',
+    label: t('Time range endpoints'),
+    hidden: true,
+    description: t('Time range endpoints (SIP-15)'),
+  },
+
   order_by_entity: {
     type: 'CheckboxControl',
     label: t('Order by entity id'),

superset/assets/src/messageToasts/components/Toast.jsx

@@ -18,6 +18,7 @@
  */
 import { Alert } from 'react-bootstrap';
 import cx from 'classnames';
+import Interweave from 'interweave';
 import PropTypes from 'prop-types';
 import React from 'react';
 
@@ -96,7 +97,7 @@ class Toast extends React.Component {
           toastType === DANGER_TOAST && 'toast--danger',
         )}
       >
-        {text}
+        <Interweave content={text} />
       </Alert>
     );
   }

superset/assets/src/messageToasts/stylesheets/toast.less

@@ -23,7 +23,7 @@
   bottom: 16px;
   left: 50%;
   transform: translate(-50%, 0);
-  width: 500px;
+  width: 600px;
   z-index: 3000; // top of the world
 }
 

superset/config.py

@@ -679,6 +679,11 @@ 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_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 <a href="{url}" class="alert-link">here</a>.'
+
 if CONFIG_PATH_ENV_VAR in os.environ:
     # Explicitly import config module that is not necessarily in pythonpath; useful
     # for case where app is being executed via pex.

superset/connectors/sqla/models.py

@@ -19,7 +19,7 @@
 import re
 from collections import OrderedDict
 from datetime import datetime
-from typing import Any, Dict, List, NamedTuple, Optional, Union
+from typing import Any, Dict, List, NamedTuple, Optional, Tuple, Union
 
 import pandas as pd
 import sqlalchemy as sa
@@ -159,14 +159,25 @@ def datasource(self) -> RelationshipProperty:
         return self.table
 
     def get_time_filter(
-        self, start_dttm: DateTime, end_dttm: DateTime
+        self,
+        start_dttm: DateTime,
+        end_dttm: DateTime,
+        time_range_endpoints: Optional[
+            Tuple[utils.TimeRangeEndpoint, utils.TimeRangeEndpoint]
+        ],
     ) -> ColumnElement:
         col = self.get_sqla_col(label="__time")
         l = []
         if start_dttm:
             l.append(col >= text(self.dttm_sql_literal(start_dttm)))
         if end_dttm:
-            l.append(col <= text(self.dttm_sql_literal(end_dttm)))
+            if (
+                time_range_endpoints
+                and time_range_endpoints[1] == utils.TimeRangeEndpoint.EXCLUSIVE
+            ):
+                l.append(col < text(self.dttm_sql_literal(end_dttm)))
+            else:
+                l.append(col <= text(self.dttm_sql_literal(end_dttm)))
         return and_(*l)
 
     def get_timestamp_expression(
@@ -705,6 +716,7 @@ def get_sqla_query(  # sqla
                 )
             metrics_exprs = []
 
+        time_range_endpoints = extras.get("time_range_endpoints")
         groupby_exprs_with_timestamp = OrderedDict(groupby_exprs_sans_timestamp.items())
         if granularity:
             dttm_col = cols[granularity]
@@ -716,16 +728,20 @@ def get_sqla_query(  # sqla
                 select_exprs += [timestamp]
                 groupby_exprs_with_timestamp[timestamp.name] = timestamp
 
-            # Use main dttm column to support index with secondary dttm columns
+            # Use main dttm column to support index with secondary dttm columns.
             if (
                 db_engine_spec.time_secondary_columns
                 and self.main_dttm_col in self.dttm_cols
                 and self.main_dttm_col != dttm_col.column_name
             ):
                 time_filters.append(
-                    cols[self.main_dttm_col].get_time_filter(from_dttm, to_dttm)
+                    cols[self.main_dttm_col].get_time_filter(
+                        from_dttm, to_dttm, time_range_endpoints
+                    )
                 )
-            time_filters.append(dttm_col.get_time_filter(from_dttm, to_dttm))
+            time_filters.append(
+                dttm_col.get_time_filter(from_dttm, to_dttm, time_range_endpoints)
+            )
 
         select_exprs += metrics_exprs
 
@@ -831,7 +847,9 @@ def get_sqla_query(  # sqla
                 inner_select_exprs += [inner_main_metric_expr]
                 subq = select(inner_select_exprs).select_from(tbl)
                 inner_time_filter = dttm_col.get_time_filter(
-                    inner_from_dttm or from_dttm, inner_to_dttm or to_dttm
+                    inner_from_dttm or from_dttm,
+                    inner_to_dttm or to_dttm,
+                    time_range_endpoints,
                 )
                 subq = subq.where(and_(*(where_clause_and + [inner_time_filter])))
                 subq = subq.group_by(*inner_groupby_exprs)

superset/utils/core.py

@@ -33,6 +33,7 @@
 from email.mime.multipart import MIMEMultipart
 from email.mime.text import MIMEText
 from email.utils import formatdate
+from enum import Enum
 from time import struct_time
 from typing import Iterator, List, NamedTuple, Optional, Tuple, Union
 from urllib.parse import unquote_plus
@@ -1244,3 +1245,19 @@ def split(
             elif not quotes:
                 quotes = True
     yield s[i:]
+
+
+class TimeRangeEndpoint(str, Enum):
+    """
+    The time range endpoint types which represent inclusive, exclusive, or unknown.
+
+    Unknown represents endpoints which are ill-defined as though the interval may be
+    [start, end] the filter may behave like (start, end] due to mixed data types and
+    lexicographical ordering.
+
+    :see: https://github.com/apache/incubator-superset/issues/6360
+    """
+
+    EXCLUSIVE = "exclusive"
+    INCLUSIVE = "inclusive"
+    UNKNOWN = "unknown"

superset/views/core.py

@@ -48,6 +48,7 @@
 from sqlalchemy.exc import SQLAlchemyError
 from sqlalchemy.orm.session import Session
 from werkzeug.routing import BaseConverter
+from werkzeug.urls import Href
 
 import superset.models.core as models
 from superset import (
@@ -1071,7 +1072,6 @@ def explore_json(self, datasource_type=None, datasource_id=None):
         results = request.args.get("results") == "true"
         samples = request.args.get("samples") == "true"
         force = request.args.get("force") == "true"
-
         form_data = get_form_data()[0]
 
         try:
@@ -1143,8 +1143,39 @@ def explorev2(self, datasource_type, datasource_id):
     def explore(self, datasource_type=None, datasource_id=None):
         user_id = g.user.get_id() if g.user else None
         form_data, slc = get_form_data(use_slice_data=True)
-        error_redirect = "/chart/list/"
 
+        # Flash the SIP-15 message if the slice is owned by the current user and has not
+        # been updated, i.e., is not using the [start, end) interval.
+        if (
+            config["SIP_15_ENABLED"]
+            and slc
+            and g.user in slc.owners
+            and (
+                not form_data.get("time_range_endpoints")
+                or form_data["time_range_endpoints"]
+                != (
+                    utils.TimeRangeEndpoint.INCLUSIVE,
+                    utils.TimeRangeEndpoint.EXCLUSIVE,
+                )
+            )
+        ):
+            url = Href("/superset/explore/")(
+                {
+                    "form_data": json.dumps(
+                        {
+                            "slice_id": slc.id,
+                            "time_range_endpoints": (
+                                utils.TimeRangeEndpoint.INCLUSIVE.value,
+                                utils.TimeRangeEndpoint.EXCLUSIVE.value,
+                            ),
+                        }
+                    )
+                }
+            )
+
+            flash(Markup(config["SIP_15_TOAST_MESSAGE"].format(url=url)))
+
+        error_redirect = "/chart/list/"
         try:
             datasource_id, datasource_type = get_datasource_info(
                 datasource_id, datasource_type, form_data