Bug description

superset modifies GROUP_CONCAT when creating charts by adding the SEPARATOR ',' syntax at the end, but this is not supported in Doris. This happens not only in Doris, also in mysql, so it can be reproduced using mysql.

How to reproduce the bug

  1. create a virtual sql dataset and write something like sql
select 
       GROUP_CONCAT( app) a
from b
GROUP BY app
  1. Create a chart casually, and then create a chart
  2. Click to view the query statement You will find that GROUP_CONCAT has been modified, similar to the following
select 
       GROUP_CONCAT( app SEPARATOR ',') a
from b
GROUP BY app

GROUP_CONCAT has SEPARATOR ',' automatically added to it, resulting in a query error.

Screenshots/recordings

No response

Superset version

master / latest-dev

Python version

3.9

Node version

16

Browser

Chrome

Additional context

No response

Checklist

  • [X] I have searched Superset docs and Slack and didn't find a solution to my problem.
  • [X] I have searched the GitHub issue tracker and didn't find a similar bug report.
  • [X] I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.

Comment From: dosubot[bot]

To address the issue of Superset automatically appending SEPARATOR ',' to GROUP_CONCAT, you can use the SQL_QUERY_MUTATOR function in the superset/config.py file. This function allows you to modify the SQL query before it is executed, enabling you to remove the SEPARATOR ',' clause from GROUP_CONCAT functions.

Here is how you can modify the SQL_QUERY_MUTATOR function in your superset/config.py:

  1. Edit superset/config.py: Add the following code to define a custom SQL_QUERY_MUTATOR function that removes the SEPARATOR ',' clause from GROUP_CONCAT:

    ```python import re from typing import Any

    def SQL_QUERY_MUTATOR(sql: str, kwargs: Any) -> str: # Remove SEPARATOR ',' from GROUP_CONCAT sql = re.sub(r'GROUP_CONCAT((.?)\s+SEPARATOR\s,\s*\'\')', r'GROUP_CONCAT(\1)', sql, flags=re.IGNORECASE) return sql ```

  2. Restart Superset: After making this change, restart your Superset instance to apply the new configuration.

This function uses a regular expression to find instances of GROUP_CONCAT with the SEPARATOR ',' clause and removes the SEPARATOR ',' part. This way, the modified SQL query will not include the SEPARATOR ',' clause, preventing query errors in both Doris and MySQL.

#### Details **Sources** - [superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts](https://github.com/apache/superset/blob/master/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts) - [superset/config.py](https://github.com/apache/superset/blob/master/superset/config.py) - [docs/docs/contributing/contributing.mdx](https://github.com/apache/superset/blob/master/docs/docs/contributing/contributing.mdx) - [CONTRIBUTING.md](https://github.com/apache/superset/blob/master/CONTRIBUTING.md) - [docs/docs/contributing/development.mdx](https://github.com/apache/superset/blob/master/docs/docs/contributing/development.mdx) - [docs/docs/contributing/guidelines.mdx](https://github.com/apache/superset/blob/master/docs/docs/contributing/guidelines.mdx) - [superset/models/core.py](https://github.com/apache/superset/blob/master/superset/models/core.py) - [superset/sql_parse.py](https://github.com/apache/superset/blob/master/superset/sql_parse.py) - [superset/db_engine_specs/base.py](https://github.com/apache/superset/blob/master/superset/db_engine_specs/base.py) - [superset/extensions/metadb.py](https://github.com/apache/superset/blob/master/superset/extensions/metadb.py) - [superset/db_engine_specs/lib.py](https://github.com/apache/superset/blob/master/superset/db_engine_specs/lib.py)
**About Dosu** This response is meant to be useful and save you time. It is **not meant to be a precise solution**, but rather a starting point for your own research. Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

To continue the conversation, mention @dosu.

Comment From: rusackas

CC @liujiwen-up

Comment From: rusackas

@dosubot can we resolve this at the dbapi / db engine spec level?

Anyone interested in tackling this? It was at risk of being closed as stale, but it sounds like it's worth further investigation.

Comment From: dosubot[bot]

Resolving the issue at the dbapi or db engine spec level involves modifying how Superset handles SQL functions like GROUP_CONCAT for specific database engines. Superset uses the BaseEngineSpec class to provide database engine-specific configurations, including SQL function handling [1].

To address the GROUP_CONCAT issue without relying on SQL_QUERY_MUTATOR, you can extend the relevant engine spec class (e.g., DorisEngineSpec or MySQLEngineSpec) to customize the SQL function handling for GROUP_CONCAT. This involves overriding methods that parse or modify SQL queries to ensure compatibility with the specific SQL dialect of Doris or MySQL.

Here's a general approach:

  1. Identify the Engine Spec Class: Locate the engine spec class for Doris or MySQL in the Superset codebase (e.g., doris.py or mysql.py).

  2. Override SQL Handling Methods: Modify or add methods in the engine spec class to handle GROUP_CONCAT without appending SEPARATOR ','. This might involve customizing the SQL parsing logic or query transformation methods.

  3. Test the Changes: Ensure that the changes work correctly by testing with both Doris and MySQL databases to verify that GROUP_CONCAT is handled as expected.

By implementing these changes at the db engine spec level, you can ensure that Superset generates compatible SQL queries for the affected databases without relying on global query mutators.

To continue talking to Dosu, mention @dosu.

Help Dosu learn! Give it feedback: Great Response | Irrelevant Answer | Incorrect Sources | Too Verbose | Hallucination | Bug Report | Other

Comment From: rusackas

Closing this as inactive, but I HOPE that the new SQLGlot migration solves this anyway. Holler if you still see this on master (or 5.1 or 6.0 when the day comes)