Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(explore): deprecated x periods pattern in new time picker value #12552

Merged
merged 15 commits into from
Jan 24, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -197,16 +197,15 @@ export default function DateFilterControl(props: DateFilterLabelProps) {
+--------------+------+----------+--------+----------+-----------+
| | Last | Previous | Custom | Advanced | No Filter |
+--------------+------+----------+--------+----------+-----------+
| control pill | HRT | HRT | ADR | ADR | ADR |
| control pill | HRT | HRT | ADR | ADR | HRT |
+--------------+------+----------+--------+----------+-----------+
| tooltip | ADR | ADR | HRT | HRT | HRT |
| tooltip | ADR | ADR | HRT | HRT | ADR |
+--------------+------+----------+--------+----------+-----------+
*/
const valueToLower = value.toLowerCase();
if (
valueToLower.startsWith('last') ||
valueToLower.startsWith('next') ||
valueToLower.startsWith('previous')
frame === 'Common' ||
frame === 'Calendar' ||
frame === 'No filter'
) {
setActualTimeRange(value);
setTooltipTitle(actualRange || '');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ import { Input } from 'src/common/components';
import { FrameComponentProps } from '../types';

export function AdvancedFrame(props: FrameComponentProps) {
const [since, until] = getAdvancedRange(props.value || '').split(SEPARATOR);
const advancedRange = getAdvancedRange(props.value || '');
const [since, until] = advancedRange.split(SEPARATOR);
if (advancedRange !== props.value) {
props.onChange(getAdvancedRange(props.value || ''));
}

function getAdvancedRange(value: string): string {
if (value.includes(SEPARATOR)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""migrate [x dateunit] to [x dateunit ago/later]

Revision ID: 260bf0649a77
Revises: c878781977c6
Create Date: 2021-01-23 16:25:14.496774

"""

# revision identifiers, used by Alembic.
revision = "260bf0649a77"
down_revision = "c878781977c6"

import json
import re
from sqlite3 import OperationalError

import sqlalchemy as sa
from alembic import op
from sqlalchemy import Column, Integer, or_, Text
from sqlalchemy.dialects.mysql.base import MySQLDialect
from sqlalchemy.dialects.sqlite.base import SQLiteDialect
from sqlalchemy.ext.declarative import declarative_base

from superset import db
from superset.utils.date_parser import DateRangeMigration

Base = declarative_base()


class Slice(Base):
__tablename__ = "slices"

id = Column(Integer, primary_key=True)
slice_name = Column(Text)
params = Column(Text)


def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind)
x_dateunit_in_since = DateRangeMigration.x_dateunit_in_since
x_dateunit_in_until = DateRangeMigration.x_dateunit_in_until

if isinstance(bind.dialect, SQLiteDialect):
try:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQLite does not support REGEXP syntax by default.
https://www.sqlite.org/lang_expr.html#regexp

# The REGEXP operator is a special syntax for the regexp() user function.
# https://www.sqlite.org/lang_expr.html#regexp
to_lower = sa.func.LOWER
where_clause = or_(
sa.func.REGEXP(to_lower(Slice.params), x_dateunit_in_since),
sa.func.REGEXP(to_lower(Slice.params), x_dateunit_in_until),
)
except OperationalError:
return

if isinstance(bind.dialect, MySQLDialect):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MySQL 5.7 support REGEXP operator, the REGEXP_LIKE function is not supported until MySQL 8.x
https://dev.mysql.com/doc/refman/5.7/en/regexp.html#operator_regexp

to_lower = sa.func.LOWER
where_clause = or_(
to_lower(Slice.params).op("REGEXP")(x_dateunit_in_since),
to_lower(Slice.params).op("REGEXP")(x_dateunit_in_until),
)
else:
# default metadata is pg, so: isinstance(bind.dialect, PGDialect):
where_clause = or_(
Slice.params.op("~*")(x_dateunit_in_since),
Slice.params.op("~*")(x_dateunit_in_until),
)

slices = session.query(Slice).filter(where_clause).all()

sep = " : "
pattern = DateRangeMigration.x_dateunit
for idx, slc in enumerate(slices):
print(f"Upgrading ({idx + 1}/{len(slices)}): {slc.slice_name}#{slc.id}")
params = json.loads(slc.params)
time_range = params["time_range"]
if sep in time_range:
start, end = time_range.split(sep)
if re.match(pattern, start):
start = f"{start.strip()} ago"
if re.match(pattern, end):
end = f"{end.strip()} later"
params["time_range"] = f"{start}{sep}{end}"

slc.params = json.dumps(params, sort_keys=True, indent=4)
session.commit()

session.close()


def downgrade():
pass
50 changes: 39 additions & 11 deletions superset/utils/date_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,35 @@ def parse_human_datetime(human_readable: str) -> datetime:
>>> year_after_1 == year_after_2
True
"""
x_periods = r"^\s*([0-9]+)\s+(second|minute|hour|day|week|month|quarter|year)s?\s*$"
if re.search(x_periods, human_readable, re.IGNORECASE):
raise ValueError(
_(
"date string is unclear."
zhaoyongjie marked this conversation as resolved.
Show resolved Hide resolved
" Please specify [%(human_readable)s ago]"
" or [%(human_readable)s later]",
human_readable=human_readable,
)
)

try:
dttm = parse(human_readable)
except Exception: # pylint: disable=broad-except
try:
cal = parsedatetime.Calendar()
parsed_dttm, parsed_flags = cal.parseDT(human_readable)
# when time is not extracted, we 'reset to midnight'
if parsed_flags & 2 == 0:
parsed_dttm = parsed_dttm.replace(hour=0, minute=0, second=0)
dttm = dttm_from_timetuple(parsed_dttm.utctimetuple())
except Exception as ex:
except (ValueError, OverflowError) as ex:
cal = parsedatetime.Calendar()
parsed_dttm, parsed_flags = cal.parseDT(human_readable)
# 0 == not parsed at all
if parsed_flags == 0:
logger.exception(ex)
raise ValueError("Couldn't parse date string [{}]".format(human_readable))
raise ValueError(
_(
"Couldn't parse date string [%(human_readable)s]",
human_readable=human_readable,
)
)
# when time is not extracted, we 'reset to midnight'
if parsed_flags & 2 == 0:
parsed_dttm = parsed_dttm.replace(hour=0, minute=0, second=0)
dttm = dttm_from_timetuple(parsed_dttm.utctimetuple())
return dttm


Expand Down Expand Up @@ -375,7 +391,9 @@ def eval(self) -> datetime:
searched_result = holiday_lookup.get_named(holiday)
if len(searched_result) == 1:
return dttm_from_timetuple(searched_result[0].timetuple())
raise ValueError(_("Unable to find such a holiday: [{}]").format(holiday))
raise ValueError(
_("Unable to find such a holiday: [%(holiday)s]", holiday=holiday)
)


@memoized
Expand Down Expand Up @@ -470,3 +488,13 @@ def datetime_eval(datetime_expression: Optional[str] = None) -> Optional[datetim
except ParseException as error:
raise ValueError(error)
return None


class DateRangeMigration: # pylint: disable=too-few-public-methods
x_dateunit_in_since = (
r'"time_range":\s"\s*[0-9]+\s(day|week|month|quarter|year)s?\s*\s:\s'
)
x_dateunit_in_until = (
r'"time_range":\s".*\s:\s\s*[0-9]+\s(day|week|month|quarter|year)s?\s*"'
)
x_dateunit = r"\s*[0-9]+\s(day|week|month|quarter|year)s?\s*"
6 changes: 3 additions & 3 deletions tests/druid_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ def test_client(self, PyDruid):
"viz_type": "table",
"granularity": "one+day",
"druid_time_origin": "",
"since": "7+days+ago",
"since": "7 days ago",
"until": "now",
"row_limit": 5000,
"include_search": "false",
Expand All @@ -193,7 +193,7 @@ def test_client(self, PyDruid):
"viz_type": "table",
"granularity": "one+day",
"druid_time_origin": "",
"since": "7+days+ago",
"since": "7 days ago",
"until": "now",
"row_limit": 5000,
"include_search": "false",
Expand Down Expand Up @@ -535,7 +535,7 @@ def test_druid_time_granularities(self, PyDruid):

form_data = {
"viz_type": "table",
"since": "7+days+ago",
"since": "7 days ago",
"until": "now",
"metrics": ["count"],
"groupby": [],
Expand Down
30 changes: 30 additions & 0 deletions tests/utils/date_parser_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
from unittest.mock import patch

from superset.utils.date_parser import (
DateRangeMigration,
datetime_eval,
get_since_until,
parse_human_datetime,
parse_human_timedelta,
parse_past_timedelta,
)
Expand Down Expand Up @@ -261,3 +263,31 @@ def test_parse_past_timedelta(self, mock_datetime):
self.assertEqual(parse_past_timedelta("-1 year"), timedelta(365))
self.assertEqual(parse_past_timedelta("52 weeks"), timedelta(364))
self.assertEqual(parse_past_timedelta("1 month"), timedelta(31))

def test_parse_human_datetime(self):
with self.assertRaises(ValueError):
parse_human_datetime(" 2 days ")

with self.assertRaises(ValueError):
parse_human_datetime("2 day")

with self.assertRaises(ValueError):
parse_human_datetime("xxxxxxx")

def test_DateRangeMigration(self):
params = '{"time_range": " 8 days : 2020-03-10T00:00:00"}'
self.assertRegex(params, DateRangeMigration.x_dateunit_in_since)

params = '{"time_range": "2020-03-10T00:00:00 : 8 days "}'
self.assertRegex(params, DateRangeMigration.x_dateunit_in_until)

params = '{"time_range": " 2 weeks : 8 days "}'
self.assertRegex(params, DateRangeMigration.x_dateunit_in_since)
self.assertRegex(params, DateRangeMigration.x_dateunit_in_until)

params = '{"time_range": "2 weeks ago : 8 days later"}'
self.assertNotRegex(params, DateRangeMigration.x_dateunit_in_since)
self.assertNotRegex(params, DateRangeMigration.x_dateunit_in_until)

field = " 8 days "
self.assertRegex(field, DateRangeMigration.x_dateunit)