You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2026/03/26 14:56:00 UTC
(superset) branch fix-type-probe created (now 790d7a73802)
This is an automated email from the ASF dual-hosted git repository.
beto pushed a change to branch fix-type-probe
in repository https://gitbox.apache.org/repos/asf/superset.git
at 790d7a73802 fix(explore): display actual data type instead of "column" in column tooltip (#38554)
This branch includes the following new commits:
new 790d7a73802 fix(explore): display actual data type instead of "column" in column tooltip (#38554)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix(explore): display actual data type instead of "column" in column tooltip (#38554)
Posted by be...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch fix-type-probe
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 790d7a7380261b7d28262fe8b9b8190732b64c8a
Author: Levis Mbote <11...@users.noreply.github.com>
AuthorDate: Mon Mar 23 19:06:54 2026 +0300
fix(explore): display actual data type instead of "column" in column tooltip (#38554)
---
.../src/components/ColumnOption.tsx | 2 +-
.../src/components/labelUtils.tsx | 23 ++++++-
.../test/components/labelUtils.test.tsx | 19 ++++++
superset/connectors/sqla/models.py | 7 +-
tests/unit_tests/models/helpers_test.py | 77 ++++++++++++++++++++++
5 files changed, 124 insertions(+), 4 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx
index ca3c9edb164..17f02eecbd5 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx
@@ -59,7 +59,7 @@ export function ColumnOption({
const type = hasExpression ? 'expression' : type_generic;
const [tooltipText, setTooltipText] = useState<ReactNode>(column.column_name);
const [columnTypeTooltipText, setcolumnTypeTooltipText] = useState<ReactNode>(
- column.type,
+ getColumnTypeTooltipNode(column),
);
useLayoutEffect(() => {
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx
index d4a1aec7fe7..ffee313a4df 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx
@@ -20,6 +20,7 @@ import { ReactNode, RefObject } from 'react';
import { t } from '@apache-superset/core/translation';
import { css, styled } from '@apache-superset/core/theme';
+import { GenericDataType } from '@apache-superset/core/common';
import { ColumnMeta, Metric } from '@superset-ui/chart-controls';
const TooltipSectionWrapper = styled.div`
@@ -64,11 +65,29 @@ export const getColumnLabelText = (column: ColumnMeta): string =>
column.verbose_name || column.column_name;
export const getColumnTypeTooltipNode = (column: ColumnMeta): ReactNode => {
- if (!column.type) {
+ const rawType = typeof column.type === 'string' ? column.type.trim() : '';
+
+ let typeLabel: ReactNode | null = null;
+
+ if (rawType && rawType.toLowerCase() !== 'column') {
+ typeLabel = rawType;
+ } else if (typeof column.type_generic === 'number') {
+ if (column.type_generic === GenericDataType.String) {
+ typeLabel = t('string');
+ } else if (column.type_generic === GenericDataType.Numeric) {
+ typeLabel = t('numeric');
+ } else if (column.type_generic === GenericDataType.Temporal) {
+ typeLabel = t('timestamp');
+ } else if (column.type_generic === GenericDataType.Boolean) {
+ typeLabel = t('boolean');
+ }
+ }
+
+ if (!typeLabel) {
return null;
}
- return <TooltipSection label={t('Column type')} text={column.type} />;
+ return <TooltipSection label={t('Column type')} text={typeLabel} />;
};
export const getColumnTooltipNode = (
diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx
index 64e75887abd..ae050c820ed 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx
@@ -24,6 +24,7 @@ import {
getMetricTooltipNode,
getColumnTypeTooltipNode,
} from '../../src/components/labelUtils';
+import { GenericDataType } from '@apache-superset/core/common';
test("should get column name when column doesn't have verbose_name", () => {
expect(
@@ -89,6 +90,24 @@ test('should get column datatype rendered as tooltip when column has a type', ()
expect(screen.getByText('text')).toBeVisible();
});
+test('should fall back to generic data type label when type is "column"', () => {
+ render(
+ <>
+ {getColumnTypeTooltipNode({
+ id: 123,
+ column_name: 'column name',
+ verbose_name: '',
+ description: '',
+ type: 'column',
+ type_generic: GenericDataType.String,
+ })}
+ </>,
+ );
+
+ expect(screen.getByText('Column type')).toBeVisible();
+ expect(screen.getByText('string')).toBeVisible();
+});
+
test('should get column name, verbose name and description when it has a verbose name', () => {
const ref = { current: { scrollWidth: 100, clientWidth: 100 } };
render(
diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index b49e9ba4d33..ba760142d93 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -1652,8 +1652,13 @@ class SqlaTable(
if has_timegrain or force_type_check:
try:
# probe adhoc column type
+ # Use WHERE false so the database returns no rows but still
+ # populates cursor.description with column type metadata.
+ # This avoids scanning the table, which can be very expensive
+ # on databases that enforce row-read limits (e.g. ClickHouse's
+ # max_rows_to_read) even for LIMIT 1 queries on large tables.
tbl, _ = self.get_from_clause(template_processor)
- qry = sa.select([sqla_column]).limit(1).select_from(tbl)
+ qry = sa.select([sqla_column]).where(sa.false()).select_from(tbl)
sql = self.database.compile_sqla_query(
qry,
catalog=self.catalog,
diff --git a/tests/unit_tests/models/helpers_test.py b/tests/unit_tests/models/helpers_test.py
index c0354844737..fe9d7aa1d19 100644
--- a/tests/unit_tests/models/helpers_test.py
+++ b/tests/unit_tests/models/helpers_test.py
@@ -1856,3 +1856,80 @@ def test_extras_having_is_parenthesized(
assert "(COUNT(*) > 0 OR 1 = 1)" in sql, (
f"extras.having should be wrapped in parentheses. Generated SQL: {sql}"
)
+
+
+def test_adhoc_column_type_probe_uses_where_false(database: Database) -> None:
+ """
+ Test that the adhoc column type probe query uses WHERE false instead of LIMIT 1.
+
+ When a chart uses a SQL expression as a BASE_AXIS column with a timeGrain set,
+ Superset probes its return type to decide whether to apply timestamp bucketing.
+ Previously this probe ran `SELECT <expr> FROM table LIMIT 1` with no WHERE clause,
+ causing a full table scan on databases that enforce row-read limits (e.g.
+ ClickHouse's max_rows_to_read) even for LIMIT 1 queries on very large tables.
+
+ The fix uses WHERE false so the database returns zero rows but still populates
+ cursor.description with column type metadata, which is all we need.
+ """
+ from unittest.mock import patch
+
+ from superset.connectors.sqla.models import SqlaTable, TableColumn
+
+ table = SqlaTable(
+ database=database,
+ schema=None,
+ table_name="t",
+ columns=[
+ TableColumn(column_name="a", type="INTEGER"),
+ ],
+ )
+
+ # A numeric SQL expression used as BASE_AXIS with a timeGrain
+ adhoc_col: AdhocColumn = {
+ "sqlExpression": "round(a / 50) * 50 / 1000",
+ "label": "Duration",
+ "columnType": "BASE_AXIS",
+ "timeGrain": "P1D",
+ }
+
+ captured_queries: list[str] = []
+
+ # Mock get_columns_description so we can inspect the SQL it receives
+ # without needing a real database connection, and return a non-temporal type
+ # so the column is left as-is (no timestamp bucketing applied).
+ def fake_get_columns_description(
+ db: object,
+ catalog: object,
+ schema: object,
+ sql: str,
+ ) -> list[dict[str, object]]:
+ captured_queries.append(sql)
+ return [
+ {
+ "column_name": "Duration",
+ "name": "Duration",
+ "type": "FLOAT",
+ "is_dttm": False,
+ }
+ ]
+
+ with patch(
+ "superset.connectors.sqla.models.get_columns_description",
+ side_effect=fake_get_columns_description,
+ ):
+ result = table.adhoc_column_to_sqla(adhoc_col)
+
+ assert result is not None
+ assert len(captured_queries) == 1, "Expected exactly one type-probe query"
+
+ probe_sql = captured_queries[0].lower()
+
+ # The probe must contain a WHERE clause that is always false — zero rows are read
+ # from the table. SQLAlchemy renders sa.false() differently per dialect:
+ # - Most databases: WHERE false
+ # - SQLite: WHERE 0 = 1
+ always_false_patterns = ("where false", "where 0 = 1")
+ assert any(p in probe_sql for p in always_false_patterns), (
+ f"Probe query should use a WHERE false condition to avoid scanning the table, "
+ f"but got: {captured_queries[0]}"
+ )