apache / superset

Apache Superset is a Data Visualization and Data Exploration Platform
https://superset.apache.org/
Apache License 2.0
62k stars 13.59k forks source link

Doris made an error in creating a chart using the GROUP_CONCAT syntax #28630

Open gaoyan1998 opened 4 months ago

gaoyan1998 commented 4 months ago

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
  2. Create a chart casually, and then create a chart
  3. 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

dosubot[bot] commented 4 months ago

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:

    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.

rusackas commented 4 months ago

CC @liujiwen-up