You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/03/21 19:22:06 UTC
(superset) branch 4.0 updated (f52647ff48 -> 4a59f23be3)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
from f52647ff48 perf(sqllab): reduce bootstrap data delay by queries (#27488)
new 4b750c0caf fix: guest queries (#27566)
new 634d72b19d fix: Skips Hive tests that are blocking PRs (#27605)
new 652e6cfa3f fix(db_engine_specs): Update convert_dttm to work correctly with CrateDB (#27567)
new 4a59f23be3 fix: bump sqlglot to support materialized CTEs (#27576)
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
requirements/base.txt | 2 +-
setup.py | 2 +-
superset/db_engine_specs/crate.py | 2 +-
superset/security/manager.py | 24 +++--
tests/integration_tests/charts/data/api_tests.py | 9 ++
tests/unit_tests/db_engine_specs/test_crate.py | 2 +-
tests/unit_tests/security/manager_test.py | 119 ++++++++++++++++++++++-
7 files changed, 145 insertions(+), 15 deletions(-)
(superset) 04/04: fix: bump sqlglot to support materialized CTEs (#27576)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 4a59f23be365e245cfcaaf6086f158408c31d9ca
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Wed Mar 20 15:26:01 2024 -0400
fix: bump sqlglot to support materialized CTEs (#27576)
---
requirements/base.txt | 2 +-
setup.py | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/requirements/base.txt b/requirements/base.txt
index 7abe2783a9..980dd9f115 100644
--- a/requirements/base.txt
+++ b/requirements/base.txt
@@ -342,7 +342,7 @@ sqlalchemy-utils==0.38.3
# via
# apache-superset
# flask-appbuilder
-sqlglot==20.8.0
+sqlglot==23.0.2
# via apache-superset
sqlparse==0.4.4
# via apache-superset
diff --git a/setup.py b/setup.py
index 0d0488bf35..1ecf23f284 100644
--- a/setup.py
+++ b/setup.py
@@ -126,7 +126,7 @@ setup(
"slack_sdk>=3.19.0, <4",
"sqlalchemy>=1.4, <2",
"sqlalchemy-utils>=0.38.3, <0.39",
- "sqlglot>=20,<21",
+ "sqlglot>=23.0.2,<24",
"sqlparse>=0.4.4, <0.5",
"tabulate>=0.8.9, <0.9",
"typing-extensions>=4, <5",
(superset) 03/04: fix(db_engine_specs): Update convert_dttm to work correctly with CrateDB (#27567)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 652e6cfa3f782bf8c9b38a1fe8e0a3a8447b9f82
Author: hlcianfagna <11...@users.noreply.github.com>
AuthorDate: Thu Mar 21 18:35:40 2024 +0000
fix(db_engine_specs): Update convert_dttm to work correctly with CrateDB (#27567)
(cherry picked from commit fcceaf081c85c501ce946a114447751d43a1f8fb)
---
superset/db_engine_specs/crate.py | 2 +-
tests/unit_tests/db_engine_specs/test_crate.py | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/superset/db_engine_specs/crate.py b/superset/db_engine_specs/crate.py
index 46ce1e08ff..4952bd5a0d 100644
--- a/superset/db_engine_specs/crate.py
+++ b/superset/db_engine_specs/crate.py
@@ -59,7 +59,7 @@ class CrateEngineSpec(BaseEngineSpec):
sqla_type = cls.get_sqla_column_type(target_type)
if isinstance(sqla_type, types.TIMESTAMP):
- return f"{dttm.timestamp() * 1000}"
+ return f"CAST('{dttm.isoformat()}' AS TIMESTAMP)"
return None
@classmethod
diff --git a/tests/unit_tests/db_engine_specs/test_crate.py b/tests/unit_tests/db_engine_specs/test_crate.py
index 2cb1cd7896..d2bace955c 100644
--- a/tests/unit_tests/db_engine_specs/test_crate.py
+++ b/tests/unit_tests/db_engine_specs/test_crate.py
@@ -59,7 +59,7 @@ def test_alter_new_orm_column() -> None:
@pytest.mark.parametrize(
"target_type,expected_result",
[
- ("TimeStamp", "1546398245678.9"),
+ ("TimeStamp", "CAST('2019-01-02T03:04:05.678900' AS TIMESTAMP)"),
("UnknownType", None),
],
)
(superset) 01/04: fix: guest queries (#27566)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 4b750c0caf6e55517a1272a21fc2ad258d8823ef
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Tue Mar 19 11:20:52 2024 -0400
fix: guest queries (#27566)
(cherry picked from commit 36290ce72fa806e8b6c063511ea434a97d91c3a9)
---
superset/security/manager.py | 24 +++---
tests/unit_tests/security/manager_test.py | 119 +++++++++++++++++++++++++++++-
2 files changed, 132 insertions(+), 11 deletions(-)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 94719cb524..a532431433 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -68,6 +68,7 @@ from superset.security.guest_token import (
GuestTokenUser,
GuestUser,
)
+from superset.superset_typing import Metric
from superset.utils.core import (
DatasourceName,
DatasourceType,
@@ -144,6 +145,13 @@ RoleModelView.edit_columns = ["name", "permissions", "user"]
RoleModelView.related_views = []
+def freeze_metric(metric: Metric) -> str:
+ """
+ Used to compare metric sets.
+ """
+ return json.dumps(metric, sort_keys=True)
+
+
def query_context_modified(query_context: "QueryContext") -> bool:
"""
Check if a query context has been modified.
@@ -154,9 +162,9 @@ def query_context_modified(query_context: "QueryContext") -> bool:
form_data = query_context.form_data
stored_chart = query_context.slice_
- # sanity checks
+ # native filter requests
if form_data is None or stored_chart is None:
- return True
+ return False
# cannot request a different chart
if form_data.get("slice_id") != stored_chart.id:
@@ -164,11 +172,10 @@ def query_context_modified(query_context: "QueryContext") -> bool:
# compare form_data
requested_metrics = {
- frozenset(metric.items()) if isinstance(metric, dict) else metric
- for metric in form_data.get("metrics") or []
+ freeze_metric(metric) for metric in form_data.get("metrics") or []
}
stored_metrics = {
- frozenset(metric.items()) if isinstance(metric, dict) else metric
+ freeze_metric(metric)
for metric in stored_chart.params_dict.get("metrics") or []
}
if not requested_metrics.issubset(stored_metrics):
@@ -176,7 +183,7 @@ def query_context_modified(query_context: "QueryContext") -> bool:
# compare queries in query_context
queries_metrics = {
- frozenset(metric.items()) if isinstance(metric, dict) else metric
+ freeze_metric(metric)
for query in query_context.queries
for metric in query.metrics or []
}
@@ -185,10 +192,7 @@ def query_context_modified(query_context: "QueryContext") -> bool:
stored_query_context = json.loads(cast(str, stored_chart.query_context))
for query in stored_query_context.get("queries") or []:
stored_metrics.update(
- {
- frozenset(metric.items()) if isinstance(metric, dict) else metric
- for metric in query.get("metrics") or []
- }
+ {freeze_metric(metric) for metric in query.get("metrics") or []}
)
return not queries_metrics.issubset(stored_metrics)
diff --git a/tests/unit_tests/security/manager_test.py b/tests/unit_tests/security/manager_test.py
index 22ec0dda4a..7ed32b0abb 100644
--- a/tests/unit_tests/security/manager_test.py
+++ b/tests/unit_tests/security/manager_test.py
@@ -26,7 +26,7 @@ from superset.connectors.sqla.models import Database, SqlaTable
from superset.exceptions import SupersetSecurityException
from superset.extensions import appbuilder
from superset.models.slice import Slice
-from superset.security.manager import SupersetSecurityManager
+from superset.security.manager import query_context_modified, SupersetSecurityManager
from superset.sql_parse import Table
from superset.superset_typing import AdhocMetric
from superset.utils.core import override_user
@@ -414,3 +414,120 @@ def test_raise_for_access_chart_owner(
sm.raise_for_access(
chart=slice,
)
+
+
+def test_query_context_modified(
+ mocker: MockFixture,
+ stored_metrics: list[AdhocMetric],
+) -> None:
+ """
+ Test the `query_context_modified` function.
+
+ The function is used to ensure guest users are not modifying the request payload on
+ embedded dashboard, preventing users from modifying it to access metrics different
+ from the ones stored in dashboard charts.
+ """
+ query_context = mocker.MagicMock()
+ query_context.slice_.id = 42
+ query_context.slice_.query_context = None
+ query_context.slice_.params_dict = {
+ "metrics": stored_metrics,
+ }
+
+ query_context.form_data = {
+ "slice_id": 42,
+ "metrics": stored_metrics,
+ }
+ query_context.queries = [QueryObject(metrics=stored_metrics)] # type: ignore
+ assert not query_context_modified(query_context)
+
+
+def test_query_context_modified_tampered(
+ mocker: MockFixture,
+ stored_metrics: list[AdhocMetric],
+) -> None:
+ """
+ Test the `query_context_modified` function when the request is tampered with.
+
+ The function is used to ensure guest users are not modifying the request payload on
+ embedded dashboard, preventing users from modifying it to access metrics different
+ from the ones stored in dashboard charts.
+ """
+ query_context = mocker.MagicMock()
+ query_context.slice_.id = 42
+ query_context.slice_.query_context = None
+ query_context.slice_.params_dict = {
+ "metrics": stored_metrics,
+ }
+
+ tampered_metrics = [
+ {
+ "column": None,
+ "expressionType": "SQL",
+ "hasCustomLabel": False,
+ "label": "COUNT(*) + 2",
+ "sqlExpression": "COUNT(*) + 2",
+ }
+ ]
+
+ query_context.form_data = {
+ "slice_id": 42,
+ "metrics": tampered_metrics,
+ }
+ query_context.queries = [QueryObject(metrics=tampered_metrics)] # type: ignore
+ assert query_context_modified(query_context)
+
+
+def test_query_context_modified_native_filter(mocker: MockFixture) -> None:
+ """
+ Test the `query_context_modified` function with a native filter request.
+
+ A native filter request has no chart (slice) associated with it.
+ """
+ query_context = mocker.MagicMock()
+ query_context.slice_ = None
+
+ assert not query_context_modified(query_context)
+
+
+def test_query_context_modified_mixed_chart(mocker: MockFixture) -> None:
+ """
+ Test the `query_context_modified` function for a mixed chart request.
+
+ The metrics in the mixed chart are a nested dictionary (due to `columns`), and need
+ to be serialized to JSON with the keys sorted in order to compare the request
+ metrics with the chart metrics.
+ """
+ stored_metrics = [
+ {
+ "optionName": "metric_vgops097wej_g8uff99zhk7",
+ "label": "AVG(num)",
+ "expressionType": "SIMPLE",
+ "column": {"column_name": "num", "type": "BIGINT(20)"},
+ "aggregate": "AVG",
+ }
+ ]
+ # different order (remember, dicts have order!)
+ requested_metrics = [
+ {
+ "aggregate": "AVG",
+ "column": {"column_name": "num", "type": "BIGINT(20)"},
+ "expressionType": "SIMPLE",
+ "label": "AVG(num)",
+ "optionName": "metric_vgops097wej_g8uff99zhk7",
+ }
+ ]
+
+ query_context = mocker.MagicMock()
+ query_context.slice_.id = 42
+ query_context.slice_.query_context = None
+ query_context.slice_.params_dict = {
+ "metrics": stored_metrics,
+ }
+
+ query_context.form_data = {
+ "slice_id": 42,
+ "metrics": requested_metrics,
+ }
+ query_context.queries = [QueryObject(metrics=requested_metrics)] # type: ignore
+ assert not query_context_modified(query_context)
(superset) 02/04: fix: Skips Hive tests that are blocking PRs (#27605)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 634d72b19d2630d867244aa9cae4c639b1580f8e
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Thu Mar 21 13:26:24 2024 -0300
fix: Skips Hive tests that are blocking PRs (#27605)
(cherry picked from commit 718cd64657248f846a03a73167d2dc32d1f9dec5)
---
tests/integration_tests/charts/data/api_tests.py | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tests/integration_tests/charts/data/api_tests.py b/tests/integration_tests/charts/data/api_tests.py
index 67586b33f8..7e5d4fda7f 100644
--- a/tests/integration_tests/charts/data/api_tests.py
+++ b/tests/integration_tests/charts/data/api_tests.py
@@ -62,6 +62,8 @@ from superset.common.chart_data import ChartDataResultFormat, ChartDataResultTyp
from tests.common.query_context_generator import ANNOTATION_LAYERS
from tests.integration_tests.fixtures.query_context import get_query_context
+from tests.integration_tests.test_app import app
+
CHART_DATA_URI = "api/v1/chart/data"
CHARTS_FIXTURE_COUNT = 10
@@ -79,6 +81,13 @@ INCOMPATIBLE_ADHOC_COLUMN_FIXTURE: AdhocColumn = {
}
+@pytest.fixture(autouse=True)
+def skip_by_backend():
+ with app.app_context():
+ if backend() == "hive":
+ pytest.skip("Skipping tests for Hive backend")
+
+
class BaseTestChartDataApi(SupersetTestCase):
query_context_payload_template = None