The Force Categorical Control is wrongly hidden in bar chart.

snippins commented 2 months ago

Bug description

This is due to the snippet of code here in customControls.tsx

export const xAxisForceCategoricalControl = {
  name: 'xAxisForceCategorical',
  config: {
    type: 'CheckboxControl',
    label: () => t('Force categorical'),
    default: false,
    description: t('Treat values as categorical.'),
    initialValue: (control: ControlState, state: ControlPanelState | null) =>
      state?.form_data?.x_axis_sort !== undefined || control.value,
    renderTrigger: true,
    visibility: ({ controls }: { controls: ControlStateMapping }) =>
      checkColumnType(
        getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
        controls?.datasource?.datasource,
        [GenericDataType.Numeric],
      ),
    shouldMapStateToProps: () => true,
  },
};

It seems that for bar chart we need to check the type of the y_axis instead?

How to reproduce the bug

Create a bar chart with the x axis of type var_string (varchar in starrocks) and y axis of a numeric type.

=> the xAxisForceCategoricalControl got hidden.

If I cast the x axis to int for example then the control is shown, but that does not make much sense as we are trying to sort the value of the y axis.

This is probably a related report, but mine seems like a more accurate description of the bug, but I don't really know why they're able to work around using the string datatype, as it is not a numeric type as in the visibility check.

https://github.com/apache/superset/issues/30238

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.

dosubot[bot] commented 2 months ago

Hey @snippins! I'm here to help you with this bug. I'm a bot designed to assist with bug solving, answer questions, and guide you to become a contributor. Let's tackle this issue together!

The xAxisForceCategoricalControl is indeed incorrectly hidden in a bar chart with a categorical x-axis and a numeric y-axis because its visibility check only considers the x-axis type. The visibility check should also consider the y-axis type to ensure the control is shown appropriately.

Currently, the visibility check is:

visibility: ({ controls }: { controls: ControlStateMapping }) =>
  checkColumnType(
    getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
    controls?.datasource?.datasource,
    [GenericDataType.Numeric],
  ),

To fix this, the visibility check should be updated to also consider the y-axis type. Here is the revised version:

visibility: ({ controls }: { controls: ControlStateMapping }) =>
  checkColumnType(
    getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
    controls?.datasource?.datasource,
    [GenericDataType.Numeric],
  ) && checkColumnType(
    getColumnLabel(controls?.y_axis?.value as QueryFormColumn),
    controls?.datasource?.datasource,
    [GenericDataType.Numeric],
  ),

This ensures that the xAxisForceCategoricalControl is visible only when both the x-axis and y-axis columns are numeric, which is appropriate for a bar chart with a categorical x-axis and a numeric y-axis ^[1].

To continue talking to Dosu, mention @dosu.

snippins commented 2 months ago

@dosu My understanding was completely wrong, it is just that the superset was not able to understand that var_string from mysql or starrocks as a string type, so the sort control is hidden.

I tested by adding a mapping for var_string to this file and it works, this probably would also works for mysql: https://github.com/apache/superset/blob/master/superset/db_engine_specs/starrocks.py

        (
            re.compile(r"^var_string", re.IGNORECASE),
            types.VARCHAR(),
            GenericDataType.STRING,
        ),

dosubot[bot] commented 2 months ago

To ensure that Superset recognizes var_string as a string type and enables the sort control functionality, you need to add a mapping for var_string in the starrocks.py file. Here is the updated code snippet:

class StarRocksEngineSpec(MySQLEngineSpec):
    engine = "starrocks"
    engine_name = "StarRocks"

    default_driver = "starrocks"
    sqlalchemy_uri_placeholder = (
        "starrocks://user:password@host:port/catalog.db[?key=value&key=value...]"
    )

    column_type_mappings = (  # type: ignore
        (
            re.compile(r"^tinyint", re.IGNORECASE),
            TINYINT(),
            GenericDataType.NUMERIC,
        ),
        (
            re.compile(r"^largeint", re.IGNORECASE),
            LARGEINT(),
            GenericDataType.NUMERIC,
        ),
        (
            re.compile(r"^decimal.*", re.IGNORECASE),
            types.DECIMAL(),
            GenericDataType.NUMERIC,
        ),
        (
            re.compile(r"^double", re.IGNORECASE),
            DOUBLE(),
            GenericDataType.NUMERIC,
        ),
        (
            re.compile(r"^varchar(\((\d+)\))*$", re.IGNORECASE),
            types.VARCHAR(),
            GenericDataType.STRING,
        ),
        (
            re.compile(r"^char(\((\d+)\))*$", re.IGNORECASE),
            types.CHAR(),
            GenericDataType.STRING,
        ),
        (
            re.compile(r"^json", re.IGNORECASE),
            types.JSON(),
            GenericDataType.STRING,
        ),
        (
            re.compile(r"^binary.*", re.IGNORECASE),
            types.String(),
            GenericDataType.STRING,
        ),
        (
            re.compile(r"^percentile", re.IGNORECASE),
            PERCENTILE(),
            GenericDataType.STRING,
        ),
        (re.compile(r"^hll", re.IGNORECASE), HLL(), GenericDataType.STRING),
        (re.compile(r"^bitmap", re.IGNORECASE), BITMAP(), GenericDataType.STRING),
        (re.compile(r"^array.*", re.IGNORECASE), ARRAY(), GenericDataType.STRING),
        (re.compile(r"^map.*", re.IGNORECASE), MAP(), GenericDataType.STRING),
        (re.compile(r"^struct.*", re.IGNORECASE), STRUCT(), GenericDataType.STRING),
        (
            re.compile(r"^var_string", re.IGNORECASE),
            types.VARCHAR(),
            GenericDataType.STRING,
        ),
    )

This addition ensures that var_string is correctly interpreted as a string type, enabling the sort control functionality in Superset ^[1].

To continue talking to Dosu, mention @dosu.

goldjee commented 2 months ago

Seems that the same thing happens when I use a calculated column for categorical axis. I just don't see "Y-axis sort by" selector. When I use regular string column provided by the query itself, everything works just fine.

apache / superset