Compare commits

...

9 Commits

Author SHA1 Message Date
Evan
bade51a42a fix(ag-grid-table): reject null/empty IN_RANGE bounds and tighten tests
Number(null) and Number('') both coerce to 0, so a null/empty filterTo
would silently produce "BETWEEN x AND 0" instead of dropping the clause.
Explicitly reject non-coercible (null/empty) bounds before coercion.

Also strengthen the converter tests to assert the surviving sibling
condition and the full compound WHERE clause, so they fail if filter
composition or the upper bound regresses.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 08:29:47 -07:00
Claude Code
ace7f72f29 fix(ag-grid-table): validate numeric bounds in IN_RANGE filter clause
simpleFilterToWhereClause interpolated the IN_RANGE (BETWEEN) bounds directly
into the WHERE clause string without escaping or type validation, unlike the
ILIKE and string-comparison branches which escape via escapeSQLString. The
range values come from the AG Grid filter model, which is client-controlled
and serialized into the query, and filterTo was never validated at all.

Coerce both bounds with Number() and emit the BETWEEN clause only when both
are finite, dropping the condition otherwise. Numeric strings from serialized
filter state still work; non-numeric input can no longer be concatenated as
raw SQL. Date ranges are unaffected (handled separately and normalized through
Date.toISOString()).

Add regression tests: a compound inRange filter with a SQL payload in the
bounds is dropped, and numeric/numeric-string bounds still produce the
expected BETWEEN clause.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-01 16:18:27 -07:00
dependabot[bot]
a183582291 chore(deps): bump markdown-to-jsx from 9.8.0 to 9.8.1 in /superset-frontend (#40316)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-05-20 22:33:08 -07:00
dependabot[bot]
3acef94ef6 chore(deps): update zod requirement from ^4.4.1 to ^4.4.3 in /superset-frontend/plugins/plugin-chart-echarts (#40313)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-05-20 22:32:46 -07:00
dependabot[bot]
9638eecdb1 chore(deps-dev): bump oxlint from 1.65.0 to 1.66.0 in /superset-frontend (#40318)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-05-20 22:29:45 -07:00
Evan Rusackas
7e74fc4192 fix(charts): handle PostgreSQL INTERVAL type in bar and pie charts (#34513)
Co-authored-by: Claude <noreply@anthropic.com>
2026-05-20 22:26:59 -07:00
Evan Rusackas
cdca6f7fdc fix(sqllab): keep saved-query list working when Jinja dataset(id) references a deleted dataset (#39703)
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-20 21:19:19 -07:00
Maxime Beauchemin
b1ca8cac6b fix(tests): fix flaky FileHandler test by awaiting LaunchQueue consumer in afterEach (#39508)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: sadpandajoe <jcli38@gmail.com>
2026-05-20 19:31:01 -07:00
Evan Rusackas
2cd5efa627 ci(deps): bump lower bound on pip dependabot PRs (#40308)
Co-authored-by: Claude Code <noreply@anthropic.com>
2026-05-21 08:53:57 +07:00
12 changed files with 605 additions and 444 deletions

View File

@@ -62,6 +62,11 @@ updates:
- package-ecosystem: "pip"
directory: "/"
open-pull-requests-limit: 10
# Bump the lower bound to the new version, not just widen the upper
# bound. Without this, a `sqlglot>=28.10.0, <29` constraint upgraded
# to `<30` would keep the stale lower bound forever, dragging
# transitively-resolved versions with it. See #40186 (review thread).
versioning-strategy: increase
schedule:
interval: "weekly"
labels:

File diff suppressed because it is too large Load Diff

View File

@@ -191,7 +191,7 @@
"json-stringify-pretty-compact": "^2.0.0",
"lodash": "^4.18.1",
"mapbox-gl": "^3.24.0",
"markdown-to-jsx": "^9.8.0",
"markdown-to-jsx": "^9.8.1",
"match-sorter": "^8.3.0",
"memoize-one": "^5.2.1",
"mousetrap": "^1.6.5",
@@ -350,7 +350,7 @@
"lightningcss": "^1.32.0",
"mini-css-extract-plugin": "^2.10.2",
"open-cli": "^9.0.0",
"oxlint": "^1.65.0",
"oxlint": "^1.66.0",
"po2json": "^0.4.5",
"prettier": "3.8.3",
"prettier-plugin-packagejson": "^3.0.2",

View File

@@ -379,7 +379,23 @@ function simpleFilterToWhereClause(
}
if (type === FILTER_OPERATORS.IN_RANGE && filterTo !== undefined) {
return `${columnName} ${SQL_OPERATORS.BETWEEN} ${value} AND ${filterTo}`;
// BETWEEN bounds are interpolated directly into the WHERE clause, so only
// accept finite numeric bounds (date ranges are handled separately above).
// Numeric strings from serialized filter state are coerced; anything that
// isn't a finite number is dropped rather than concatenated as raw SQL.
// Reject null/empty bounds explicitly: Number(null) and Number('') both
// coerce to 0, which would otherwise produce a misleading BETWEEN ... AND 0.
const isCoercibleBound = (bound: FilterValue): boolean =>
(typeof bound === 'number' || typeof bound === 'string') && bound !== '';
if (!isCoercibleBound(value) || !isCoercibleBound(filterTo)) {
return '';
}
const from = Number(value);
const to = Number(filterTo);
if (!Number.isFinite(from) || !Number.isFinite(to)) {
return '';
}
return `${columnName} ${SQL_OPERATORS.BETWEEN} ${from} AND ${to}`;
}
const formattedValue = formatValueForOperator(type, value!);

View File

@@ -771,6 +771,60 @@ describe('agGridFilterConverter', () => {
// Should reject column names longer than 255 characters
expect(result.simpleFilters).toHaveLength(0);
});
test('should drop inRange bounds that are not numeric', () => {
const filterModel: AgGridFilterModel = {
age: {
filterType: 'number',
operator: 'AND',
condition1: {
filterType: 'number',
type: 'inRange',
filter: '0 AND 1=1--',
filterTo: '100',
},
condition2: {
filterType: 'number',
type: 'greaterThan',
filter: 5,
},
} as AgGridCompoundFilter,
};
const result = convertAgGridFiltersToSQL(filterModel);
// The malicious range condition is dropped, so its payload never reaches
// the WHERE clause; the sibling numeric condition survives unchanged.
expect(result.complexWhere ?? '').not.toContain('1=1');
expect(result.complexWhere ?? '').not.toContain('BETWEEN');
expect(result.complexWhere).toBe('age > 5');
});
test('should keep numeric inRange bounds (including numeric strings)', () => {
const filterModel: AgGridFilterModel = {
age: {
filterType: 'number',
operator: 'AND',
condition1: {
filterType: 'number',
type: 'inRange',
filter: '18',
filterTo: 65,
},
condition2: {
filterType: 'number',
type: 'lessThan',
filter: 100,
},
} as AgGridCompoundFilter,
};
const result = convertAgGridFiltersToSQL(filterModel);
// Assert the full compound clause so the upper bound and the sibling
// condition are both validated, not just the BETWEEN fragment.
expect(result.complexWhere).toBe('(age BETWEEN 18 AND 65 AND age < 100)');
});
});
describe('Edge cases', () => {

View File

@@ -29,7 +29,7 @@
"acorn": "^8.16.0",
"d3-array": "^3.2.4",
"lodash": "^4.18.1",
"zod": "^4.4.1"
"zod": "^4.4.3"
},
"peerDependencies": {
"@apache-superset/core": "*",

View File

@@ -97,7 +97,7 @@ export function createWrapper(options?: Options) {
}
if (useDnd) {
// @ts-expect-error react-dnd types not updated for React 18
// @ts-ignore react-dnd's DndProviderProps omits `children` under React 18 types
result = <DndProvider backend={HTML5Backend}>{result}</DndProvider>;
}

View File

@@ -117,6 +117,7 @@ type LaunchQueue = {
const pendingTimerIds = new Set<ReturnType<typeof setTimeout>>();
const MAX_CONSUMER_POLL_ATTEMPTS = 50;
const consumerPromises: Promise<void>[] = [];
// Defer the consumer call to a macrotask so it doesn't fire synchronously inside
// the component's useEffect — calling it inline deadlocks Jest because the
@@ -131,7 +132,11 @@ const setupLaunchQueue = (fileHandle: MockFileHandle | null = null) => {
if (fileHandle) {
const id = setTimeout(() => {
pendingTimerIds.delete(id);
consumer({ files: [fileHandle] });
consumerPromises.push(
Promise.resolve(consumer({ files: [fileHandle] })).then(
() => undefined,
),
);
}, 0);
pendingTimerIds.add(id);
}
@@ -165,9 +170,19 @@ beforeEach(() => {
.launchQueue;
});
afterEach(() => {
afterEach(async () => {
pendingTimerIds.forEach(id => clearTimeout(id));
pendingTimerIds.clear();
if (consumerPromises.length > 0) {
const results = await Promise.allSettled(consumerPromises);
results.forEach(r => {
if (r.status === 'rejected') {
// eslint-disable-next-line no-console
console.warn('LaunchQueue consumer rejected:', r.reason);
}
});
consumerPromises.length = 0;
}
delete (window as unknown as Window & { launchQueue?: LaunchQueue })
.launchQueue;
});

View File

@@ -21,10 +21,11 @@ import logging
import re
from datetime import datetime
from re import Pattern
from typing import Any, Optional, TYPE_CHECKING
from typing import Any, Callable, Optional, TYPE_CHECKING
from flask_babel import gettext as __
from sqlalchemy.dialects.postgresql import DOUBLE_PRECISION, ENUM, JSON
from sqlalchemy import types
from sqlalchemy.dialects.postgresql import DOUBLE_PRECISION, ENUM, INTERVAL, JSON
from sqlalchemy.dialects.postgresql.base import PGInspector
from sqlalchemy.engine.reflection import Inspector
from sqlalchemy.engine.url import URL
@@ -135,6 +136,34 @@ def parse_options(connect_args: dict[str, Any]) -> dict[str, str]:
return {token[0]: token[1] for token in tokens}
def _normalize_interval(v: Any) -> Optional[float]:
"""Convert PostgreSQL INTERVAL values to milliseconds.
psycopg2 and psycopg3 always return INTERVAL values as datetime.timedelta
objects. We convert to milliseconds so users can apply the built-in
"DURATION" number format for human-readable display (e.g.,
"1d 2h 30m 45s") and so the values participate cleanly in numeric
aggregations in bar/pie charts.
Returns None for the NULL case (preserves NULL semantics) and for any
unexpected non-timedelta type (avoids producing a mixed-type column
when an unfamiliar driver surfaces something other than timedelta).
"""
if v is None:
return None
if hasattr(v, "total_seconds"):
return v.total_seconds() * 1000
# Defensive: psycopg2/3 should always hand us a timedelta. If a future
# driver doesn't, surface the surprise in the logs rather than silently
# dropping the value so operators can diagnose it.
logger.warning(
"Cannot normalize PostgreSQL INTERVAL value of type %s to numeric; "
"returning None.",
type(v).__name__,
)
return None
class PostgresBaseEngineSpec(BaseEngineSpec):
"""Abstract class for Postgres 'like' databases"""
@@ -526,8 +555,17 @@ class PostgresEngineSpec(BasicParametersMixin, PostgresBaseEngineSpec):
ENUM(),
GenericDataType.STRING,
),
(
re.compile(r"^interval", re.IGNORECASE),
INTERVAL(),
GenericDataType.NUMERIC,
),
)
column_type_mutators: dict[types.TypeEngine, Callable[[Any], Any]] = {
INTERVAL: _normalize_interval,
}
@classmethod
def get_schema_from_engine_params(
cls,

View File

@@ -53,7 +53,11 @@ from superset_core.queries.models import (
)
from superset import security_manager
from superset.exceptions import SupersetParseError, SupersetSecurityException
from superset.exceptions import (
SupersetException,
SupersetParseError,
SupersetSecurityException,
)
from superset.explorables.base import TimeGrainDict
from superset.jinja_context import BaseTemplateProcessor, get_template_processor
from superset.models.helpers import (
@@ -99,6 +103,14 @@ class SqlTablesMixin: # pylint: disable=too-few-public-methods
)
except (SupersetSecurityException, SupersetParseError, TemplateError):
return []
except SupersetException as ex:
# Jinja macros such as ``{{ dataset(id) }}`` or ``{{ metric(...) }}``
# may reference resources that no longer exist (e.g. a deleted
# dataset). Surfacing the failure here would break list endpoints
# that include ``sql_tables`` in their payload, hiding every saved
# query from the user. Treat it as a parse failure instead.
logger.warning("Unable to extract tables from SQL via Jinja: %s", ex)
return []
class Query(

View File

@@ -15,14 +15,14 @@
# specific language governing permissions and limitations
# under the License.
from datetime import datetime
from datetime import datetime, timedelta
from typing import Any, Optional
from unittest.mock import MagicMock
import pytest
from pytest_mock import MockerFixture
from sqlalchemy import column, types
from sqlalchemy.dialects.postgresql import DOUBLE_PRECISION, ENUM, JSON
from sqlalchemy.dialects.postgresql import DOUBLE_PRECISION, ENUM, INTERVAL, JSON
from sqlalchemy.engine.interfaces import Dialect
from sqlalchemy.engine.url import make_url
@@ -87,6 +87,8 @@ def test_convert_dttm(
("TIME", types.Time, None, GenericDataType.TEMPORAL, True),
# Boolean
("BOOLEAN", types.Boolean, None, GenericDataType.BOOLEAN, False),
# Interval (mapped to NUMERIC for chart rendering)
("INTERVAL", INTERVAL, None, GenericDataType.NUMERIC, False),
],
)
def test_get_column_spec(
@@ -366,3 +368,38 @@ class TestRedshiftDetection:
spec.update_params_from_encrypted_extra(database, params)
assert "pool_events" not in params
def test_interval_type_mutator() -> None:
"""
DB Eng Specs (postgres): Test INTERVAL type mutator
INTERVAL values are converted to milliseconds so users can apply
the built-in "DURATION" number format for human-readable display.
"""
mutator = spec.column_type_mutators[INTERVAL]
# Timedelta conversion — the only path psycopg2/psycopg3 actually
# exercises. Result is in milliseconds for compatibility with the
# DURATION formatter.
td = timedelta(days=1, hours=2, minutes=30, seconds=45)
assert mutator(td) == 95445000.0 # (1*86400 + 2*3600 + 30*60 + 45) * 1000
# Zero duration
assert mutator(timedelta(0)) == 0.0
# Negative interval
assert mutator(timedelta(days=-1)) == -86400000.0
# None preserves NULL semantics (not converted to 0)
assert mutator(None) is None
# Unexpected non-timedelta types fall through to the defensive
# `return None` (and emit a warning) rather than producing a
# mixed-type column.
assert mutator("1 day 02:30:45") is None
assert mutator("P1DT2H30M45S") is None
assert mutator(12345) is None
assert mutator(True) is None
assert mutator([1, 2, 3]) is None
assert mutator({"days": 1}) is None

View File

@@ -21,8 +21,14 @@ from flask_appbuilder import Model
from jinja2.exceptions import TemplateError
from pytest_mock import MockerFixture
from superset.commands.dataset.exceptions import DatasetNotFoundError
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import SupersetParseError, SupersetSecurityException
from superset.exceptions import (
SupersetParseError,
SupersetSecurityException,
SupersetTemplateException,
)
from superset.models import sql_lab as sql_lab_module
from superset.models.sql_lab import Query, SavedQuery
@@ -34,34 +40,61 @@ from superset.models.sql_lab import Query, SavedQuery
],
)
@pytest.mark.parametrize(
"exception",
("exception", "should_warn"),
[
SupersetSecurityException(
SupersetError(
error_type=SupersetErrorType.QUERY_SECURITY_ACCESS_ERROR,
message="",
level=ErrorLevel.ERROR,
)
# Original silent handler — security/parse/template errors are
# expected during list rendering and produce no log noise.
(
SupersetSecurityException(
SupersetError(
error_type=SupersetErrorType.QUERY_SECURITY_ACCESS_ERROR,
message="",
level=ErrorLevel.ERROR,
)
),
False,
),
SupersetParseError(
sql="INVALID SQL",
message="Invalid SQL syntax",
(
SupersetParseError(
sql="INVALID SQL",
message="Invalid SQL syntax",
),
False,
),
TemplateError,
(TemplateError, False),
# ``{{ dataset(id) }}`` referencing a deleted dataset previously
# bubbled up through ``sql_tables`` and broke saved-query list
# endpoints (see issue #32771). The new handler swallows it but
# logs a warning so the underlying breakage is still observable —
# pinned here so a future refactor that collapses the case into
# the silent handler fails this test.
(DatasetNotFoundError("Dataset 1 not found!"), True),
(SupersetTemplateException("Template rendering failed"), True),
],
)
def test_sql_tables_mixin_sql_tables_exception(
klass: type[Model],
exception: Exception,
should_warn: bool,
mocker: MockerFixture,
) -> None:
mocker.patch(
"superset.models.sql_lab.process_jinja_sql",
side_effect=exception,
)
warning_spy = mocker.spy(sql_lab_module.logger, "warning")
assert klass(sql="SELECT 1", database=MagicMock()).sql_tables == []
if should_warn:
assert warning_spy.call_count == 1, (
f"{type(exception).__name__} should hit the warning-logging "
"handler; if this fails, the case was likely collapsed into "
"the silent first-handler clause."
)
else:
warning_spy.assert_not_called()
@pytest.mark.parametrize(
"klass",