From 7b62e4e7a61ca04760d5114110a6fa624114d575 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 16 Mar 2022 10:34:15 +0800 Subject: [PATCH 1/7] fix: time filter should be [start, end) --- superset/connectors/sqla/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 9cc2f8a78136b..4a68e666bdac8 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -308,7 +308,7 @@ def get_time_filter( if start_dttm: l.append(col >= self.table.text(self.dttm_sql_literal(start_dttm))) if end_dttm: - l.append(col <= self.table.text(self.dttm_sql_literal(end_dttm))) + l.append(col < self.table.text(self.dttm_sql_literal(end_dttm))) return and_(*l) def get_timestamp_expression( From d023a7e3ac9e18442b7f1179140aa8c9e561c844 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 16 Mar 2022 16:28:08 +0800 Subject: [PATCH 2/7] add ut --- tests/integration_tests/sqla_models_tests.py | 49 ++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/integration_tests/sqla_models_tests.py b/tests/integration_tests/sqla_models_tests.py index 14480f10bcc69..3570a22c6aa2f 100644 --- a/tests/integration_tests/sqla_models_tests.py +++ b/tests/integration_tests/sqla_models_tests.py @@ -41,6 +41,7 @@ FilterOperator, GenericDataType, TemporalType, + backend, ) from superset.utils.database import get_example_database from tests.integration_tests.fixtures.birth_names_dashboard import ( @@ -605,6 +606,54 @@ def test_filter_on_text_column(text_column_table): assert result_object.df["count"][0] == 1 +def test_should_generate_closed_and_open_time_filter_range(): + with app.app_context(): + if backend() in ["sqlite", "mysql"]: + pytest.skip(f"{backend()} has different dialect for datetime column") + + table = SqlaTable( + table_name="temporal_column_table", + sql=("SELECT '2022-03-10'::timestamp as datetime_col"), + database=get_example_database(), + ) + TableColumn( + column_name="datetime_col", type="TIMESTAMP", table=table, is_dttm=True, + ) + SqlMetric(metric_name="count", expression="count(*)", table=table) + result_object = table.query( + { + "metrics": ["count"], + "is_timeseries": True, + "filter": [], + "from_dttm": datetime(2022, 1, 1), + "to_dttm": datetime(2023, 1, 1), + "granularity": "datetime_col", + } + ) + """ >>> result_object.query + SELECT datetime_col AS __timestamp, + count(*) AS count + FROM + (SELECT '2022-03-10' as datetime_col) AS virtual_table + WHERE datetime_col >= '2022-01-01 00:00:00.000000' + AND datetime_col < '2023-01-01 00:00:00.000000' + GROUP BY datetime_col + """ + unformat_sql = result_object.query.replace("\n", " ") + unformat_sql = re.sub(r"(\s){2,}", " ", unformat_sql) + assert ( + "WHERE " + "datetime_col " + ">= " + "TO_TIMESTAMP('2022-01-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US') " + "AND " + "datetime_col < " + "TO_TIMESTAMP('2023-01-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US')" + in unformat_sql + ) + assert result_object.df.iloc[0]["count"] == 1 + + @pytest.mark.parametrize( "row,dimension,result", [ From 1c9224a5275f3329c3e13fc978fd4aaba9b7f85f Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 16 Mar 2022 16:31:32 +0800 Subject: [PATCH 3/7] minor fix --- tests/integration_tests/sqla_models_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_tests/sqla_models_tests.py b/tests/integration_tests/sqla_models_tests.py index 3570a22c6aa2f..7f2f8d67ba26e 100644 --- a/tests/integration_tests/sqla_models_tests.py +++ b/tests/integration_tests/sqla_models_tests.py @@ -608,7 +608,7 @@ def test_filter_on_text_column(text_column_table): def test_should_generate_closed_and_open_time_filter_range(): with app.app_context(): - if backend() in ["sqlite", "mysql"]: + if backend() != "postgresql": pytest.skip(f"{backend()} has different dialect for datetime column") table = SqlaTable( From aae7e270983e4bf24f8315ff0f0a10fce551d140 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 16 Mar 2022 16:33:13 +0800 Subject: [PATCH 4/7] minor fix --- tests/integration_tests/sqla_models_tests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration_tests/sqla_models_tests.py b/tests/integration_tests/sqla_models_tests.py index 7f2f8d67ba26e..5d5b37a5d21ca 100644 --- a/tests/integration_tests/sqla_models_tests.py +++ b/tests/integration_tests/sqla_models_tests.py @@ -647,7 +647,8 @@ def test_should_generate_closed_and_open_time_filter_range(): ">= " "TO_TIMESTAMP('2022-01-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US') " "AND " - "datetime_col < " + "datetime_col " + "< " "TO_TIMESTAMP('2023-01-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US')" in unformat_sql ) From 5c97116dbebd3ed0efaaf17bd15ede34c06a39c7 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 16 Mar 2022 16:36:40 +0800 Subject: [PATCH 5/7] minor fix --- tests/integration_tests/sqla_models_tests.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/integration_tests/sqla_models_tests.py b/tests/integration_tests/sqla_models_tests.py index 5d5b37a5d21ca..94134b07dc90c 100644 --- a/tests/integration_tests/sqla_models_tests.py +++ b/tests/integration_tests/sqla_models_tests.py @@ -631,13 +631,13 @@ def test_should_generate_closed_and_open_time_filter_range(): } ) """ >>> result_object.query - SELECT datetime_col AS __timestamp, - count(*) AS count - FROM - (SELECT '2022-03-10' as datetime_col) AS virtual_table - WHERE datetime_col >= '2022-01-01 00:00:00.000000' - AND datetime_col < '2023-01-01 00:00:00.000000' - GROUP BY datetime_col + SELECT datetime_col AS __timestamp, + count(*) AS count + FROM + (SELECT '2022-03-10'::timestamp as datetime_col) AS virtual_table + WHERE datetime_col >= TO_TIMESTAMP('2022-01-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US') + AND datetime_col < TO_TIMESTAMP('2023-01-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US') + GROUP BY datetime_col """ unformat_sql = result_object.query.replace("\n", " ") unformat_sql = re.sub(r"(\s){2,}", " ", unformat_sql) From b74937b071ded2ef27b6c5fac931a324a059fa02 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 16 Mar 2022 16:46:10 +0800 Subject: [PATCH 6/7] minor fix --- tests/integration_tests/sqla_models_tests.py | 27 ++++++-------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/tests/integration_tests/sqla_models_tests.py b/tests/integration_tests/sqla_models_tests.py index 94134b07dc90c..995616bab676e 100644 --- a/tests/integration_tests/sqla_models_tests.py +++ b/tests/integration_tests/sqla_models_tests.py @@ -613,7 +613,10 @@ def test_should_generate_closed_and_open_time_filter_range(): table = SqlaTable( table_name="temporal_column_table", - sql=("SELECT '2022-03-10'::timestamp as datetime_col"), + sql=( + "SELECT '2022-01-01'::timestamp as datetime_col" + " UNION SELECT '2023-01-01'::timestamp" + ), database=get_example_database(), ) TableColumn( @@ -623,7 +626,7 @@ def test_should_generate_closed_and_open_time_filter_range(): result_object = table.query( { "metrics": ["count"], - "is_timeseries": True, + "is_timeseries": False, "filter": [], "from_dttm": datetime(2022, 1, 1), "to_dttm": datetime(2023, 1, 1), @@ -631,27 +634,13 @@ def test_should_generate_closed_and_open_time_filter_range(): } ) """ >>> result_object.query - SELECT datetime_col AS __timestamp, - count(*) AS count + SELECT count(*) AS count FROM - (SELECT '2022-03-10'::timestamp as datetime_col) AS virtual_table + (SELECT '2022-01-01'::timestamp as datetime_col + UNION SELECT '2023-01-01'::timestamp) AS virtual_table WHERE datetime_col >= TO_TIMESTAMP('2022-01-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US') AND datetime_col < TO_TIMESTAMP('2023-01-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US') - GROUP BY datetime_col """ - unformat_sql = result_object.query.replace("\n", " ") - unformat_sql = re.sub(r"(\s){2,}", " ", unformat_sql) - assert ( - "WHERE " - "datetime_col " - ">= " - "TO_TIMESTAMP('2022-01-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US') " - "AND " - "datetime_col " - "< " - "TO_TIMESTAMP('2023-01-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US')" - in unformat_sql - ) assert result_object.df.iloc[0]["count"] == 1 From 1d719be6d4e03f6d2ad9cb2959883e3b5627bac8 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Thu, 17 Mar 2022 09:32:23 +0800 Subject: [PATCH 7/7] more accurate ut --- tests/integration_tests/sqla_models_tests.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/integration_tests/sqla_models_tests.py b/tests/integration_tests/sqla_models_tests.py index 995616bab676e..54779fcbbff9b 100644 --- a/tests/integration_tests/sqla_models_tests.py +++ b/tests/integration_tests/sqla_models_tests.py @@ -614,8 +614,11 @@ def test_should_generate_closed_and_open_time_filter_range(): table = SqlaTable( table_name="temporal_column_table", sql=( - "SELECT '2022-01-01'::timestamp as datetime_col" - " UNION SELECT '2023-01-01'::timestamp" + "SELECT '2021-12-31'::timestamp as datetime_col " + "UNION SELECT '2022-01-01'::timestamp " + "UNION SELECT '2022-03-10'::timestamp " + "UNION SELECT '2023-01-01'::timestamp " + "UNION SELECT '2023-03-10'::timestamp " ), database=get_example_database(), ) @@ -636,12 +639,15 @@ def test_should_generate_closed_and_open_time_filter_range(): """ >>> result_object.query SELECT count(*) AS count FROM - (SELECT '2022-01-01'::timestamp as datetime_col - UNION SELECT '2023-01-01'::timestamp) AS virtual_table + (SELECT '2021-12-31'::timestamp as datetime_col + UNION SELECT '2022-01-01'::timestamp + UNION SELECT '2022-03-10'::timestamp + UNION SELECT '2023-01-01'::timestamp + UNION SELECT '2023-03-10'::timestamp) AS virtual_table WHERE datetime_col >= TO_TIMESTAMP('2022-01-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US') AND datetime_col < TO_TIMESTAMP('2023-01-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US') """ - assert result_object.df.iloc[0]["count"] == 1 + assert result_object.df.iloc[0]["count"] == 2 @pytest.mark.parametrize(