You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/07/08 00:22:55 UTC

(superset) 01/01: fix(sqla): render Jinja templates in calculated columns used via orderby adhoc metrics

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch fix/issue-29378-implement-dosu-s-fix-test-first-in-super
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 5da4fe799674a7f172b0025e5ad69b1aaf0c50c8
Author: Evan <ev...@preset.io>
AuthorDate: Tue Jul 7 17:21:31 2026 -0700

    fix(sqla): render Jinja templates in calculated columns used via orderby adhoc metrics
    
    A calculated column's Jinja expression was rendered correctly in SELECT,
    WHERE, and GROUP BY, but not when the column was referenced by a SIMPLE
    adhoc metric (e.g. MAX(calc_col)) used in ORDER BY, since
    `adhoc_metric_to_sqla` wasn't passed the template processor at that call
    site in `get_sqla_query`. Add a failing-first integration test that pins
    the bug, then fix it by threading `template_processor` through.
    
    Fixes #29378
---
 superset/models/helpers.py                         |  7 ++++
 .../db_engine_specs/base_engine_spec_tests.py      | 45 ++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 43f581b6055..6e140a43770 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -3331,9 +3331,16 @@ class ExploreMixin:  # pylint: disable=too-many-public-methods
                     )
                 if utils.is_adhoc_metric(col):
                     # add adhoc sort by column to columns_by_name if not exists
+                    # Pass the template processor so a SIMPLE adhoc metric that
+                    # references a calculated column has that column's Jinja
+                    # expression rendered, matching SELECT/WHERE/GROUP BY. This
+                    # is a no-op for the SQL expression type, whose
+                    # `sqlExpression` was already rendered above; `processed`
+                    # only guards that branch.
                     col = self.adhoc_metric_to_sqla(
                         col,
                         columns_by_name,
+                        template_processor=template_processor,
                         processed=True,
                     )
                     # use the existing instance, if possible
diff --git a/tests/integration_tests/db_engine_specs/base_engine_spec_tests.py b/tests/integration_tests/db_engine_specs/base_engine_spec_tests.py
index d36e75e08e3..22032b754ec 100644
--- a/tests/integration_tests/db_engine_specs/base_engine_spec_tests.py
+++ b/tests/integration_tests/db_engine_specs/base_engine_spec_tests.py
@@ -202,6 +202,51 @@ class SupersetTestCases(SupersetTestCase):
             in sql
         )
 
+    @mock.patch("superset.models.core.Database.db_engine_spec", BaseEngineSpec)
+    @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+    def test_jinja_calculated_column_in_order_by(self):
+        """
+        A calculated column referenced by a SIMPLE adhoc metric in `orderby`
+        should have its Jinja template rendered, the same way it is for
+        SELECT/WHERE/GROUP BY.
+        """
+        table = self.get_table(name="birth_names")
+        TableColumn(
+            column_name="gender_cc_jinja",
+            type="VARCHAR(255)",
+            table=table,
+            expression="""
+            case
+              when gender='boy' then {{ "'male'" }}
+              else {{ "'female'" }}
+            end
+            """,
+        )
+
+        table.database.sqlalchemy_uri = "sqlite://"
+        query_obj = {
+            "groupby": ["gender_cc_jinja"],
+            "is_timeseries": False,
+            "filter": [],
+            "orderby": [
+                [
+                    {
+                        "expressionType": "SIMPLE",
+                        "column": {"column_name": "gender_cc_jinja"},
+                        "aggregate": "MAX",
+                        "label": "max_gender_cc_jinja",
+                    },
+                    True,
+                ]
+            ],
+        }
+        sql = table.get_query_str(query_obj)
+        orderby_clause = sql.split("ORDER BY")[1]
+        assert "{%" not in orderby_clause
+        assert "{{" not in orderby_clause
+        assert "'male'" in orderby_clause
+        assert "'female'" in orderby_clause
+
 
 def test_time_grain_denylist():
     config = app.config.copy()