(superset) 01/01: fix(charts): Fix unquoted 'Others' literal in series limit GROUP BY clause

[ma...@apache.org]

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

maximebeauchemin pushed a commit to branch series_limit
in repository https://gitbox.apache.org/repos/asf/superset.git

commit da011aa606763d14a10f1c90df344a6851b821dc
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Tue Jul 29 15:56:16 2025 -0700

    fix(charts): Fix unquoted 'Others' literal in series limit GROUP BY clause
    
    When using the "Group remaining as Others" feature with series limits,
    the generated SQL contained an unquoted literal in the GROUP BY clause,
    causing queries to fail.
    
    Problem:
    - SQL was generated with: ELSE Others END (unquoted)
    - Should be: ELSE 'Others' END (quoted string literal)
    
    Root cause:
    1. Used sa.literal_column("Others") which treats "Others" as a column name
    2. make_sqla_column_compatible interfered with proper literal quoting in GROUP BY
    
    Solution:
    1. Changed to sa.literal("Others") for proper string literal handling
    2. Removed make_sqla_column_compatible from GROUP BY expressions since:
       - GROUP BY doesn't need aliases/labels
       - The labeling was interfering with SQLAlchemy's literal compilation
    
    Related to #34308 which introduced the "Group remaining as Others" feature.
---
 superset/models/helpers.py | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 6b7c07b784..400255dd07 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -1290,9 +1290,7 @@ class ExploreMixin:  # pylint: disable=too-many-public-methods
                 condition = condition_factory(expr.name, expr)
 
                 # Create CASE expression: condition true -> original, else "Others"
-                case_expr = sa.case(
-                    [(condition, expr)], else_=sa.literal_column("Others")
-                )
+                case_expr = sa.case([(condition, expr)], else_=sa.literal("Others"))
                 case_expr = self.make_sqla_column_compatible(case_expr, expr.name)
                 modified_select_exprs.append(case_expr)
             else:
@@ -1308,9 +1306,13 @@ class ExploreMixin:  # pylint: disable=too-many-public-methods
                 # Create CASE expression for groupby
                 case_expr = sa.case(
                     [(condition, gby_expr)],
-                    else_=sa.literal_column("Others"),
+                    else_=sa.literal("Others"),
                 )
-                case_expr = self.make_sqla_column_compatible(case_expr, col_name)
+                # Don't apply make_sqla_column_compatible to GROUP BY expressions.
+                # When make_sqla_column_compatible adds a label to the expression,
+                # it can cause SQLAlchemy to incorrectly render string literals
+                # without quotes in the GROUP BY clause (e.g., "ELSE Others"
+                # instead of "ELSE 'Others'")
                 modified_groupby_all_columns[col_name] = case_expr
             else:
                 modified_groupby_all_columns[col_name] = gby_expr