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/22 12:30:11 UTC

(superset) branch 4.0 updated (4a59f23be3 -> 7c14968e6d)

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 4a59f23be3 fix: bump sqlglot to support materialized CTEs (#27576)
     new 336f6bd021 fix: sqlglot SQL Server (#27577)
     new 90afb34df5 fix: Volatile datasource ordering in dashboard export (#19595)
     new fe95adac98 fix(utils): fix off-by-one error in how rolling window's min_periods truncates dataframe (#27388)
     new a2fb13b522 fix(Dashboard): Add editMode conditional for translate3d fix on charts to allow intended Fullscreen (#27613)
     new 4ff331a66c fix: Persist query params appended to permalink (#27601)
     new 7c14968e6d fix(sql_parse): Ensure table extraction handles Jinja templating (#27470)

The 6 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:
 .../src/dashboard/components/dnd/DragDroppable.jsx |   5 +-
 superset/commands/sql_lab/execute.py               |   4 +-
 superset/jinja_context.py                          |  10 +-
 superset/models/dashboard.py                       |   5 +-
 superset/models/sql_lab.py                         |  40 +++++---
 superset/security/manager.py                       |  13 +--
 superset/sql_parse.py                              | 107 ++++++++++++++++-----
 superset/sqllab/query_render.py                    |   3 +-
 superset/utils/pandas_postprocessing/rolling.py    |   2 +-
 superset/views/core.py                             |   2 +
 tests/integration_tests/charts/data/api_tests.py   |   6 ++
 tests/integration_tests/core_tests.py              |  15 +++
 .../pandas_postprocessing/test_rolling.py          |   4 +-
 tests/unit_tests/sql_parse_tests.py                |  41 ++++++++
 14 files changed, 198 insertions(+), 59 deletions(-)


(superset) 06/06: fix(sql_parse): Ensure table extraction handles Jinja templating (#27470)

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 7c14968e6d3b83498c20d6896f5239bf520d2c79
Author: John Bodley <45...@users.noreply.github.com>
AuthorDate: Fri Mar 22 13:39:28 2024 +1300

    fix(sql_parse): Ensure table extraction handles Jinja templating (#27470)
---
 superset/commands/sql_lab/execute.py |   4 +-
 superset/jinja_context.py            |  10 ++--
 superset/models/sql_lab.py           |  40 ++++++++-----
 superset/security/manager.py         |  13 ++---
 superset/sql_parse.py                | 105 +++++++++++++++++++++++++++--------
 superset/sqllab/query_render.py      |   3 +-
 tests/unit_tests/sql_parse_tests.py  |  41 ++++++++++++++
 7 files changed, 163 insertions(+), 53 deletions(-)

diff --git a/superset/commands/sql_lab/execute.py b/superset/commands/sql_lab/execute.py
index 5d955571d8..533264fb28 100644
--- a/superset/commands/sql_lab/execute.py
+++ b/superset/commands/sql_lab/execute.py
@@ -144,11 +144,13 @@ class ExecuteSqlCommand(BaseCommand):
         try:
             logger.info("Triggering query_id: %i", query.id)
 
+            # Necessary to check access before rendering the Jinjafied query as the
+            # some Jinja macros execute statements upon rendering.
+            self._validate_access(query)
             self._execution_context.set_query(query)
             rendered_query = self._sql_query_render.render(self._execution_context)
             validate_rendered_query = copy.copy(query)
             validate_rendered_query.sql = rendered_query
-            self._validate_access(validate_rendered_query)
             self._set_query_limit_if_required(rendered_query)
             self._query_dao.update(
                 query, {"limit": self._execution_context.query.limit}
diff --git a/superset/jinja_context.py b/superset/jinja_context.py
index 54d1f54866..9edddf24d9 100644
--- a/superset/jinja_context.py
+++ b/superset/jinja_context.py
@@ -24,7 +24,7 @@ from typing import Any, Callable, cast, Optional, TYPE_CHECKING, TypedDict, Unio
 import dateutil
 from flask import current_app, g, has_request_context, request
 from flask_babel import gettext as _
-from jinja2 import DebugUndefined
+from jinja2 import DebugUndefined, Environment
 from jinja2.sandbox import SandboxedEnvironment
 from sqlalchemy.engine.interfaces import Dialect
 from sqlalchemy.sql.expression import bindparam
@@ -462,11 +462,11 @@ class BaseTemplateProcessor:
         self._applied_filters = applied_filters
         self._removed_filters = removed_filters
         self._context: dict[str, Any] = {}
-        self._env = SandboxedEnvironment(undefined=DebugUndefined)
+        self.env: Environment = SandboxedEnvironment(undefined=DebugUndefined)
         self.set_context(**kwargs)
 
         # custom filters
-        self._env.filters["where_in"] = WhereInMacro(database.get_dialect())
+        self.env.filters["where_in"] = WhereInMacro(database.get_dialect())
 
     def set_context(self, **kwargs: Any) -> None:
         self._context.update(kwargs)
@@ -479,7 +479,7 @@ class BaseTemplateProcessor:
         >>> process_template(sql)
         "SELECT '2017-01-01T00:00:00'"
         """
-        template = self._env.from_string(sql)
+        template = self.env.from_string(sql)
         kwargs.update(self._context)
 
         context = validate_template_context(self.engine, kwargs)
@@ -623,7 +623,7 @@ class TrinoTemplateProcessor(PrestoTemplateProcessor):
     engine = "trino"
 
     def process_template(self, sql: str, **kwargs: Any) -> str:
-        template = self._env.from_string(sql)
+        template = self.env.from_string(sql)
         kwargs.update(self._context)
 
         # Backwards compatibility if migrating from Presto.
diff --git a/superset/models/sql_lab.py b/superset/models/sql_lab.py
index f4724d6dab..2d7384a74e 100644
--- a/superset/models/sql_lab.py
+++ b/superset/models/sql_lab.py
@@ -46,6 +46,7 @@ from sqlalchemy.orm import backref, relationship
 from sqlalchemy.sql.elements import ColumnElement, literal_column
 
 from superset import security_manager
+from superset.exceptions import SupersetSecurityException
 from superset.jinja_context import BaseTemplateProcessor, get_template_processor
 from superset.models.helpers import (
     AuditMixinNullable,
@@ -53,7 +54,7 @@ from superset.models.helpers import (
     ExtraJSONMixin,
     ImportExportMixin,
 )
-from superset.sql_parse import CtasMethod, ParsedQuery, Table
+from superset.sql_parse import CtasMethod, extract_tables_from_jinja_sql, Table
 from superset.sqllab.limiting_factor import LimitingFactor
 from superset.utils.core import get_column_name, MediumText, QueryStatus, user_label
 
@@ -65,8 +66,25 @@ if TYPE_CHECKING:
 logger = logging.getLogger(__name__)
 
 
+class SqlTablesMixin:  # pylint: disable=too-few-public-methods
+    @property
+    def sql_tables(self) -> list[Table]:
+        try:
+            return list(
+                extract_tables_from_jinja_sql(
+                    self.sql,  # type: ignore
+                    self.database.db_engine_spec.engine,  # type: ignore
+                )
+            )
+        except SupersetSecurityException:
+            return []
+
+
 class Query(
-    ExtraJSONMixin, ExploreMixin, Model
+    SqlTablesMixin,
+    ExtraJSONMixin,
+    ExploreMixin,
+    Model,
 ):  # pylint: disable=abstract-method,too-many-public-methods
     """ORM model for SQL query
 
@@ -181,10 +199,6 @@ class Query(
     def username(self) -> str:
         return self.user.username
 
-    @property
-    def sql_tables(self) -> list[Table]:
-        return list(ParsedQuery(self.sql, engine=self.db_engine_spec.engine).tables)
-
     @property
     def columns(self) -> list["TableColumn"]:
         from superset.connectors.sqla.models import (  # pylint: disable=import-outside-toplevel
@@ -355,7 +369,13 @@ class Query(
         return self.make_sqla_column_compatible(sqla_column, label)
 
 
-class SavedQuery(AuditMixinNullable, ExtraJSONMixin, ImportExportMixin, Model):
+class SavedQuery(
+    SqlTablesMixin,
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+    Model,
+):
     """ORM model for SQL query"""
 
     __tablename__ = "saved_query"
@@ -425,12 +445,6 @@ class SavedQuery(AuditMixinNullable, ExtraJSONMixin, ImportExportMixin, Model):
     def url(self) -> str:
         return f"/sqllab?savedQueryId={self.id}"
 
-    @property
-    def sql_tables(self) -> list[Table]:
-        return list(
-            ParsedQuery(self.sql, engine=self.database.db_engine_spec.engine).tables
-        )
-
     @property
     def last_run_humanized(self) -> str:
         return naturaltime(datetime.now() - self.changed_on)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index a532431433..2833e88645 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -52,14 +52,12 @@ from sqlalchemy.orm import eagerload
 from sqlalchemy.orm.mapper import Mapper
 from sqlalchemy.orm.query import Query as SqlaQuery
 
-from superset import sql_parse
 from superset.constants import RouteMethod
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
 from superset.exceptions import (
     DatasetInvalidPermissionEvaluationException,
     SupersetSecurityException,
 )
-from superset.jinja_context import get_template_processor
 from superset.security.guest_token import (
     GuestToken,
     GuestTokenResources,
@@ -68,6 +66,7 @@ from superset.security.guest_token import (
     GuestTokenUser,
     GuestUser,
 )
+from superset.sql_parse import extract_tables_from_jinja_sql
 from superset.superset_typing import Metric
 from superset.utils.core import (
     DatasourceName,
@@ -1961,16 +1960,12 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
                 return
 
             if query:
-                # make sure the quuery is valid SQL by rendering any Jinja
-                processor = get_template_processor(database=query.database)
-                rendered_sql = processor.process_template(query.sql)
                 default_schema = database.get_default_schema_for_query(query)
                 tables = {
                     Table(table_.table, table_.schema or default_schema)
-                    for table_ in sql_parse.ParsedQuery(
-                        rendered_sql,
-                        engine=database.db_engine_spec.engine,
-                    ).tables
+                    for table_ in extract_tables_from_jinja_sql(
+                        query.sql, database.db_engine_spec.engine
+                    )
                 }
             elif table:
                 tables = {table}
diff --git a/superset/sql_parse.py b/superset/sql_parse.py
index db51991e22..58bca48a6e 100644
--- a/superset/sql_parse.py
+++ b/superset/sql_parse.py
@@ -16,16 +16,19 @@
 # under the License.
 
 # pylint: disable=too-many-lines
+from __future__ import annotations
 
 import logging
 import re
 import urllib.parse
 from collections.abc import Iterable, Iterator
 from dataclasses import dataclass
-from typing import Any, cast, Optional
+from typing import Any, cast
+from unittest.mock import Mock
 
 import sqlparse
 from flask_babel import gettext as __
+from jinja2 import nodes
 from sqlalchemy import and_
 from sqlglot import exp, parse, parse_one
 from sqlglot.dialects import Dialects
@@ -142,7 +145,7 @@ class CtasMethod(StrEnum):
     VIEW = "VIEW"
 
 
-def _extract_limit_from_query(statement: TokenList) -> Optional[int]:
+def _extract_limit_from_query(statement: TokenList) -> int | None:
     """
     Extract limit clause from SQL statement.
 
@@ -163,9 +166,7 @@ def _extract_limit_from_query(statement: TokenList) -> Optional[int]:
     return None
 
 
-def extract_top_from_query(
-    statement: TokenList, top_keywords: set[str]
-) -> Optional[int]:
+def extract_top_from_query(statement: TokenList, top_keywords: set[str]) -> int | None:
     """
     Extract top clause value from SQL statement.
 
@@ -189,7 +190,7 @@ def extract_top_from_query(
     return top
 
 
-def get_cte_remainder_query(sql: str) -> tuple[Optional[str], str]:
+def get_cte_remainder_query(sql: str) -> tuple[str | None, str]:
     """
     parse the SQL and return the CTE and rest of the block to the caller
 
@@ -197,7 +198,7 @@ def get_cte_remainder_query(sql: str) -> tuple[Optional[str], str]:
     :return: CTE and remainder block to the caller
 
     """
-    cte: Optional[str] = None
+    cte: str | None = None
     remainder = sql
     stmt = sqlparse.parse(sql)[0]
 
@@ -215,7 +216,7 @@ def get_cte_remainder_query(sql: str) -> tuple[Optional[str], str]:
     return cte, remainder
 
 
-def strip_comments_from_sql(statement: str, engine: Optional[str] = None) -> str:
+def strip_comments_from_sql(statement: str, engine: str | None = None) -> str:
     """
     Strips comments from a SQL statement, does a simple test first
     to avoid always instantiating the expensive ParsedQuery constructor
@@ -239,8 +240,8 @@ class Table:
     """
 
     table: str
-    schema: Optional[str] = None
-    catalog: Optional[str] = None
+    schema: str | None = None
+    catalog: str | None = None
 
     def __str__(self) -> str:
         """
@@ -262,7 +263,7 @@ class ParsedQuery:
         self,
         sql_statement: str,
         strip_comments: bool = False,
-        engine: Optional[str] = None,
+        engine: str | None = None,
     ):
         if strip_comments:
             sql_statement = sqlparse.format(sql_statement, strip_comments=True)
@@ -271,7 +272,7 @@ class ParsedQuery:
         self._dialect = SQLGLOT_DIALECTS.get(engine) if engine else None
         self._tables: set[Table] = set()
         self._alias_names: set[str] = set()
-        self._limit: Optional[int] = None
+        self._limit: int | None = None
 
         logger.debug("Parsing with sqlparse statement: %s", self.sql)
         self._parsed = sqlparse.parse(self.stripped())
@@ -382,7 +383,7 @@ class ParsedQuery:
         return source.name in ctes_in_scope
 
     @property
-    def limit(self) -> Optional[int]:
+    def limit(self) -> int | None:
         return self._limit
 
     def _get_cte_tables(self, parsed: dict[str, Any]) -> list[dict[str, Any]]:
@@ -463,7 +464,7 @@ class ParsedQuery:
 
         return True
 
-    def get_inner_cte_expression(self, tokens: TokenList) -> Optional[TokenList]:
+    def get_inner_cte_expression(self, tokens: TokenList) -> TokenList | None:
         for token in tokens:
             if self._is_identifier(token):
                 for identifier_token in token.tokens:
@@ -527,7 +528,7 @@ class ParsedQuery:
         return statements
 
     @staticmethod
-    def get_table(tlist: TokenList) -> Optional[Table]:
+    def get_table(tlist: TokenList) -> Table | None:
         """
         Return the table if valid, i.e., conforms to the [[catalog.]schema.]table
         construct.
@@ -563,7 +564,7 @@ class ParsedQuery:
     def as_create_table(
         self,
         table_name: str,
-        schema_name: Optional[str] = None,
+        schema_name: str | None = None,
         overwrite: bool = False,
         method: CtasMethod = CtasMethod.TABLE,
     ) -> str:
@@ -723,8 +724,8 @@ def add_table_name(rls: TokenList, table: str) -> None:
 def get_rls_for_table(
     candidate: Token,
     database_id: int,
-    default_schema: Optional[str],
-) -> Optional[TokenList]:
+    default_schema: str | None,
+) -> TokenList | None:
     """
     Given a table name, return any associated RLS predicates.
     """
@@ -770,7 +771,7 @@ def get_rls_for_table(
 def insert_rls_as_subquery(
     token_list: TokenList,
     database_id: int,
-    default_schema: Optional[str],
+    default_schema: str | None,
 ) -> TokenList:
     """
     Update a statement inplace applying any associated RLS predicates.
@@ -786,7 +787,7 @@ def insert_rls_as_subquery(
     This method is safer than ``insert_rls_in_predicate``, but doesn't work in all
     databases.
     """
-    rls: Optional[TokenList] = None
+    rls: TokenList | None = None
     state = InsertRLSState.SCANNING
     for token in token_list.tokens:
         # Recurse into child token list
@@ -862,7 +863,7 @@ def insert_rls_as_subquery(
 def insert_rls_in_predicate(
     token_list: TokenList,
     database_id: int,
-    default_schema: Optional[str],
+    default_schema: str | None,
 ) -> TokenList:
     """
     Update a statement inplace applying any associated RLS predicates.
@@ -873,7 +874,7 @@ def insert_rls_in_predicate(
         after:  SELECT * FROM some_table WHERE ( 1=1) AND some_table.id=42
 
     """
-    rls: Optional[TokenList] = None
+    rls: TokenList | None = None
     state = InsertRLSState.SCANNING
     for token in token_list.tokens:
         # Recurse into child token list
@@ -1007,7 +1008,7 @@ RE_JINJA_BLOCK = re.compile(r"\{[%#][^\{\}%#]+[%#]\}")
 
 def extract_table_references(
     sql_text: str, sqla_dialect: str, show_warning: bool = True
-) -> set["Table"]:
+) -> set[Table]:
     """
     Return all the dependencies from a SQL sql_text.
     """
@@ -1051,3 +1052,61 @@ def extract_table_references(
         Table(*[part["value"] for part in table["name"][::-1]])
         for table in find_nodes_by_key(tree, "Table")
     }
+
+
+def extract_tables_from_jinja_sql(sql: str, engine: str | None = None) -> set[Table]:
+    """
+    Extract all table references in the Jinjafied SQL statement.
+
+    Due to Jinja templating, a multiphase approach is necessary as the Jinjafied SQL
+    statement may represent invalid SQL which is non-parsable by SQLGlot.
+
+    Firstly, we extract any tables referenced within the confines of specific Jinja
+    macros. Secondly, we replace these non-SQL Jinja calls with a pseudo-benign SQL
+    expression to help ensure that the resulting SQL statements are parsable by
+    SQLGlot.
+
+    :param sql: The Jinjafied SQL statement
+    :param engine: The associated database engine
+    :returns: The set of tables referenced in the SQL statement
+    :raises SupersetSecurityException: If SQLGlot is unable to parse the SQL statement
+    """
+
+    from superset.jinja_context import (  # pylint: disable=import-outside-toplevel
+        get_template_processor,
+    )
+
+    # Mock the required database as the processor signature is exposed publically.
+    processor = get_template_processor(database=Mock(backend=engine))
+    template = processor.env.parse(sql)
+
+    tables = set()
+
+    for node in template.find_all(nodes.Call):
+        if isinstance(node.node, nodes.Getattr) and node.node.attr in (
+            "latest_partition",
+            "latest_sub_partition",
+        ):
+            # Extract the table referenced in the macro.
+            tables.add(
+                Table(
+                    *[
+                        remove_quotes(part)
+                        for part in node.args[0].value.split(".")[::-1]
+                        if len(node.args) == 1
+                    ]
+                )
+            )
+
+            # Replace the potentially problematic Jinja macro with some benign SQL.
+            node.__class__ = nodes.TemplateData
+            node.fields = nodes.TemplateData.fields
+            node.data = "NULL"
+
+    return (
+        tables
+        | ParsedQuery(
+            sql_statement=processor.process_template(template),
+            engine=engine,
+        ).tables
+    )
diff --git a/superset/sqllab/query_render.py b/superset/sqllab/query_render.py
index 5597bcb086..caf9a3cb2b 100644
--- a/superset/sqllab/query_render.py
+++ b/superset/sqllab/query_render.py
@@ -79,8 +79,7 @@ class SqlQueryRenderImpl(SqlQueryRender):
         sql_template_processor: BaseTemplateProcessor,
     ) -> None:
         if is_feature_enabled("ENABLE_TEMPLATE_PROCESSING"):
-            # pylint: disable=protected-access
-            syntax_tree = sql_template_processor._env.parse(rendered_query)
+            syntax_tree = sql_template_processor.env.parse(rendered_query)
             undefined_parameters = find_undeclared_variables(syntax_tree)
             if undefined_parameters:
                 self._raise_undefined_parameter_exception(
diff --git a/tests/unit_tests/sql_parse_tests.py b/tests/unit_tests/sql_parse_tests.py
index 025108e9b5..81ea0e5a7a 100644
--- a/tests/unit_tests/sql_parse_tests.py
+++ b/tests/unit_tests/sql_parse_tests.py
@@ -32,6 +32,7 @@ from superset.exceptions import (
 from superset.sql_parse import (
     add_table_name,
     extract_table_references,
+    extract_tables_from_jinja_sql,
     get_rls_for_table,
     has_table_query,
     insert_rls_as_subquery,
@@ -1874,3 +1875,43 @@ WITH t AS (
 )
 SELECT * FROM t"""
     ).is_select()
+
+
+@pytest.mark.parametrize(
+    "engine",
+    [
+        "hive",
+        "presto",
+        "trino",
+    ],
+)
+@pytest.mark.parametrize(
+    "macro",
+    [
+        "latest_partition('foo.bar')",
+        "latest_sub_partition('foo.bar', baz='qux')",
+    ],
+)
+@pytest.mark.parametrize(
+    "sql,expected",
+    [
+        (
+            "SELECT '{{{{ {engine}.{macro} }}}}'",
+            {Table(table="bar", schema="foo")},
+        ),
+        (
+            "SELECT * FROM foo.baz WHERE quux = '{{{{ {engine}.{macro} }}}}'",
+            {Table(table="bar", schema="foo"), Table(table="baz", schema="foo")},
+        ),
+    ],
+)
+def test_extract_tables_from_jinja_sql(
+    engine: str,
+    macro: str,
+    sql: str,
+    expected: set[Table],
+) -> None:
+    assert (
+        extract_tables_from_jinja_sql(sql.format(engine=engine, macro=macro), engine)
+        == expected
+    )


(superset) 02/06: fix: Volatile datasource ordering in dashboard export (#19595)

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 90afb34df59e818731921ec86dd708e13ce9187e
Author: Pat Nadolny <pa...@gmail.com>
AuthorDate: Thu Mar 21 17:06:15 2024 -0500

    fix: Volatile datasource ordering in dashboard export (#19595)
    
    Co-authored-by: Michael S. Molina <70...@users.noreply.github.com>
    (cherry picked from commit bfe55b9ded5d7efdcb7919d70d5dc14c97126afd)
---
 superset/models/dashboard.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py
index 5570e892ff..0a0d789c7a 100644
--- a/superset/models/dashboard.py
+++ b/superset/models/dashboard.py
@@ -365,8 +365,11 @@ class Dashboard(AuditMixinNullable, ImportExportMixin, Model):
             copied_dashboard.alter_params(remote_id=dashboard_id)
             copied_dashboards.append(copied_dashboard)
 
+        datasource_id_list = list(datasource_ids)
+        datasource_id_list.sort()
+
         eager_datasources = []
-        for datasource_id, _ in datasource_ids:
+        for datasource_id, _ in datasource_id_list:
             eager_datasource = SqlaTable.get_eager_sqlatable_datasource(datasource_id)
             copied_datasource = eager_datasource.copy()
             copied_datasource.alter_params(


(superset) 03/06: fix(utils): fix off-by-one error in how rolling window's min_periods truncates dataframe (#27388)

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 fe95adac988dff6198286eb90e83b944fab08af9
Author: Sam Firke <sf...@users.noreply.github.com>
AuthorDate: Thu Mar 21 18:06:36 2024 -0400

    fix(utils): fix off-by-one error in how rolling window's min_periods truncates dataframe (#27388)
    
    (cherry picked from commit d4d8625ab83168b10a5977a7cc402707b5fff2a9)
---
 superset/utils/pandas_postprocessing/rolling.py        | 2 +-
 tests/unit_tests/pandas_postprocessing/test_rolling.py | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/superset/utils/pandas_postprocessing/rolling.py b/superset/utils/pandas_postprocessing/rolling.py
index f93a047be9..775acc7aa6 100644
--- a/superset/utils/pandas_postprocessing/rolling.py
+++ b/superset/utils/pandas_postprocessing/rolling.py
@@ -97,5 +97,5 @@ def rolling(  # pylint: disable=too-many-arguments
     df_rolling = _append_columns(df, df_rolling, columns)
 
     if min_periods:
-        df_rolling = df_rolling[min_periods:]
+        df_rolling = df_rolling[min_periods - 1 :]
     return df_rolling
diff --git a/tests/unit_tests/pandas_postprocessing/test_rolling.py b/tests/unit_tests/pandas_postprocessing/test_rolling.py
index 1859b0c2c7..cd162fb2d7 100644
--- a/tests/unit_tests/pandas_postprocessing/test_rolling.py
+++ b/tests/unit_tests/pandas_postprocessing/test_rolling.py
@@ -107,7 +107,7 @@ def test_rolling():
         )
 
 
-def test_rolling_should_empty_df():
+def test_rolling_min_periods_trims_correctly():
     pivot_df = pp.pivot(
         df=single_metric_df,
         index=["dttm"],
@@ -121,7 +121,7 @@ def test_rolling_should_empty_df():
         min_periods=2,
         columns={"sum_metric": "sum_metric"},
     )
-    assert rolling_df.empty is True
+    assert len(rolling_df) == 1
 
 
 def test_rolling_after_pivot_with_single_metric():


(superset) 04/06: fix(Dashboard): Add editMode conditional for translate3d fix on charts to allow intended Fullscreen (#27613)

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 a2fb13b5227bc6bd51a705a4a150bb975a820a87
Author: Ross Mabbett <92...@users.noreply.github.com>
AuthorDate: Fri Mar 22 00:36:17 2024 -0400

    fix(Dashboard): Add editMode conditional for translate3d fix on charts to allow intended Fullscreen (#27613)
    
    (cherry picked from commit 842b0939f6a182a8f7d3c7c893200d93be3a4b0c)
---
 superset-frontend/src/dashboard/components/dnd/DragDroppable.jsx | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/superset-frontend/src/dashboard/components/dnd/DragDroppable.jsx b/superset-frontend/src/dashboard/components/dnd/DragDroppable.jsx
index bce458dfe6..b5b7373f40 100644
--- a/superset-frontend/src/dashboard/components/dnd/DragDroppable.jsx
+++ b/superset-frontend/src/dashboard/components/dnd/DragDroppable.jsx
@@ -79,7 +79,9 @@ const DragDroppableStyles = styled.div`
       preview expands outside of the bounds of the drag source card, see:
       https://github.com/react-dnd/react-dnd/issues/832#issuecomment-442071628
     */
-    transform: translate3d(0, 0, 0);
+    &.dragdroppable--edit-mode {
+      transform: translate3d(0, 0, 0);
+    }
 
     &.dragdroppable--dragging {
       opacity: 0.2;
@@ -180,6 +182,7 @@ export class UnwrappedDragDroppable extends React.PureComponent {
         data-test="dragdroppable-object"
         className={cx(
           'dragdroppable',
+          editMode && 'dragdroppable--edit-mode',
           orientation === 'row' && 'dragdroppable-row',
           orientation === 'column' && 'dragdroppable-column',
           isDragging && 'dragdroppable--dragging',


(superset) 05/06: fix: Persist query params appended to permalink (#27601)

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 4ff331a66c099fdeda7278f3b374e6b33a62a6c8
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Fri Mar 22 10:24:31 2024 +0100

    fix: Persist query params appended to permalink (#27601)
    
    (cherry picked from commit 5083ca0e819d0cb024c597735329566575beccdb)
---
 superset/views/core.py                |  2 ++
 tests/integration_tests/core_tests.py | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/superset/views/core.py b/superset/views/core.py
index cc349d9f32..4faede0f34 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -843,6 +843,8 @@ class Superset(BaseSupersetView):
         if url_params := state.get("urlParams"):
             params = parse.urlencode(url_params)
             url = f"{url}&{params}"
+        if original_params := request.query_string.decode():
+            url = f"{url}&{original_params}"
         if hash_ := state.get("anchor", state.get("hash")):
             url = f"{url}#{hash_}"
         return redirect(url)
diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py
index 573b096fc7..ef20fc25af 100644
--- a/tests/integration_tests/core_tests.py
+++ b/tests/integration_tests/core_tests.py
@@ -1185,6 +1185,21 @@ class TestCore(SupersetTestCase):
             is True
         )
 
+    @mock.patch("superset.views.core.request")
+    @mock.patch(
+        "superset.commands.dashboard.permalink.get.GetDashboardPermalinkCommand.run"
+    )
+    def test_dashboard_permalink(self, get_dashboard_permalink_mock, request_mock):
+        request_mock.query_string = b"standalone=3"
+        get_dashboard_permalink_mock.return_value = {"dashboardId": 1}
+        self.login()
+        resp = self.client.get("superset/dashboard/p/123/")
+
+        expected_url = "/superset/dashboard/1?permalink_key=123&standalone=3"
+
+        self.assertEqual(resp.headers["Location"], expected_url)
+        assert resp.status_code == 302
+
 
 if __name__ == "__main__":
     unittest.main()


(superset) 01/06: fix: sqlglot SQL Server (#27577)

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 336f6bd0219c43ce520357104158522164cc466f
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Thu Mar 21 17:09:40 2024 -0400

    fix: sqlglot SQL Server (#27577)
    
    (cherry picked from commit 72a41c16424e86c92d7423aac7e9fbab505a2c37)
---
 superset/sql_parse.py                            | 2 +-
 tests/integration_tests/charts/data/api_tests.py | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/superset/sql_parse.py b/superset/sql_parse.py
index 49ee4c491f..db51991e22 100644
--- a/superset/sql_parse.py
+++ b/superset/sql_parse.py
@@ -112,7 +112,7 @@ SQLGLOT_DIALECTS = {
     # "impala": ???
     # "kustokql": ???
     # "kylin": ???
-    # "mssql": ???
+    "mssql": Dialects.TSQL,
     "mysql": Dialects.MYSQL,
     "netezza": Dialects.POSTGRES,
     # "ocient": ???
diff --git a/tests/integration_tests/charts/data/api_tests.py b/tests/integration_tests/charts/data/api_tests.py
index 7e5d4fda7f..f53ebeffb3 100644
--- a/tests/integration_tests/charts/data/api_tests.py
+++ b/tests/integration_tests/charts/data/api_tests.py
@@ -507,6 +507,9 @@ class TestPostChartDataApi(BaseTestChartDataApi):
         """
         Chart data API: Ensure prophet post transformation works
         """
+        if backend() == "hive":
+            return
+
         time_grain = "P1Y"
         self.query_context_payload["queries"][0]["is_timeseries"] = True
         self.query_context_payload["queries"][0]["groupby"] = []
@@ -541,6 +544,9 @@ class TestPostChartDataApi(BaseTestChartDataApi):
         """
         Chart data API: Ensure incorrect post processing returns correct response
         """
+        if backend() == "hive":
+            return
+
         query_context = self.query_context_payload
         query = query_context["queries"][0]
         query["columns"] = ["name", "gender"]