sadpandajoe commented on code in PR #43870:
URL: https://github.com/apache/superset/pull/43870#discussion_r4251048976
##########
superset/daos/dataset.py:
##########
@@ -476,10 +606,104 @@ def _validate_column_date_formats(
"python_date_format is an invalid date/timestamp format."
)
+ @staticmethod
+ def clear_unmapped_partition_transforms(model: SqlaTable) -> None:
+ """
+ Drop the value transform from every column the mapping does not mirror.
+
+ A mapping has exactly one mirrored column, so at most one column may
+ carry a transform. A transform parked on any other column is invisible
+ -- no row but the mapped one renders one -- yet it is still stored, and
+ it goes live the moment the mapped column resolves back to it. Clearing
+ an override is enough to do that: a null `partition_mapped_column`
means
+ "follow `main_dttm_col`", so dropping a mapping would otherwise
activate
+ whatever the default datetime column happened to be holding, turning a
+ request to remove a mapping into a request to add a different one.
+
+ Enforced here rather than in the editor alone because the editor is
only
+ one writer: a PUT, an `override_columns=true` metadata sync and an
+ import all reach the columns directly. The same argument
+ `clear_dangling_partition_mapping` makes about dangling columns.
+
+ `DatasetDAO.update` calls this twice, before and after it applies the
+ request, and the reason is here rather than only at the call site: this
+ reads the mapping as it stands, so a single call can only enforce the
+ invariant against one of the two resolutions a mapping change has.
+
+ Gated on the feature flag, unlike its sibling, because this discards
+ stored configuration rather than repairing a broken reference. With the
+ flag off nothing mirrors, so there is no armed mapping to disarm and
+ clearing would be pure loss.
+ """
+ if not
feature_flag_manager.is_feature_enabled(PARTITION_FILTER_MAPPING_FLAG):
+ return
+
+ mapped_column = (
+ (model.partition_mapped_column or model.main_dttm_col)
+ if model.partition_column
+ else None
+ )
+ # `_upsert_columns` and `_override_columns` insert a new column with
+ # `db.session.add(TableColumn(..., table_id=model.id))` rather than
+ # appending to this relationship, and `BaseDAO.update` does not flush
--
+ # so a column created *and* given a transform in the same request was
+ # invisible here and kept it. That is exactly the stray transform this
+ # method exists to clear, waiting for the mapping to resolve back to
it.
+ db.session.flush()
+ db.session.expire(model, ["columns"])
+ for column in model.columns:
+ if column.column_name == mapped_column:
+ continue
+ if (
+ column.partition_value_transform
+ or column.partition_transform_is_monotonic
+ ):
+ column.partition_value_transform = None
+ column.partition_transform_is_monotonic = False
+
+ @staticmethod
+ def clear_dangling_partition_mapping(
Review Comment:
The dangling-reference repair only runs from `DatasetDAO.update`.
`DeleteDatasetColumnCommand` (`DELETE /api/v1/dataset/<pk>/column/<column_id>`)
removes a column through `DatasetColumnDAO.delete` without calling it. With the
flag on, deleting the designated partition column that way leaves
`partition_column` pointing at a column that no longer exists, and the next
description-only PUT (no `columns` payload) validates against the model's
remaining columns and is rejected. Should the column-delete path clear the
mapping as well?
##########
superset/dashboards/schemas.py:
##########
@@ -379,6 +379,9 @@ class DashboardDatasetSchema(Schema):
granularity_sqla = fields.List(fields.List(fields.Str()))
normalize_columns = fields.Bool()
always_filter_main_dttm = fields.Bool()
+ partition_column = fields.Str()
Review Comment:
`partition_column` and `partition_mapped_column` are declared as
non-nullable strings, but `SqlaTable.data` always includes them and both are
`None` on any dataset without a mapping (and `partition_mapped_column` is
`None` whenever the mapping follows `main_dttm_col`). The generated
`DashboardDatasetSchema` in `openapi.json` then rejects an ordinary successful
response under strict client validation. Could both fields be `allow_none=True`
with the spec regenerated?
##########
superset/dashboards/schemas.py:
##########
@@ -379,6 +379,9 @@ class DashboardDatasetSchema(Schema):
granularity_sqla = fields.List(fields.List(fields.Str()))
normalize_columns = fields.Bool()
always_filter_main_dttm = fields.Bool()
+ partition_column = fields.Str()
+ partition_mapped_column = fields.Str()
+ partition_filter_mapping = fields.Dict(allow_none=True)
Review Comment:
`DASHBOARD_DATASET_INACCESSIBLE_FIELDS` strips columns, SQL, `main_dttm_col`
and similar from a dataset the caller cannot access, but
`partition_filter_mapping` is not in that list. `GET
/api/v1/dashboard/<id>/datasets` therefore still returns the mapped and
partition column names and the raw transform (`evaluated_transform`) to a Gamma
user with dashboard access but no grant on that dataset. Should the three new
fields be added to the redaction list?
##########
tests/unit_tests/connectors/sqla/partition_mapping_test.py:
##########
@@ -0,0 +1,2674 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Unit tests for ``superset.connectors.sqla.partition_mapping``."""
+
+from __future__ import annotations
+
+from datetime import datetime
+from decimal import Decimal
+from importlib import import_module
+from typing import Any, cast
+from unittest.mock import MagicMock, patch, PropertyMock
+
+import pandas as pd
+import pytest
+from dateutil.relativedelta import relativedelta
+from flask import Flask
+from sqlalchemy.dialects import mysql, postgresql
+
+from superset.config import DISALLOWED_SQL_FUNCTIONS
+from superset.connectors.sqla.models import SqlaTable, TableColumn
+from superset.connectors.sqla.partition_mapping import (
+ _mirror_verdict_cache_key,
+ _placeholder_is_bindable,
+ _probe_cache_key,
+ _reads_as_temporal,
+ _render_literal,
+ build_mirrored_predicates,
+ build_probe_sql,
+ contains_executable_comment,
+ contains_jinja,
+ contains_value_placeholder,
+ drop_unmapped_value_transforms,
+ equality_mirrors_safely,
+ evaluate_transform,
+ find_non_deterministic_functions,
+ grain_bucket_width,
+ GRAIN_BUCKET_WIDTHS,
+ is_bare_expression,
+ is_parseable,
+ is_transform_active,
+ is_unfinished,
+ known_mirror_verdict,
+ MappingValidationIssue,
+ mirror_operator,
+ MIRRORABLE_ALWAYS,
+ MIRRORABLE_IF_MONOTONIC,
+ mirrorable_operators,
+ parse_error_detail,
+ placeholder_is_executable,
+ probe_sql_is_evaluable,
+ probed_value_type_error,
+ RawProbeValue,
+ resolve_partition_mapping,
+ stored_expression_error,
+ validate_partition_mapping,
+ validate_transform,
+)
+from superset.constants import TimeGrain
+from superset.db_engine_specs.base import BaseEngineSpec
+from superset.db_engine_specs.oracle import OracleEngineSpec
+from superset.db_engine_specs.postgres import PostgresEngineSpec
+from superset.models.core import Database
+from superset.sql.parse import SQLStatement
+from superset.utils.core import FilterOperator, GenericDataType, override_user
+
+
[email protected](autouse=True)
+def real_probe_cache(app: Flask) -> Any:
+ """
+ The test app runs a null cache, which would make every cache assertion here
+ vacuously pass. Swap in a real in-memory one for the duration.
+ """
+ from flask_caching import Cache
+
+ from superset.extensions import cache_manager
+
+ cache = Cache(config={"CACHE_TYPE": "SimpleCache",
"CACHE_DEFAULT_TIMEOUT": 300})
+ cache.init_app(app)
+ original = cache_manager._cache # noqa: SLF001
+ cache_manager._cache = cache # noqa: SLF001
+ yield
+ cache_manager._cache = original # noqa: SLF001
+
+
[email protected](autouse=True)
+def enable_partition_filter_mapping(app: Flask) -> Any:
+ """The feature ships off; every test here exercises it on."""
+ original =
app.config["DEFAULT_FEATURE_FLAGS"].get("PARTITION_FILTER_MAPPING")
+ app.config["DEFAULT_FEATURE_FLAGS"]["PARTITION_FILTER_MAPPING"] = True
+ yield
+ if original is None:
+ del app.config["DEFAULT_FEATURE_FLAGS"]["PARTITION_FILTER_MAPPING"]
+ else:
+ app.config["DEFAULT_FEATURE_FLAGS"]["PARTITION_FILTER_MAPPING"] =
original
+
+
+def _table(**kwargs: Any) -> SqlaTable:
+ database = Database(database_name="test_db", sqlalchemy_uri="sqlite://")
+ defaults: dict[str, Any] = {
+ "table_name": "web_events",
+ "database": database,
+ "main_dttm_col": "event_time",
+ "columns": [
+ TableColumn(column_name="event_time", is_dttm=True,
type="TIMESTAMP"),
+ TableColumn(
+ column_name="dt_epoch",
+ type="BIGINT",
+ partition_value_transform=None,
+ ),
+ TableColumn(column_name="country", type="VARCHAR"),
+ TableColumn(column_name="region_key", type="VARCHAR"),
+ ],
+ }
+ defaults.update(kwargs)
+ return SqlaTable(**defaults)
+
+
+def _mapped_table(
+ *,
+ partition_column: str = "dt_epoch",
+ mapped_column: str = "event_time",
+ transform: str | None = "unix_timestamp(:value)",
+ monotonic: bool = True,
+ partition_mapped_column: str | None = None,
+ main_dttm_col: str | None = "event_time",
+) -> SqlaTable:
+ table = _table(main_dttm_col=main_dttm_col)
+ table.partition_column = partition_column
+ table.partition_mapped_column = partition_mapped_column
+ for column in table.columns:
+ if column.column_name == mapped_column:
+ column.partition_value_transform = transform
+ column.partition_transform_is_monotonic = monotonic
+ return table
+
+
+# ---------------------------------------------------------------------------
+# §2 — operator safety matrix
+# ---------------------------------------------------------------------------
+
+
+def test_equality_and_in_are_always_mirrorable() -> None:
+ """``=`` and ``IN`` are safe for any function ``T``."""
+ assert MIRRORABLE_ALWAYS == {FilterOperator.EQUALS, FilterOperator.IN}
+
+
+def test_range_operators_require_a_monotonic_transform() -> None:
+ assert MIRRORABLE_IF_MONOTONIC == {
+ FilterOperator.GREATER_THAN,
+ FilterOperator.GREATER_THAN_OR_EQUALS,
+ FilterOperator.LESS_THAN,
+ FilterOperator.LESS_THAN_OR_EQUALS,
+ FilterOperator.TEMPORAL_RANGE,
+ }
+
+
+def test_mirrorable_operators_excludes_ranges_when_not_monotonic() -> None:
+ assert mirrorable_operators(is_monotonic=False) == MIRRORABLE_ALWAYS
+
+
+def test_mirrorable_operators_includes_ranges_when_monotonic() -> None:
+ assert mirrorable_operators(is_monotonic=True) == (
+ MIRRORABLE_ALWAYS | MIRRORABLE_IF_MONOTONIC
+ )
+
+
[email protected](
+ "operator",
+ [
+ FilterOperator.NOT_EQUALS,
+ FilterOperator.NOT_IN,
+ FilterOperator.LIKE,
+ FilterOperator.ILIKE,
+ FilterOperator.NOT_LIKE,
+ FilterOperator.NOT_ILIKE,
+ FilterOperator.IS_NULL,
+ FilterOperator.IS_NOT_NULL,
+ FilterOperator.IS_TRUE,
+ FilterOperator.IS_FALSE,
+ ],
+)
+def test_negations_and_pattern_matches_are_never_mirrorable(
+ operator: FilterOperator,
+) -> None:
+ """
+ ``T`` is not injective, so ``col != v`` does **not** imply ``T(col) !=
T(v)``:
+ mirroring it would drop rows the original filter keeps.
+ """
+ assert operator not in mirrorable_operators(is_monotonic=True)
+
+
+def test_equality_is_dropped_when_the_engine_does_not_compare_byte_exactly()
-> None:
+ """
+ "Safe for any function ``T``" reasons about value equality: ``T`` is a
+ function, so ``col = v`` gives ``T(col) = T(v)``. The engine reasons about
+ *SQL* equality, and the two part company under a case-insensitive
+ collation -- a stored ``'us'`` satisfies a filter for ``'US'`` while the
+ mirror ``hex('US')`` excludes the row, and the chart loses it silently.
+ """
+ assert mirrorable_operators(is_monotonic=False, equality_is_safe=False) ==
set()
+
+
+def test_ranges_survive_an_unsafe_equality() -> None:
+ """
+ Range mirroring already rests on the owner declaring ``T``
+ order-preserving with respect to the column's own order -- an assertion
+ about the very comparison semantics this gate checks for. Equality has no
+ such declaration behind it, which is why only equality is withdrawn.
+ """
+ assert (
+ mirrorable_operators(is_monotonic=True, equality_is_safe=False)
+ == MIRRORABLE_IF_MONOTONIC
+ )
+
+
[email protected](
+ "operator,expected",
+ [
+ (FilterOperator.GREATER_THAN, FilterOperator.GREATER_THAN_OR_EQUALS),
+ (FilterOperator.LESS_THAN, FilterOperator.LESS_THAN_OR_EQUALS),
+ (
+ FilterOperator.GREATER_THAN_OR_EQUALS,
+ FilterOperator.GREATER_THAN_OR_EQUALS,
+ ),
+ (FilterOperator.LESS_THAN_OR_EQUALS,
FilterOperator.LESS_THAN_OR_EQUALS),
+ (FilterOperator.EQUALS, FilterOperator.EQUALS),
+ (FilterOperator.IN, FilterOperator.IN),
+ ],
+)
+def test_a_strict_bound_mirrors_non_strictly(
+ operator: FilterOperator, expected: FilterOperator
+) -> None:
+ """
+ Order-preserving means non-decreasing, not injective, so ``col < v``
+ implies only ``T(col) <= T(v)``. A day-key transform maps a whole day onto
+ one value and a strict mirror drops the boundary bucket. The non-strict
+ cases are already safe and pass through, which is what makes applying this
+ twice harmless.
+ """
+ assert mirror_operator(operator) == expected
+
+
+def test_relaxing_a_bound_stays_inside_the_mirrorable_set() -> None:
+ """
+ The substitution is about strictness, not about which filters mirror. The
+ Explore indicator reads `mirrorable_operators`, so leaving that set alone
is
+ what keeps the glyph from promising pruning the SQL does not do -- and it
is
+ also what guarantees `handle_comparison_filter` is never handed an operator
+ it has no case for.
+ """
+ assert {
+ mirror_operator(operator) for operator in MIRRORABLE_IF_MONOTONIC
+ } <= MIRRORABLE_IF_MONOTONIC
+
+
+def _column(type_: str) -> TableColumn:
+ database = Database(database_name="t", sqlalchemy_uri="sqlite://")
+ table = SqlaTable(table_name="t", database=database)
+ return TableColumn(column_name="c", type=type_, table=table)
+
+
[email protected](
+ "type_, binary, expected",
+ [
+ ("VARCHAR", True, True),
+ ("VARCHAR", False, False),
+ # Numeric and temporal comparison is exact everywhere.
+ ("BIGINT", False, True),
+ ("TIMESTAMP", False, True),
+ ("DOUBLE", False, True),
+ ],
+)
+def test_equality_mirrors_safely_only_guards_string_columns(
+ app: Flask, type_: str, binary: bool, expected: bool
+) -> None:
+ class _Spec(BaseEngineSpec):
+ binary_string_comparison = binary
+
+ with app.app_context():
+ assert equality_mirrors_safely(_column(type_), _Spec) is expected
+
+
[email protected](
+ "type_, expected",
+ [
+ # The padded types: `'US'` and `'US '` are equal in the column, and
+ # `lower()` of the two is not.
+ ("CHAR(2)", False),
+ ("CHAR", False),
+ ("CHARACTER(2)", False),
+ ("NCHAR(2)", False),
+ # What PostgreSQL reflection reports for a `CHAR(n)` column.
+ ("BPCHAR", False),
+ ("bpchar(2)", False),
+ ("NATIONAL CHARACTER(2)", False),
+ # The variable-width types start with the same letters and must not be
+ # caught by it.
+ ("VARCHAR(2)", True),
+ ("NVARCHAR(2)", True),
+ ("CHARACTER VARYING(2)", True),
+ ("TEXT", True),
+ ],
+)
+def test_a_padded_character_column_declines_equality_on_a_byte_exact_engine(
+ app: Flask, type_: str, expected: bool
+) -> None:
+ """
+ `binary_string_comparison` speaks for the engine's default comparison,
+ which PostgreSQL declares -- and a `CHAR(2)` there still matches a filter
+ for `'US '` against a stored `'US'`, so the mirror `lower('US ')` excludes
+ the row. Padding is a property of the type, so it is asked per column.
+ """
+
+ class _Spec(BaseEngineSpec):
+ binary_string_comparison = True
+
+ with app.app_context():
+ assert equality_mirrors_safely(_column(type_), _Spec) is expected
+
+
+def test_redshift_does_not_opt_in_to_byte_exact_comparison() -> None:
+ """
+ Redshift ignores trailing blanks for `VARCHAR` as well as `CHAR`, so the
+ `True` it inherits from `PostgresBaseEngineSpec` would be wrong.
+ """
+ from superset.db_engine_specs.redshift import RedshiftEngineSpec
+
+ assert RedshiftEngineSpec.binary_string_comparison is False
+
+
+def test_equality_mirrors_safely_fails_closed_on_an_unresolvable_type(
+ app: Flask,
+) -> None:
+ """
+ The gate exists to stop a silent wrong answer, so a column whose type it
+ cannot read is treated as the risky case rather than waved through.
+ """
+ column = MagicMock()
+ type(column).type_generic = PropertyMock(side_effect=ValueError("no type"))
+
+ with app.app_context():
+ assert equality_mirrors_safely(column, BaseEngineSpec) is False
+
+
+def test_no_mapped_column_leaves_the_matrix_alone(app: Flask) -> None:
+ """Nothing to reason about, and the resolver bails out on it anyway."""
+ with app.app_context():
+ assert equality_mirrors_safely(None, BaseEngineSpec) is True
+
+
[email protected](
+ "module, spec_name",
+ [
+ ("presto", "PrestoEngineSpec"),
+ ("hive", "HiveEngineSpec"),
+ ("trino", "TrinoEngineSpec"),
+ ("impala", "ImpalaEngineSpec"),
+ ("postgres", "PostgresEngineSpec"),
+ ("bigquery", "BigQueryEngineSpec"),
+ ("sqlite", "SqliteEngineSpec"),
+ ],
+)
+def test_the_byte_exact_engines_opt_in(module: str, spec_name: str) -> None:
+ """
+ The engines this feature targets compare text byte-exactly, so the gate
+ must not cost them the second canonical mapping (`lower(:value)` onto a
+ lowercased key).
+ """
+ spec = getattr(import_module(f"superset.db_engine_specs.{module}"),
spec_name)
+ assert spec.binary_string_comparison is True
+
+
[email protected]("module, spec_name", [("mysql", "MySQLEngineSpec")])
+def test_a_case_insensitive_engine_does_not_opt_in(module: str, spec_name:
str) -> None:
+ """
+ MySQL's default collation is `utf8mb4_0900_ai_ci`, which is exactly the
+ case the reviewer raised.
+ """
+ spec = getattr(import_module(f"superset.db_engine_specs.{module}"),
spec_name)
+ assert spec.binary_string_comparison is False
+
+
+def test_the_base_spec_does_not_opt_in() -> None:
+ """A spec that has not said so does not mirror string equality."""
+ assert BaseEngineSpec.binary_string_comparison is False
+
+
+# ---------------------------------------------------------------------------
+# Time grain bucket widths
+# ---------------------------------------------------------------------------
+
+
[email protected]("grain", list(TimeGrain))
+def test_every_builtin_grain_has_a_known_bucket_width(
+ app: Flask, grain: TimeGrain
+) -> None:
+ """
+ The widening is only sound for a grain whose bucket width we know, so a
+ grain added upstream without a width here would silently stop mirroring.
+ This is the test that notices.
+ """
+ with app.app_context():
+ assert grain_bucket_width(grain.value, "sqlite") is not None
+
+
+def test_a_calendar_grain_widens_by_a_calendar_month(app: Flask) -> None:
+ """A month is not thirty days, and the month it lands in decides its
width."""
+ with app.app_context():
+ width = grain_bucket_width(TimeGrain.MONTH.value, "sqlite")
+
+ assert width is not None
+ assert datetime(2026, 1, 31) + width == datetime(2026, 2, 28)
+ assert datetime(2026, 2, 1) + width == datetime(2026, 3, 1)
+
+
+def test_a_fractional_grain_has_a_width(app: Flask) -> None:
+ """
+ ``PT0.5H`` and ``P0.25Y`` are why the widths are written out rather than
+ parsed -- and the anchored week grains are intervals ``parse_duration``
+ rejects outright.
+ """
+ with app.app_context():
+ assert grain_bucket_width(TimeGrain.HALF_HOUR.value, "sqlite") ==
relativedelta(
+ minutes=30
+ )
+ assert grain_bucket_width(
+ TimeGrain.WEEK_ENDING_SATURDAY.value, "sqlite"
+ ) == relativedelta(days=7)
+
+
+def test_an_unknown_grain_has_no_known_width(app: Flask) -> None:
+ with app.app_context():
+ assert grain_bucket_width("P13X", "sqlite") is None
+ assert grain_bucket_width(None, "sqlite") is None
+
+
+def test_an_addon_grain_has_no_known_width(app: Flask) -> None:
+ """An operator-declared grain carries whatever duration string they
typed."""
+ with app.app_context():
+ app.config["TIME_GRAIN_ADDONS"] = {"PT2S": "2 second"}
+ try:
+ assert grain_bucket_width("PT2S", "sqlite") is None
+ finally:
+ app.config["TIME_GRAIN_ADDONS"] = {}
+
+
+def test_a_builtin_grain_an_operator_redefined_has_no_known_width(
+ app: Flask,
+) -> None:
+ """
+ ``TIME_GRAIN_ADDON_EXPRESSIONS`` replaces a built-in grain's SQL for one
+ engine, so its duration no longer bounds the displacement *there* -- but it
+ still does on every other engine.
+ """
+ with app.app_context():
+ app.config["TIME_GRAIN_ADDON_EXPRESSIONS"] = {
+ "sqlite": {"P1D": "DATETIME({col}, 'start of year')"}
+ }
+ try:
+ assert grain_bucket_width(TimeGrain.DAY.value, "sqlite") is None
+ assert grain_bucket_width(TimeGrain.DAY.value, "hive") is not None
+ finally:
+ app.config["TIME_GRAIN_ADDON_EXPRESSIONS"] = {}
+
+
+def test_the_width_table_is_keyed_on_grain_durations(app: Flask) -> None:
+ """Keys are the ISO duration a filter carries, not the enum member name."""
+ assert GRAIN_BUCKET_WIDTHS[TimeGrain.DAY] == relativedelta(days=1)
+ assert set(GRAIN_BUCKET_WIDTHS) == {grain.value for grain in TimeGrain}
+
+
+# ---------------------------------------------------------------------------
+# §4.1 — resolving the effective mapping, and the defensive bail-outs
+# ---------------------------------------------------------------------------
+
+
+def test_resolve_returns_the_mapping_when_everything_lines_up(app: Flask) ->
None:
+ with app.app_context():
+ mapping = resolve_partition_mapping(_mapped_table())
+
+ assert mapping is not None
+ assert mapping.partition_column == "dt_epoch"
+ assert mapping.mapped_column == "event_time"
+ assert mapping.value_transform == "unix_timestamp(:value)"
+ assert mapping.is_monotonic is True
+
+
+def test_effective_mapped_column_follows_main_dttm_col(app: Flask) -> None:
+ """No explicit override: the mapping follows the default datetime
column."""
+ with app.app_context():
+ mapping = resolve_partition_mapping(_mapped_table())
+
+ assert mapping is not None
+ assert mapping.mapped_column == "event_time"
+
+
+def test_explicit_override_wins_over_main_dttm_col(app: Flask) -> None:
+ table = _mapped_table(
+ mapped_column="country",
+ transform="lower(:value)",
+ monotonic=False,
+ partition_mapped_column="country",
+ )
+ table.partition_column = "region_key"
+
+ with app.app_context():
+ mapping = resolve_partition_mapping(table)
+
+ assert mapping is not None
+ assert mapping.mapped_column == "country"
+ assert mapping.partition_column == "region_key"
+ assert mapping.is_monotonic is False
+
+
+def test_resolve_returns_none_when_the_feature_flag_is_off(app: Flask) -> None:
+ table = _mapped_table()
+ with app.app_context():
+ with patch(
+ "superset.connectors.sqla.partition_mapping.feature_flag_manager."
+ "is_feature_enabled",
+ return_value=False,
+ ):
+ assert resolve_partition_mapping(table) is None
+
+
+def test_resolve_returns_none_without_a_partition_column(app: Flask) -> None:
+ table = _mapped_table()
+ table.partition_column = None
+ with app.app_context():
+ assert resolve_partition_mapping(table) is None
+
+
+def test_resolve_returns_none_when_partition_column_no_longer_exists(
+ app: Flask,
+) -> None:
+ """A column sync can drop the physical partition column out from under
us."""
+ table = _mapped_table()
+ table.partition_column = "dt_epoch_gone"
+ with app.app_context():
+ assert resolve_partition_mapping(table) is None
+
+
+def test_resolve_returns_none_when_the_mapped_column_no_longer_exists(
+ app: Flask,
+) -> None:
+ table = _mapped_table(main_dttm_col="vanished")
+ with app.app_context():
+ assert resolve_partition_mapping(table) is None
+
+
+def test_resolve_returns_none_on_self_mapping(app: Flask) -> None:
+ """
+ ``partition_column == effective mapped column`` mirrors a column onto
itself.
+ Save-time validation rejects it, but rows predating that validation exist.
+ """
+ table = _mapped_table(
+ partition_column="event_time",
+ mapped_column="event_time",
+ )
+ with app.app_context():
+ assert resolve_partition_mapping(table) is None
+
+
[email protected]("transform", [None, "", " "])
+def test_resolve_returns_none_without_a_transform(
+ app: Flask, transform: str | None
+) -> None:
+ table = _mapped_table(transform=transform)
+ with app.app_context():
+ assert resolve_partition_mapping(table) is None
+
+
+def test_resolve_returns_none_when_the_transform_lacks_the_placeholder(
+ app: Flask,
+) -> None:
+ table = _mapped_table(transform="unix_timestamp(event_time)")
+ with app.app_context():
+ assert resolve_partition_mapping(table) is None
+
+
+def test_resolve_returns_none_for_a_non_deterministic_transform(app: Flask) ->
None:
+ """
+ The query-time gate is `is_transform_active`, the same one the Explore
+ indicator reads. Anything narrower would let a transform that reached
+ storage without passing `UpdateDatasetCommand` -- through import, or a
+ hand-written bundle -- mirror a filter with a value frozen at probe time,
+ while the editor reported the mapping as inactive and nothing on screen
+ said otherwise.
+ """
+ table = _mapped_table(transform="unix_timestamp(:value) + rand()")
+ with app.app_context():
+ assert resolve_partition_mapping(table) is None
+
+
+def test_resolve_returns_none_for_a_multi_expression_transform(app: Flask) ->
None:
+ """A select list is not an expression; the probe cannot align its
columns."""
+ table = _mapped_table(transform="lower(:value), 'x'")
+ with app.app_context():
+ assert resolve_partition_mapping(table) is None
+
+
+def test_resolve_returns_none_for_an_unparseable_transform(app: Flask) -> None:
+ table = _mapped_table(transform="unix_timestamp(:value")
+ with app.app_context():
+ assert resolve_partition_mapping(table) is None
+
+
+def test_resolve_returns_none_when_the_mapped_column_has_an_advanced_data_type(
+ app: Flask,
+) -> None:
+ """
+ ``translate_filter`` builds its own predicate shape from *translated*
values,
+ so the ``(operator, value)`` pair the operator matrix reasons about does
not
+ exist. Mirroring anyway would silently apply the wrong values (§4.1).
+ """
+ table = _mapped_table()
+ for column in table.columns:
+ if column.column_name == "event_time":
+ column.advanced_data_type = "port"
+
+ with app.app_context():
+ with patch.dict(app.config["ADVANCED_DATA_TYPES"], {"port":
MagicMock()}):
+ with patch(
+
"superset.connectors.sqla.partition_mapping.feature_flag_manager."
+ "is_feature_enabled",
+ side_effect=lambda flag: (
+ flag in {"PARTITION_FILTER_MAPPING",
"ENABLE_ADVANCED_DATA_TYPES"}
+ ),
+ ):
+ assert resolve_partition_mapping(table) is None
+
+
+def test_advanced_data_type_does_not_block_when_the_flag_is_off(app: Flask) ->
None:
+ """An inert ``advanced_data_type`` is not a reason to skip mirroring."""
+ table = _mapped_table()
+ for column in table.columns:
+ if column.column_name == "event_time":
+ column.advanced_data_type = "port"
+
+ with app.app_context():
+ with patch(
+ "superset.connectors.sqla.partition_mapping.feature_flag_manager."
+ "is_feature_enabled",
+ side_effect=lambda flag: flag == "PARTITION_FILTER_MAPPING",
+ ):
+ assert resolve_partition_mapping(table) is not None
+
+
+# ---------------------------------------------------------------------------
+# §5 — transform inspection helpers
+# ---------------------------------------------------------------------------
+
+
[email protected](
+ "transform,expected",
+ [
+ ("unix_timestamp(:value)", True),
+ ("lower(:value)", True),
+ ("CAST(:value AS BIGINT)", True),
+ ("unix_timestamp(event_time)", False),
+ (":values", False),
+ ("", False),
+ (None, False),
+ ],
+)
+def test_contains_value_placeholder(transform: str | None, expected: bool) ->
None:
+ assert contains_value_placeholder(transform) is expected
+
+
[email protected](
+ "transform,expected",
+ [
+ ("unix_timestamp(:value)", False),
+ ("{{ current_username() }}", True),
+ ("lower({% if x %}:value{% endif %})", True),
+ ("lower(:value) -- {# comment #}", True),
+ ],
+)
+def test_contains_jinja(transform: str, expected: bool) -> None:
+ assert contains_jinja(transform) is expected
+
+
[email protected](
+ ("transform", "expected"),
+ [
+ ("lower(:value)", False),
+ ("lower(:value) /* an ordinary comment */", False),
+ ("lower(:value) -- a line comment", False),
+ (":value /*!50000 + 1 */", True),
+ (":value /*M!50000 + 1 */", True),
+ (":value /*! + 1 */", True),
+ ("", False),
+ (None, False),
+ ],
+)
+def test_contains_executable_comment(transform: str | None, expected: bool) ->
None:
+ assert contains_executable_comment(transform) is expected
+
+
[email protected](
+ "transform",
+ [
+ "unix_timestamp(:value)",
+ "lower(:value)",
+ "CAST(:value AS BIGINT)",
+ "date_format(:value, 'yyyyMMdd')",
+ ],
+)
+def test_pure_transforms_report_no_non_deterministic_functions(
+ transform: str,
+) -> None:
+ assert find_non_deterministic_functions(transform, "hive") == set()
+
+
[email protected](
+ "transform,expected_name",
+ [
+ ("date_diff(:value, now())", "NOW"),
+ ("CAST(:value AS BIGINT) + rand()", "RAND"),
+ ("CAST(:value AS DATE) - current_date", "CURRENT_DATE"),
+ ],
+)
+def test_non_deterministic_functions_are_reported(
+ transform: str, expected_name: str
+) -> None:
+ """
+ The probe runs at a different moment and in a different session from the
+ chart query, and its result is cached, so anything time- or
+ randomness-dependent freezes a snapshot of probe time into the predicate.
+ """
+ assert expected_name in find_non_deterministic_functions(transform, "hive")
+
+
+def test_niladic_unix_timestamp_is_rejected_but_the_unary_form_is_not() ->
None:
+ """
+ On Hive/Impala ``unix_timestamp()`` means "now" while ``unix_timestamp(x)``
+ -- the canonical temporal transform -- is pure. The distinction is the
whole
+ reason this check inspects arity rather than just the name.
+ """
+ assert find_non_deterministic_functions("unix_timestamp(:value)", "hive")
== set()
+ assert find_non_deterministic_functions(
+ "unix_timestamp(:value) - unix_timestamp()", "hive"
+ )
+
+
+# ---------------------------------------------------------------------------
+# §4.2 — probe evaluation
+# ---------------------------------------------------------------------------
+
+
+def _user(username: str) -> Any:
+ """A stand-in for `override_user`, which only needs `.username` here."""
+ user = MagicMock()
+ user.username = username
+ user.is_anonymous = False
+ return user
+
+
+def _database_returning(values: list[Any]) -> Database:
+ """A real ``Database`` (so the dialect is real) with only ``get_df``
stubbed."""
+ database = Database(database_name="probe_db", sqlalchemy_uri="sqlite://")
+ database.get_df = MagicMock( # type: ignore[method-assign]
+ return_value=pd.DataFrame(
+ [values], columns=[f"v{i}" for i in range(len(values))]
+ )
+ )
+ return database
+
+
+def _database_returning_frame(frame: pd.DataFrame) -> Database:
+ """
+ ``_database_returning`` for a probe result whose column dtypes matter.
+
+ The flat-list sibling builds its frame from one Python list, so pandas
+ infers a single dtype and the row-versus-cell distinction cannot be seen.
+ """
+ database = Database(database_name="probe_db", sqlalchemy_uri="sqlite://")
+ database.get_df = MagicMock( # type: ignore[method-assign]
+ return_value=frame
+ )
+ return database
+
+
+def _probe(database: Database) -> MagicMock:
+ """
+ The stubbed ``get_df``, typed for call assertions.
+
+ ``Database.get_df`` is a real method, so mypy reads its declared signature
+ rather than the mock that replaced it and rejects ``.call_args`` and
+ friends.
+ """
+ return cast(MagicMock, database.get_df)
+
+
+def test_evaluate_transform_returns_one_value_per_input(app: Flask) -> None:
+ database = _database_returning([1767225600, 1769904000])
+
+ with app.app_context():
+ result = evaluate_transform(
+ database,
+ None,
+ "default",
+ "unix_timestamp(:value)",
+ ["2026-01-01 00:00:00", "2026-02-01 00:00:00"],
+ )
+
+ assert result == [1767225600, 1769904000]
+ _probe(database).assert_called_once()
+
+
+def test_evaluate_transform_binds_values_rather_than_interpolating(
+ app: Flask,
+) -> None:
+ """
+ Filter values are attacker-controlled (a Gamma user picks them), so they
must
+ be bound and escaped, never pasted into the probe SQL.
+ """
+ database = _database_returning(["o''brien"])
+
+ with app.app_context():
+ evaluate_transform(database, None, None, "lower(:value)", ["O'Brien"])
+
+ sql = _probe(database).call_args.kwargs["sql"]
+ assert "O'Brien" not in sql
+ assert "O''Brien" in sql
+
+
[email protected](
+ ("transform", "expected"),
+ [
+ ("lower(:value)", True),
+ ("CAST(:value AS BIGINT)", True),
+ ("date_format(:value, '%Y%m%d')", True),
+ # The cast shorthand: a trailing colon stops SQLAlchemy's bind scan.
+ (":value::bigint", False),
+ ("(:value::date)::text", False),
+ ],
+)
+def test_a_postgres_cast_shorthand_is_not_bindable(
+ transform: str, expected: bool
+) -> None:
+ """
+ `VALUE_PLACEHOLDER_RE` is `:value\b`, which matches inside
`:value::bigint`.
+ SQLAlchemy's own scan ends with `(?![:\\w$])`, so the following colon stops
+ it: `text(":value::bigint")` reports a parameter named `valu`, and asking
it
+ to bind `value` raises.
+
+ Pinned because the two regexes disagreeing is the whole bug: every
+ write-side gate accepted the transform and the probe then died on an
+ `ArgumentError` that `_probe` swallows.
+ """
+ assert _placeholder_is_bindable(transform) is expected
+
+
+def test_a_cast_shorthand_transform_still_probes(app: Flask) -> None:
+ """
+ The transform is valid SQL, passes every gate, and used to prune nothing:
+ `build_probe_sql` raised, `_probe` swallowed it, and the only symptom was a
+ mapping that silently never mirrored -- with the preview reporting an
engine
+ failure for SQL that was never sent.
+ """
+ database = _database_returning([20260115])
+
+ with app.app_context():
+ result = evaluate_transform(
+ database, None, None, ":value::bigint", ["20260115"]
+ )
+
+ assert result == [20260115]
+ assert _probe(database).call_count == 1
+ assert "::bigint" in _probe(database).call_args.kwargs["sql"]
+
+
+def test_a_cast_shorthand_value_is_still_escaped_by_the_dialect(
+ app: Flask,
+) -> None:
+ """
+ The fallback substitutes where the bind path binds, so it has to earn the
+ same guarantee its sibling above pins: the value is rendered by the
+ dialect's literal processor, not pasted in.
+
+ Binding was only ever the vehicle for reaching that processor --
+ `_compile_literal` inlines the result either way -- so this renders first
+ and substitutes second. Same string, different order.
+ """
+ database = _database_returning(["x"])
+
+ with app.app_context():
+ evaluate_transform(database, None, None, ":value::text", ["O'Brien"])
+
+ sql = _probe(database).call_args.kwargs["sql"]
+ assert "O'Brien" not in sql
+ assert "O''Brien" in sql
+
+
+def test_evaluate_transform_dedupes_repeated_values(app: Flask) -> None:
+ """Three inputs, two distinct: the probe evaluates the transform twice."""
+ database = _database_returning(["us", "us"])
+
+ with app.app_context():
+ result = evaluate_transform(
+ database, None, None, "lower(:value)", ["US", "US", "us"]
+ )
+
+ assert result == ["us", "us", "us"]
+ sql = _probe(database).call_args.kwargs["sql"]
+ assert sql.count("lower") == 2
+
+
+def test_evaluate_transform_fails_open_on_a_short_result_row(app: Flask) ->
None:
+ """A row narrower than the probe asked for means the results cannot be
+ aligned back to their inputs; pruning is skipped rather than guessed at."""
+ database = _database_returning(["us"])
+
+ with app.app_context():
+ assert (
+ evaluate_transform(database, None, None, "lower(:value)", ["US",
"CA"])
+ is None
+ )
+
+
+def test_evaluate_transform_fails_open_on_a_wide_result_row(app: Flask) ->
None:
+ """
+ Too many columns is the dangerous direction. A transform whose select list
+ holds two expressions returns 2N columns for N inputs, and reading the
first
+ N interleaves the expressions instead of taking one per value -- a
predicate
+ built from the wrong values rather than one that is merely short. A frame
of
+ `('us', 'x', 'ca', 'x')` read as the first two columns would have yielded
+ `('us', 'x')` and dropped every CA row.
+
+ The transform is a perfectly ordinary `lower(:value)` and the width comes
+ from the stubbed frame, which is the only way to reach the guard: a
+ transform that really does carry two expressions -- `lower(:value), 'x'` --
+ is refused by `stored_expression_error` for not being a single expression,
+ so `get_df` is never called and `None` comes back for an unrelated reason.
+ Asserting the probe *ran* is what separates the two, and it is what makes
+ relaxing the guard from `!=` to `<` fail here rather than pass: four
columns
+ for two inputs is not fewer than two.
+ """
+ database = _database_returning(["us", "x", "ca", "x"])
+
+ with app.app_context():
+ assert (
+ evaluate_transform(database, None, None, "lower(:value)", ["US",
"CA"])
+ is None
+ )
+
+ assert _probe(database).call_count == 1
+
+
+def test_the_probe_select_carries_the_engine_s_from_clause(app: Flask) -> None:
+ """
+ A `SELECT` with no `FROM` is not universal SQL: Oracle and Db2 need a
+ one-row table to select from. Without this every probe on those engines
+ raises, which `evaluate_transform` swallows -- so the only symptom is a
+ correctly configured mapping that silently never prunes.
+ """
+ assert build_probe_sql("lower(:value)", ["US"]) == "SELECT lower('US') AS
v0"
+ assert build_probe_sql("lower(:value)", ["US"], None, " FROM
DUAL").endswith(
+ " FROM DUAL"
+ )
+ assert OracleEngineSpec.select_without_from_suffix == " FROM DUAL"
+ assert BaseEngineSpec.select_without_from_suffix == ""
+
+
[email protected]("dialect", [mysql.dialect(), postgresql.dialect()])
+def test_the_probe_select_does_not_double_percent_signs(dialect: Any) -> None:
+ """
+ `TextClause` compilation runs the dialect's `post_process_text`, which
+ doubles every `%` on any paramstyle whose DBAPI later interpolates
+ parameters -- MySQL and Postgres, and every spec inheriting from them. The
+ probe has no such interpolation pass: it executes with no bound parameters
+ at all. Left doubled, `date_format(:value, '%Y%m%d')` asks the warehouse
+ for the literal string `%%Y%%m%%d`, so the probe returns that instead of a
+ date and the mirrored predicate drops every row the filter keeps.
+ """
+ sql = build_probe_sql("date_format(:value, '%Y%m%d')", ["2026-01-15"],
dialect)
+
+ assert "'%Y%m%d'" in sql
+ assert "%%" not in sql
+
+
+def test_the_probe_select_is_unchanged_on_a_dialect_that_does_not_double() ->
None:
+ """
+ Pins the no-dialect path, which is also the blind spot: the default dialect
+ does not double percent signs, so no assertion made through it can see the
+ bug the two cases above cover.
+ """
+ assert (
+ build_probe_sql("date_format(:value, '%Y%m%d')", ["2026-01-15"])
+ == "SELECT date_format('2026-01-15', '%Y%m%d') AS v0"
+ )
+
+
+def test_a_raw_probe_value_is_substituted_literally() -> None:
+ r"""
+ `RawProbeValue.sql` is rendered from a filter value, so it reaches
+ `parse_skeleton` as the *replacement* for `:value`. Read as a replacement
+ template, a backslash in it is an escape: `array('a\nb')` -- which is how a
+ dialect renders the two characters `\` and `n` -- would become a real
+ newline, so the engine is asked about a different array than the predicate
+ compares and the mirror can exclude rows the filter keeps.
+ """
+ sql = build_probe_sql("toYYYYMM(:value)",
[RawProbeValue(r"array('a\nb')")])
+
+ assert sql == r"SELECT toYYYYMM(array('a\nb')) AS v0"
+
+
+def test_a_raw_probe_value_does_not_re_inject_the_placeholder() -> None:
+ r"""
+ `\g<0>` in a replacement template expands to the whole match, which would
+ put `:value` back into the probe -- an unbound parameter the engine
refuses.
+ """
+ sql = build_probe_sql("toYYYYMM(:value)", [RawProbeValue(r"'a\g<0>b'")])
+
+ assert sql == r"SELECT toYYYYMM('a\g<0>b') AS v0"
+ assert ":value" not in sql
+
+
+def test_a_raw_probe_value_with_a_group_reference_does_not_raise() -> None:
+ r"""
+ `\1` is an invalid group reference for this pattern, so as a replacement
+ template it raised `re.PatternError` rather than probing anything.
+ """
+ sql = build_probe_sql("toYYYYMM(:value)", [RawProbeValue(r"'a\1b'")])
+
+ assert sql == r"SELECT toYYYYMM('a\1b') AS v0"
+
+
+def test_a_probed_literal_keeps_its_percent_sign(app: Flask) -> None:
+ """
+ The preview panel renders probed values through the same dialect, so a
+ value that merely contains a `%` would be shown doubled.
+ """
+ database = Database(database_name="pct_db",
sqlalchemy_uri="postgresql://h/d")
+
+ with app.app_context():
+ assert _render_literal(database, "100%") == "'100%'"
+
+
+def test_the_probe_reads_each_cell_without_row_wide_dtype_coercion(
+ app: Flask,
+) -> None:
+ """
+ A row is a Series, so it carries one dtype across every probe column: a row
+ mixing an exact int64 with a float unifies to float64 and an integer
+ partition key above 2^53 comes back rounded. The mirror then asks for a key
+ the warehouse does not hold, and the row the filter matched is dropped.
+ """
+ database = _database_returning_frame(
+ pd.DataFrame({"v0": [9007199254740993], "v1": [1.5]})
+ )
+
+ with app.app_context():
+ probed = evaluate_transform(
+ database, None, None, "CAST(:value AS NUMERIC)",
["9007199254740993", "1.5"]
+ )
+
+ assert probed == [9007199254740993, 1.5]
+
+
+def test_evaluate_transform_pins_catalog_and_schema(app: Flask) -> None:
+ """
+ The probe runs in a different session from the chart query; pinning the
+ catalog and schema keeps session settings as close as the pool allows.
+ """
+ database = _database_returning([1])
+
+ with app.app_context():
+ evaluate_transform(database, "prod", "analytics", "lower(:value)",
["x"])
+
+ kwargs = _probe(database).call_args.kwargs
+ assert kwargs["catalog"] == "prod"
+ assert kwargs["schema"] == "analytics"
+
+
+def test_evaluate_transform_fails_open_when_the_probe_raises(app: Flask) ->
None:
+ """A wedged engine must not break the chart — it just stops pruning."""
+ database = Database(database_name="probe_db", sqlalchemy_uri="sqlite://")
+ database.get_df = MagicMock( # type: ignore[method-assign]
+ side_effect=RuntimeError("connection reset")
+ )
+
+ with app.app_context():
+ assert evaluate_transform(database, None, None, "lower(:value)",
["x"]) is None
+
+
+def test_evaluate_transform_fails_open_on_an_empty_result(app: Flask) -> None:
+ database = Database(database_name="probe_db", sqlalchemy_uri="sqlite://")
+ database.get_df = MagicMock( # type: ignore[method-assign]
+ return_value=pd.DataFrame()
+ )
+
+ with app.app_context():
+ assert evaluate_transform(database, None, None, "lower(:value)",
["x"]) is None
+
+
+def test_evaluate_transform_returns_none_for_no_values(app: Flask) -> None:
+ database = _database_returning([])
+
+ with app.app_context():
+ assert evaluate_transform(database, None, None, "lower(:value)", [])
is None
+
+ _probe(database).assert_not_called()
+
+
[email protected](
+ "transform",
+ [
+ "(SELECT password FROM ab_user LIMIT 1) || :value",
+ "lower(:value) UNION ALL SELECT password FROM ab_user",
+ "lower(:value); DROP TABLE ab_user",
+ "password || :value FROM ab_user",
+ "lower(:value) FROM ab_user WHERE 1 = 1",
+ ],
+ ids=[
+ "subquery",
+ "set-operation",
+ "multi-statement",
+ "bare-from",
+ "from-and-where",
+ ],
+)
+def test_a_transform_that_is_not_a_storable_expression_never_runs(
+ app: Flask, transform: str
+) -> None:
+ """
+ The probe is where a transform stops being text and becomes SQL.
+
+ `build_probe_sql` binds `:value` and splices the rest in verbatim, so a
+ transform that was never a single harmless expression is arbitrary SQL
+ against the dataset's database -- read back out of the emitted predicate
+ by anyone who can open "View query". The write-side gates refuse these,
+ but they can only speak for rows written after they existed, so the last
+ word belongs here.
+ """
+ database = _database_returning(["irrelevant"])
+
+ with app.app_context():
+ assert evaluate_transform(database, None, None, transform, ["us"]) is
None
+
+ _probe(database).assert_not_called()
+
+
[email protected](
+ "transform, unfinished",
+ [
+ ("unix_timestamp(:value", True),
+ ("unix_timestamp(:value))", True),
+ ("unix_timestamp(:value) +", True),
+ ("unix_timestamp(:value)", False),
+ ("unix_timestamp(:value); DROP TABLE ab_user", False),
+ ("unix_timestamp(:value) UNION ALL SELECT password FROM ab_user",
False),
+ ("password || :value FROM ab_user", False),
+ ],
+)
+def test_is_unfinished_separates_not_sql_yet_from_the_wrong_sql(
+ transform: str, unfinished: bool
+) -> None:
+ """
+ The two deserve opposite treatment -- a half-typed transform is stored and
+ parked inactive, the wrong SQL is refused -- and neither `is_parseable` nor
+ parsing as a single statement tells them apart, because the multi-statement
+ form fails both exactly as unfinished text does. Parsing as a script does.
+ """
+ assert is_unfinished(transform, "hive") is unfinished
+
+
+def test_a_transform_smuggling_its_own_from_clause_is_refused(app: Flask) ->
None:
+ """
+ The hole this gate was added for.
+
+ `is_parseable` counts only the projection, so `password || :value FROM
+ ab_user` reads as a single select expression and passed every write-side
+ gate -- including with `ALLOW_ADHOC_SUBQUERY` off, since a top-level FROM
+ is not a sub-query. `build_probe_sql` then emitted `SELECT password || 'us'
+ AS v0 FROM ab_user`, where its own alias is read as a table alias on the
+ FROM clause the transform brought with it: valid SQL, one row, and the
+ secret handed back through the predicate the preview panel renders.
+ """
+ database = Database(database_name="probe_db", sqlalchemy_uri="sqlite://")
+ transform = "password || :value FROM ab_user"
+
+ assert is_parseable(transform, "sqlite") is True
+ assert is_bare_expression(transform, "sqlite") is False
+
+ with app.app_context():
+ reason = stored_expression_error(database, None, None, transform)
+ assert reason is not None
+
+ blocking = _blocking(validate_transform(transform, "sqlite"))
+ assert len(blocking) == 1
+ assert "single SQL expression" in str(blocking[0].message)
+ assert is_transform_active(transform, "sqlite") is False
+
+
+def test_a_subquery_transform_is_refused_even_with_adhoc_subquery_enabled(
+ app: Flask,
+) -> None:
+ """
+ `validate_adhoc_subquery` admits a sub-query under `ALLOW_ADHOC_SUBQUERY`
+ and returns RLS-rewritten SQL -- which this path has no way to propagate,
+ because `build_probe_sql` splices the original transform text. So the probe
+ ran a sub-query with no row-level-security predicates at all and reported
+ the hidden row in `emitted_predicate`. A transform is a scalar function of
+ `:value`, so refusing the sub-query outright is both correct and what makes
+ the missing RLS rewrite moot.
+ """
+ database = Database(database_name="probe_db", sqlalchemy_uri="sqlite://")
+ transform = "(SELECT secret FROM vault LIMIT 1) || :value"
+
+ with app.app_context():
+ app.config["DEFAULT_FEATURE_FLAGS"]["ALLOW_ADHOC_SUBQUERY"] = True
+ try:
+ reason = stored_expression_error(database, None, None, transform)
+ finally:
+ del app.config["DEFAULT_FEATURE_FLAGS"]["ALLOW_ADHOC_SUBQUERY"]
+
+ assert reason is not None
+ assert "sub-query" in reason
+
+
[email protected](
+ "comment",
+ ["/*!50000 + (SELECT secret FROM vault LIMIT 1) */", "/*M!50000 + 1 */"],
+)
+def test_an_executable_comment_is_refused_by_every_gate(
+ app: Flask, comment: str
+) -> None:
+ """
+ The one construct that made the shape gates describe a statement the engine
+ would not run.
+
+ sqlglot reads a MySQL executable comment as comment data: the pinned parse
+ below reports no sub-query and no clause, so `stored_expression_error` and
+ `probe_sql_is_evaluable` both passed it, while MySQL and MariaDB execute
+ what is inside. An owner with dataset write and no SQL Lab could put one in
+ a transform, call the preview and read a table back out of
+ `emitted_predicate`, with no RLS on the hidden query.
+
+ Refused on every engine, not only the two that honour it -- hence SQLite
+ here. There is no legitimate transform carrying one, and a per-engine gate
+ would store on Postgres what detonates after the dataset is re-pointed.
+ """
+ database = Database(database_name="probe_db", sqlalchemy_uri="sqlite://")
+ transform = f"(:value {comment})"
+
+ # The premise: no parse-tree gate can see it.
+ statement = SQLStatement(f"SELECT (1 {comment}) AS v0", "mysql")
+ assert statement.has_subquery() is False
+ assert statement.get_clause_names() == set()
+
+ with app.app_context():
+ assert stored_expression_error(database, None, None, transform) is not
None
+
+ blocking = [
+ issue for issue in validate_transform(transform, "mysql") if
issue.blocking
+ ]
+ assert len(blocking) == 1
+ assert "executable comment" in str(blocking[0].message)
+
+
+def test_an_executable_comment_stops_the_probe_before_the_engine(app: Flask)
-> None:
+ """
+ The gate above is the save path. This is the row an earlier release stored:
+ `_probe` consults `stored_expression_error` on every call, so a transform
+ already in the metadata database never reaches `get_df`.
+ """
+ database = _database_returning(["x"])
+ transform = "(:value /*!50000 + (SELECT secret FROM vault LIMIT 1) */)"
+
+ with app.app_context():
+ assert evaluate_transform(database, None, None, transform, ["US"]) is
None
+
+ assert _probe(database).call_count == 0
+
+
+def test_a_plain_comment_is_still_allowed_through_the_shape_gates(
+ app: Flask,
+) -> None:
+ """
+ Only the executable form is refused. An ordinary comment is inert on every
+ engine, so rejecting it would cost an owner a transform that works -- and
+ the placeholder gate already covers the case that matters, a `:value` the
+ comment swallows.
+ """
+ database = Database(database_name="probe_db", sqlalchemy_uri="sqlite://")
+
+ with app.app_context():
+ assert (
+ stored_expression_error(database, None, None, "lower(:value) /* ok
*/")
+ is None
+ )
+
+
+def test_a_transform_calling_a_disallowed_function_is_refused(app: Flask) ->
None:
+ """
+ The probe executes the transform, so the operator's function denylist has
+ to apply to it for the same reason it applies in SQL Lab.
+ """
+ database = Database(database_name="probe_db", sqlalchemy_uri="sqlite://")
+ transform = "version() || :value"
+
+ with app.app_context():
+ original = app.config["DISALLOWED_SQL_FUNCTIONS"]
+ try:
+ app.config["DISALLOWED_SQL_FUNCTIONS"] = {"sqlite": {"version"}}
+ reason = stored_expression_error(database, None, None, transform)
+ assert reason is not None
+ assert "version" in reason
+
+ # Named, rather than echoing the operator's whole denylist back.
+ assert "current_user" not in reason
+
+ app.config["DISALLOWED_SQL_FUNCTIONS"] = {}
+ assert stored_expression_error(database, None, None, transform) is
None
+ finally:
+ # Restored: `app.config` is shared, and leaving it emptied silently
+ # disabled this gate for every test that ran afterwards.
+ app.config["DISALLOWED_SQL_FUNCTIONS"] = original
+
+
[email protected](
+ "function",
+ [
+ "database_to_xml",
+ "database_to_xmlschema",
+ "database_to_xml_and_xmlschema",
+ "schema_to_xml",
+ "schema_to_xmlschema",
+ "schema_to_xml_and_xmlschema",
+ "table_to_xml",
+ "table_to_xmlschema",
+ "table_to_xml_and_xmlschema",
+ "query_to_xml",
+ "query_to_xmlschema",
+ "query_to_xml_and_xmlschema",
+ ],
+)
+def test_the_whole_postgres_xml_family_is_denied_by_default(function: str) ->
None:
+ """
+ These read tables with no FROM clause and no sub-query, so every gate that
+ reasons about table references is blind to them -- the denylist is the only
+ thing that can refuse them, and it has to name each shape. `schema_to_xml`
+ and three siblings were missing, so a bare scalar call cleared
+ `stored_expression_error` entirely and returned connection-user-readable
+ schema contents through preview.
+
+ Asserted against the shipped default rather than through the gate, because
+ the gap was an incomplete enumeration rather than broken wiring -- and
+ because `app.config` is ambient here, which the behavioural test below
+ handles by setting the denylist itself.
+ """
+ assert function in DISALLOWED_SQL_FUNCTIONS["postgresql"]
+
+
+def test_a_denied_read_capable_function_is_refused_despite_having_no_from(
+ app: Flask,
+) -> None:
+ """
+ The wiring half: a bare scalar call passes the length, single-statement,
+ bare-expression, sub-query and placeholder gates, so the denylist is what
+ has to stop it.
+ """
+ database = Database(database_name="probe_db",
sqlalchemy_uri="postgresql://u@h/d")
+ transform = "schema_to_xml('public', true, false, '') || :value"
+
+ with app.app_context():
+ original = app.config["DISALLOWED_SQL_FUNCTIONS"]
+ app.config["DISALLOWED_SQL_FUNCTIONS"] = {"postgresql":
{"schema_to_xml"}}
+ try:
+ reason = stored_expression_error(database, None, None, transform)
+ finally:
+ app.config["DISALLOWED_SQL_FUNCTIONS"] = original
+
+ assert reason is not None
+ assert "schema_to_xml" in reason
+
+
[email protected]("function", ["ts_rewrite", "ts_stat"])
+def test_the_postgres_full_text_query_functions_are_denied_by_default(
+ function: str,
+) -> None:
+ """
+ `ts_stat('select ...')` and `ts_rewrite(q, 'select ...')` run their *text*
+ argument as a query. The query is a string, so these have the same blind
+ spot the XML family above does: no FROM clause and no sub-query for a
+ table-reference gate to reason about, which leaves the denylist as the only
+ thing that can refuse them.
+ """
+ assert function in DISALLOWED_SQL_FUNCTIONS["postgresql"]
+
+
[email protected](
+ "function", ["getxml", "getxmltype", "newcontext",
"newcontextfromhierarchy"]
+)
+def test_the_oracle_query_running_xml_functions_are_denied_by_default(
+ function: str,
+) -> None:
+ """
+ `DBMS_XMLGEN.GETXML('select secret from vault')` runs its text argument as
+ a query -- the same blind spot as the PostgreSQL families above, on the
+ engine both reviewers named. Listed bare because the package prefix never
+ reaches the matcher: sqlglot parses the call as an anonymous function named
+ `GETXML`, and `get_disallowed_functions` compares against that name.
+ """
+ assert function in DISALLOWED_SQL_FUNCTIONS["oracle"]
+
+
+def test_a_package_qualified_denied_function_is_refused(app: Flask) -> None:
+ """
+ The wiring half for the bare-name listing above: the denylist entry has to
+ match a call written with its package prefix, which is the only way anyone
+ writes these.
+ """
+ database = Database(database_name="probe_db",
sqlalchemy_uri="oracle://u@h/d")
+ transform = "DBMS_XMLGEN.GETXML('select secret from vault') || :value"
+
+ with app.app_context():
+ reason = stored_expression_error(database, None, None, transform)
+
+ assert reason is not None
+ assert "getxml" in reason
+
+
+def test_a_denied_query_running_text_function_is_refused(app: Flask) -> None:
+ """
+ The wiring half, and the only thing that proves the *name* matching works
+ for these two: neither is a function sqlglot models, so each reaches
+ `get_disallowed_functions` through its ANONYMOUS branch rather than as a
+ typed node. A denylist entry that only matched modelled functions would
+ pass the enumeration test above and still let these through.
+ """
+ database = Database(database_name="probe_db",
sqlalchemy_uri="postgresql://u@h/d")
+ transform = "ts_stat('select 1') || :value"
+
+ with app.app_context():
+ original = app.config["DISALLOWED_SQL_FUNCTIONS"]
+ app.config["DISALLOWED_SQL_FUNCTIONS"] = {"postgresql": {"ts_stat"}}
+ try:
+ reason = stored_expression_error(database, None, None, transform)
+ finally:
+ app.config["DISALLOWED_SQL_FUNCTIONS"] = original
+
+ assert reason is not None
+ assert "ts_stat" in reason
+
+
+def test_the_function_denylist_is_keyed_on_the_engine_spec_name(app: Flask) ->
None:
+ """
+ Every other denylist gate in the codebase keys on the engine spec's own
+ name, which is also the name the config documentation tells an operator to
+ write, so this one does too -- a spec covering several SQLAlchemy backends
+ through `engine_aliases` reports the one canonical name under which its
+ entry exists.
+
+ Tested through an *aliased* engine, which is the only way to tell the two
+ names apart. On `sqlite://` the backend and the spec name are both
+ `"sqlite"`, so the assertion held whichever one the gate read, and the test
+ did not check what its name promises. `postgres://` is
+ `PostgresEngineSpec.engine_aliases`, so the URL's backend is `postgres`
+ while the spec -- and the config key an operator writes -- is `postgresql`.
+ """
+ database = Database(database_name="probe_db",
sqlalchemy_uri="postgres://u@h/d")
+
+ with app.app_context():
+ # The premise, asserted rather than assumed: if this alias ever goes
+ # away, the test below stops discriminating and should say so here.
+ assert database.backend == "postgres"
+ assert database.db_engine_spec.engine == "postgresql"
+
+ # Denied only under the spec's name. Keyed on `database.backend` the
+ # gate would find no entry and let the call through.
+ app.config["DISALLOWED_SQL_FUNCTIONS"] = {"postgresql": {"version"}}
Review Comment:
`app` is module-scoped, and this test (like the one at :1517) overwrites
`app.config["DISALLOWED_SQL_FUNCTIONS"]` without restoring it. Run before
`test_a_package_qualified_denied_function_is_refused` (for example with `-k`,
random ordering or xdist), that test sees `{"postgresql": {"version"}}`, has no
Oracle entry, and fails its refusal assertion even though the shipped defaults
are correct. Could these use `monkeypatch.setitem` or the try/finally restore
the neighbouring tests use?
##########
superset/connectors/sqla/partition_mapping.py:
##########
@@ -0,0 +1,2929 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Partition filter mapping.
+
+Datasets on Hadoop-family engines are commonly partitioned on a *technical*
+column -- an epoch integer, a lowercased region key -- that no analyst would
+filter on. Unless a query carries a predicate on that column the engine scans
+every partition.
+
+A dataset owner names one partition column ``p``, one business column that
+filters are mirrored from, and a value transform ``T`` (a SQL expression
+containing a ``:value`` placeholder). Superset then appends an equivalent
+predicate on ``p`` to every query, so chart authors change nothing and queries
+prune.
+
+The load-bearing assumption
+---------------------------
+Everything here reasons about ``T(col) op T(v)``, but what is emitted is
+``p op T(v)`` -- a predicate on a *physically different column*. The step from
+one to the other is::
+
+ p = T(mapped_col) for every row in the table
+
+Superset cannot verify that; it is a property of whatever ETL populates the
+partition column. If that job lags, backfills with different logic, or writes
+the partition key in a different timezone, mirrored predicates silently drop
+real rows. The mapping is only as trustworthy as the pipeline behind it.
+
+Operator safety
+---------------
+A mirrored predicate ``P2`` may only be ``AND``-ed onto a query when the
+original predicate ``P1`` *implies* it:
+
+=========================================== =============================
+Original Safe when
+=========================================== =============================
+``col = v``, ``col IN (...)`` ``T`` is a function *and* the
+ engine compares ``col``
+ byte-exactly
+``col >=|>|<|<= v``, ``TEMPORAL_RANGE`` only if ``T`` is monotonic
+``col != v``, ``NOT IN``, ``LIKE``, ... never
+=========================================== =============================
+
+Monotonic here means non-decreasing, not strictly increasing: a bucketing
+transform such as a day key is monotonic and maps a whole day of timestamps
+onto one value. ``col < v`` then only implies ``T(col) <= T(v)``, so a strict
+bound is mirrored non-strictly -- see `mirror_operator`.
+
+Equality under the engine's own rules
+-------------------------------------
+"``T`` is a function" gives ``col = v`` implies ``T(col) = T(v)`` for *value*
+equality. The engine compares with *SQL* equality, and the two part company
+wherever that comparison is not byte-exact: under a case-insensitive collation
+a stored ``country`` of ``'us'`` satisfies a filter for ``'US'``, while the
+mirror ``region_key = hex('US')`` excludes the row and the chart loses it with
+nothing to indicate why. Trailing-space-insensitive ``CHAR`` comparison says
+the same thing about padding.
+
+So equality and ``IN`` are withdrawn for a *string* mapped column unless the
+engine spec declares ``binary_string_comparison`` -- `equality_mirrors_safely`.
+Numeric and temporal comparison is exact everywhere and is untouched, and range
+mirroring is untouched too: it already rests on the owner declaring ``T``
+order-preserving *with respect to the column's own order*, which is an
+assertion about the very semantics this is checking for.
+
+What the gate cannot see is a column declaring its own collation
+(``country COLLATE NOCASE``) on an otherwise byte-exact engine. Nothing in
+SQLAlchemy's reflection or the engine specs exposes that, so there it remains
+the owner's assumption, like ``p = T(mapped_col)`` itself.
+
+Resolution of the value the engine compares
+-------------------------------------------
+Every row of the table above assumes the engine compares the value the mirror
+was built from. It does not always. A ``DATE`` column compared against
+``'2026-07-06 10:08:11'`` is compared on the date part alone -- whether the
+engine writes the literal itself (``TO_DATE('2026-07-06', 'YYYY-MM-DD')`` on
+Postgres, ``DATE '2026-07-06'`` on Presto, Trino, BigQuery and some two dozen
+others) or applies an implicit cast to a bound string. The filter therefore
+keeps the whole of that day while ``p = T(2026-07-06 10:08:11)`` keeps one
+instant of it: narrower than the predicate it stands in for, which is the one
+thing a mirror may never be.
+
+What an engine throws away is detected from the column's type rather than
+enumerated per engine -- `ExploreMixin._engine_literal_resolution` -- and what
+to do about it is a property of the *operator*, not of the value:
+
+* a bound rounds outward, backward for a lower one and forward for an upper
one,
+ through `ExploreMixin._round_bounds_outward`. The mirror then reads one extra
+ bucket at the edge, and the original predicate is still in the query to
+ exclude the rows that bucket adds.
+* an equality, or one member of an ``IN`` list, has nowhere to widen to: these
+ predicates are ``AND``-ed onto the query, so "or the day either side" is not
+ something the mirror can express. They decline, which costs the query its
+ pruning and nothing else.
+
+So a ``DATE``-typed mapped column mirrors a range at day granularity, and does
+not mirror an equality whose value carries a time of day at all. Map a
+``TIMESTAMP`` column instead and every operator is available again.
+
+Negations are never safe because ``T`` need not be injective:
+``lower(:value)`` with ``country != 'US'`` mirrors to ``region_key != 'us'``,
+which wrongly excludes rows whose ``country`` is already lowercase ``'us'`` --
+rows the original filter *keeps*.
+
+Monotonicity is a property of the transform, not of the column's data type:
+``hour(:value)``, ``date_format(:value, 'dd')`` and ``dayofweek(:value)`` are
+all reasonable transforms on a ``TIMESTAMP`` column and none of them preserve
+ordering. It is therefore declared by the owner, not inferred.
+
+Time grains
+-----------
+A grained filter -- drill-to-detail, mostly -- compares the *truncated* column,
+``trunc(col) op v``, so the raw bounds it carries do not describe the rows it
+keeps. A row in the final partial bucket satisfies ``trunc(col) < until`` while
+``col < until`` excludes it.
+
+Which direction a grain rounds is not knowable from the duration alone, and the
+obvious guess is wrong: ``WEEK_ENDING_SATURDAY`` rounds *forward* on Hive and
+Presto, and Ocient's grains are ``ROUND``, i.e. to nearest. What every grain
+does satisfy is a bound on the displacement::
+
+ |trunc(ts) - ts| < width(grain)
+
+so widening *both* bounds by one bucket width is no narrower than the real
+predicate whichever way the grain rounds -- and it needs no monotonicity of
+``trunc`` itself, which is what rescues Hive's oddly-anchored ``P1W``. Width is
+a property of the grain, not of the engine, which is what makes this
+maintainable; see `grain_bucket_width`. A grain whose SQL an operator supplied
+has no known width, and does not mirror at all.
+"""
+
+from __future__ import annotations
+
+import hashlib
+import logging
+import numbers
+import re
+from collections.abc import Sequence
+from dataclasses import dataclass
+from datetime import date, datetime
+from functools import lru_cache
+from typing import Any, cast, TYPE_CHECKING
+
+import numpy as np
+import pandas as pd
+import sqlalchemy as sa
+from dateutil.relativedelta import relativedelta
+from flask import current_app as app
+from flask_babel import lazy_gettext as _
+from sqlalchemy.engine.interfaces import Dialect
+from sqlalchemy.sql.elements import ColumnElement
+
+from superset.constants import LRU_CACHE_MAX_SIZE, TimeGrain
+from superset.exceptions import (
+ QueryClauseValidationException,
+ SupersetParseError,
+ SupersetSecurityException,
+)
+from superset.extensions import cache_manager, feature_flag_manager
+from superset.sql.parse import SQLScript, SQLStatement
+from superset.superset_typing import FilterValues
+from superset.utils import core as utils, json
+from superset.utils.core import FilterOperator, get_username
+
+if TYPE_CHECKING:
+ from superset.connectors.sqla.models import SqlaTable, TableColumn
+ from superset.db_engine_specs.base import BaseEngineSpec
+ from superset.models.core import Database
+
+logger = logging.getLogger(__name__)
+
+FEATURE_FLAG = "PARTITION_FILTER_MAPPING"
+
+#: Longest transform accepted anywhere. A transform is one SQL expression an
+#: owner types by hand, so this is generous; the point is that it is bounded.
+#: The bound matters because `is_transform_active` parses the stored value on
+#: each Explore load, and an unbounded string would make that parse the
+#: expensive part of rendering a chart.
+#:
+#: The typed column field, the import schema and the preview request each
+#: enforce it through a marshmallow `Length` validator, which is what produces
+#: the per-field message those three callers want. `stored_expression_error`
+#: enforces it too, for the paths that load no schema -- the deprecated
+#: `POST /datasource/save/` is one, and it writes straight through
+#: `update_from_object`.
+MAX_TRANSFORM_LENGTH = 1024
+
+#: Placeholder the owner writes in the transform, e.g.
``unix_timestamp(:value)``.
+#: Matched with word boundaries so ``:values`` is not mistaken for it.
+VALUE_PLACEHOLDER_RE = re.compile(r":value\b")
+
+#: Balanced Jinja blocks. The probe would render these in a different context
+#: at a different time from the chart query, so they are rejected at save time.
+JINJA_BLOCK_RE = re.compile(r"\{\{.*?\}\}|\{%.*?%\}|\{#.*?#\}", re.DOTALL)
+
+#: MySQL and MariaDB "executable comments": ``/*!50000 ... */`` and
+#: ``/*M!50000 ... */`` are comments to every other reader and *statement text*
+#: to those two engines.
+#:
+#: This is the one construct that breaks the premise the probe's shape gates
+#: rest on -- that the parse this module inspects and the statement the engine
+#: runs are the same statement. ``1 /*!50000 + (SELECT secret FROM vault) */``
+#: parses with no sub-query and no FROM, clears `stored_expression_error` and
+#: `probe_sql_is_evaluable` alike, and then reads a table on MySQL. The result
+#: comes back to a dataset owner through the preview's ``emitted_predicate``,
+#: with no RLS applied and no SQL Lab access needed.
+#:
+#: Refused on every engine rather than only the two that honour it. A transform
+#: is a scalar function of ``:value``; there is no legitimate reason for one to
+#: carry a comment of any kind, let alone this one, and a per-engine gate would
+#: store a transform on Postgres that detonates when the dataset is re-pointed
+#: at MySQL.
+EXECUTABLE_COMMENT_RE = re.compile(r"/\*M?!")
+
+#: Substituted for ``:value`` before parsing -- sqlglot rejects a bare
``:value``
+#: on most dialects. Mirrors the ``_JINJA_BLOCK_RE`` -> ``NULL`` trick used by
+#: ``validate_stored_expression``.
+_PARSE_STANDIN = "NULL"
+
+#: Substituted for ``:value`` to find out *where* the placeholder landed, which
+#: `_PARSE_STANDIN` cannot answer: ``NULL`` is a keyword, so after substitution
+#: the parse tree no longer records that a placeholder was ever there. An
+#: identifier does leave a trace -- a column reference -- and one that is
+#: missing is a placeholder the engine will never evaluate.
+#:
+#: Named so it cannot plausibly collide with a real column, but a transform
that
+#: writes it anyway is refused rather than counted. A hand-written occurrence
+#: does not merely make the count disagree: it *supplies* the column reference
a
+#: non-executable placeholder failed to produce, and the two cancel exactly.
See
+#: `placeholder_is_executable`.
+_PLACEHOLDER_STANDIN = "superset_pfm_value_standin"
+
+#: Functions whose value depends on wall-clock time or randomness. The probe
+#: runs in a different session at a different moment from the chart query and
+#: its result is then cached, so any of these freezes a snapshot of probe time
+#: into the emitted predicate.
+NON_DETERMINISTIC_FUNCTIONS = {
+ "CURRENT_DATE",
+ "CURRENT_TIME",
+ "CURRENT_TIMESTAMP",
+ "NOW",
+ "RAND",
+ "RANDOM",
+ "UUID",
+}
+
+#: Functions that mean "now" only in their zero-argument form. On Hive and
+#: Impala ``unix_timestamp()`` is the current time while ``unix_timestamp(x)``
+#: -- the canonical transform for this feature -- is pure.
+NON_DETERMINISTIC_WHEN_NILADIC = {"UNIX_TIMESTAMP"}
+
+#: Safe for any function ``T``.
+MIRRORABLE_ALWAYS = {FilterOperator.EQUALS, FilterOperator.IN}
+
+#: Safe only when ``T`` preserves ordering.
+MIRRORABLE_IF_MONOTONIC = {
+ FilterOperator.GREATER_THAN,
+ FilterOperator.GREATER_THAN_OR_EQUALS,
+ FilterOperator.LESS_THAN,
+ FilterOperator.LESS_THAN_OR_EQUALS,
+ FilterOperator.TEMPORAL_RANGE,
+}
+
+
+#: Every operator that can be mirrored under *some* transform. The preview
+#: endpoint accepts these; whether a given one actually mirrors still depends
on
+#: the monotonicity declaration.
+MIRRORABLE_OPERATORS = MIRRORABLE_ALWAYS | MIRRORABLE_IF_MONOTONIC
+
+#: Operators the preview endpoint can construct. `TEMPORAL_RANGE` is mirrored
by
+#: the query path, but as *two* bounds that `_collect_partition_mirror_range`
+#: decomposes a time range into -- the preview request carries sample values,
not
+#: a since/until pair, so there is no range for it to build. The editor never
+#: asks for one either: `previewOperatorFor` sends `>=`, `=` or `IN`.
+PREVIEWABLE_OPERATORS = MIRRORABLE_OPERATORS - {FilterOperator.TEMPORAL_RANGE}
+
+
+class Unmirrorable:
+ """
+ A filter no mirror can stand in for, and why.
+
+ Returned by `ExploreMixin.mirror_probe_request` for a value the engine
+ compares more coarsely than the filter carries, one it cannot render as a
+ literal, one in a time frame the transform would discard, or an ``IN`` list
+ with a member of any of those kinds. An object rather than `None`, because
+ `None` is a filter value in its own right.
+
+ The reason rides along because the preview has to explain the decline to
+ the owner, and the two declines want different advice: "the engine compares
+ less of this value than you typed, so map a column it compares in full" is
+ sound guidance for a `DATE` column and actively misleading for an
+ offset-bearing bound on a `TIMESTAMP` one, which already is such a column.
+ Carried rather than re-derived so the preview's explanation cannot come
+ apart from the chart path's decision.
+
+ Lives here rather than beside that method so the preview can read it too:
+ `superset.models.helpers` imports this module, so these can only cross the
+ boundary in this direction. Both callers recognise the same instances -- a
+ preview that answered "valid" for a request the chart path declines is the
+ drift this is here to prevent.
+ """
+
+ __slots__ = ("reason",)
+
+ def __init__(self, reason: str) -> None:
+ self.reason = reason
+
+ def __repr__(self) -> str:
+ return f"<Unmirrorable {self.reason}>"
+
+
+#: The ordinary decline: whatever the mirror cannot be built from, with the
+#: engine's own comparison resolution as the reason an owner can act on.
+UNMIRRORABLE = Unmirrorable("resolution")
+
+#: A value carrying a UTC offset, declined whatever the column's resolution.
+#: A transform such as ``CAST(:value AS TIMESTAMP)`` drops the offset, so even
+#: an engine that compares the whole instant would be probed in the wrong
+#: frame -- which is not something mapping a different column fixes.
+UNMIRRORABLE_OFFSET = Unmirrorable("offset")
+
+
+def is_unmirrorable(value: Any) -> bool:
+ """Whether `mirror_probe_request` declined this request."""
+ return isinstance(value, Unmirrorable)
+
+
+#: A transform that preserves ordering need not be *strictly* increasing: a day
+#: key such as ``to_char(ts, 'YYYYMMDD')`` maps a whole day of timestamps onto
+#: one value. For such a ``T``, ``col < v`` only implies ``T(col) <= T(v)`` and
+#: ``col > v`` only implies ``T(col) >= T(v)`` -- the rows in the bucket at the
+#: boundary satisfy the non-strict form and not the strict one. A strict mirror
+#: therefore drops rows the filter keeps, which is the one thing a mirror may
+#: never do, so a strict bound is mirrored non-strictly.
+_NON_STRICT_EQUIVALENT = {
+ FilterOperator.GREATER_THAN: FilterOperator.GREATER_THAN_OR_EQUALS,
+ FilterOperator.LESS_THAN: FilterOperator.LESS_THAN_OR_EQUALS,
+}
+
+
+def mirror_operator(operator: FilterOperator) -> FilterOperator:
+ """
+ The operator a mirrored predicate uses to stand in for ``operator``.
+
+ Strict bounds widen; everything else passes through. Idempotent, so it is
+ safe wherever a request may already have been widened.
+
+ The widened bound reads one extra partition at the edge, and the original
+ predicate is still in the query to exclude the rows that partition adds --
+ the same trade as the grain widening and the ``NULL``-partition escape.
+ Declaring the transform strictly increasing would buy that partition back,
+ which is not worth a second property only the owner can vouch for.
+ """
+ return _NON_STRICT_EQUIVALENT.get(operator, operator)
+
+
+def mirrorable_operators(
+ is_monotonic: bool, *, equality_is_safe: bool = True
+) -> set[FilterOperator]:
+ """
+ The operators whose predicates may be mirrored onto the partition column.
+
+ :param is_monotonic: whether the owner declared the transform
+ order-preserving
+ :param equality_is_safe: whether the engine's ``=`` on the mapped column
+ compares the way ``T`` was written against -- see
+ `equality_mirrors_safely`
+
+ Both call sites -- the query path through `PartitionMapping.mirrors` and
the
+ Explore indicator through `partition_filter_mapping_summary` -- come
through
+ here, which is what keeps the glyph from promising pruning the SQL will not
+ do.
+ """
+ operators = MIRRORABLE_ALWAYS if equality_is_safe else set()
+ if is_monotonic:
+ operators = operators | MIRRORABLE_IF_MONOTONIC
+ return set(operators)
+
+
+#: Declared types whose comparison ignores trailing blanks.
+#:
+#: The fixed-width character types, which the SQL standard has compare as
+#: though both sides were padded to the same length -- so ``'US'`` and
+#: ``'US '`` are equal in a ``CHARACTER(3)``, while ``lower()`` of the two is
+#: not. Spelled out rather than matched by prefix because the variable-width
+#: names start with the same letters: ``VARCHAR`` and ``CHARACTER VARYING``
+#: compare the blanks and must not land here.
+PADDED_CHARACTER_TYPES = frozenset(
+ {
+ "CHAR",
+ "NCHAR",
+ # PostgreSQL's own name for ``CHARACTER``, which is what reflection
+ # reports for a ``CHAR(n)`` column there.
+ "BPCHAR",
+ "CHARACTER",
+ "NATIONAL CHAR",
+ "NATIONAL CHARACTER",
+ }
+)
+
+
+def _is_padded_character_type(column: TableColumn) -> bool:
+ """
+ Whether ``column``'s declared type compares without its trailing blanks.
+
+ Read off the declared type rather than the engine spec's resolved one: the
+ specs map ``CHAR`` and ``VARCHAR`` onto the same `sqlalchemy.String`, so
+ the distinction this needs is gone by then. The length suffix is dropped
+ and the name matched against `PADDED_CHARACTER_TYPES`, so an unfamiliar
+ spelling answers ``False`` and the engine-level flag decides as before --
+ this gate adds a decline rather than lifting one. A column with no
+ declared type has nothing to read, and answers the same way.
+ """
+ if not isinstance(column.type, str):
+ return False
+ native = re.sub(r"\s+", " ", column.type.strip().upper())
+ return native.split("(")[0].strip() in PADDED_CHARACTER_TYPES
+
+
+def equality_mirrors_safely(
+ column: TableColumn | None,
+ db_engine_spec: type[BaseEngineSpec],
+) -> bool:
+ """
+ Whether ``col = v`` on this column implies ``T(col) = T(v)`` in the engine.
+
+ The operator matrix calls equality safe "for any function ``T``" because
+ ``T`` is a function, so ``col = v`` gives ``T(col) = T(v)``. That reasons
+ about *value* equality while the engine reasons about *SQL* equality, and
+ the two part company under any comparison that is not byte-exact: with a
+ case-insensitive collation a stored ``country`` of ``'us'`` satisfies a
+ filter for ``'US'``, but the mirror ``region_key = hex('US')`` excludes the
+ row and the chart loses it with no indication why.
Trailing-space-insensitive
+ ``CHAR`` comparison says the same thing about padding.
+
+ Only string columns are at risk; numeric and temporal comparison is exact
+ everywhere. And only equality: range mirroring already requires the owner
+ to declare ``T`` order-preserving *with respect to the column's own order*,
+ which is an assertion about the same comparison semantics this is checking
+ for -- so the declaration covers it where equality has nothing to cover it.
+
+ Padding is asked about per column, because it is a property of the *type*
+ rather than of the engine. ``binary_string_comparison`` speaks for an
+ engine's default text comparison, and on PostgreSQL -- which declares it
+ -- a ``CHAR(2)`` still matches a filter for ``'US '`` against a stored
+ ``'US'``, because the standard says a padded character type compares
+ without its trailing blanks. The mirror ``region_key = lower('US ')`` then
+ excludes the row. So a fixed-width character type declines whatever the
+ engine says, and an engine that ignores trailing blanks on its *variable*
+ types says so by not declaring the flag at all (see `RedshiftEngineSpec`).
+
+ What this cannot see is a column that declares its own collation
+ (``country COLLATE NOCASE``) on an otherwise byte-exact engine. Neither
+ SQLAlchemy's reflection nor the engine specs expose it, so on such a column
+ the assumption stays where ``p = T(mapped_col)`` already is: with the
owner.
+ """
+ if column is None:
+ return True
+ # Ahead of the generic-type question, which a padded type can lose: the
+ # specs map ``CHARACTER`` onto no generic type at all on some engines, and
+ # a fixed-width character type is a string whether or not Superset can say
+ # so from the name.
+ if _is_padded_character_type(column):
+ return False
+ try:
+ is_string = column.type_generic == utils.GenericDataType.STRING
+ except Exception: # pylint: disable=broad-except # noqa: BLE001
+ # An unresolvable type is treated as a string: the gate exists to stop
+ # a silent wrong answer, so it fails closed.
+ return False
+ if not is_string:
+ return True
+ return bool(db_engine_spec.binary_string_comparison)
+
+
+#: How wide one bucket of each built-in time grain is.
+#:
+#: Keyed on the ISO duration a filter carries in its ``grain``. Written out
+#: rather than parsed: four of the week grains are ISO *intervals* with an
+#: anchor (``P1W/1970-01-03T00:00:00Z``) that ``isodate.parse_duration``
+#: rejects outright, and ``PT0.5H`` / ``P0.25Y`` are fractional. A literal
+#: table is also the thing a reviewer can check a line at a time.
+#:
+#: ``relativedelta`` rather than ``timedelta`` so the calendar grains stay
+#: calendar arithmetic: a month is not 30 days.
+GRAIN_BUCKET_WIDTHS: dict[str, relativedelta] = {
+ TimeGrain.SECOND: relativedelta(seconds=1),
+ TimeGrain.FIVE_SECONDS: relativedelta(seconds=5),
+ TimeGrain.THIRTY_SECONDS: relativedelta(seconds=30),
+ TimeGrain.MINUTE: relativedelta(minutes=1),
+ TimeGrain.FIVE_MINUTES: relativedelta(minutes=5),
+ TimeGrain.TEN_MINUTES: relativedelta(minutes=10),
+ TimeGrain.FIFTEEN_MINUTES: relativedelta(minutes=15),
+ TimeGrain.THIRTY_MINUTES: relativedelta(minutes=30),
+ TimeGrain.HALF_HOUR: relativedelta(minutes=30),
+ TimeGrain.HOUR: relativedelta(hours=1),
+ TimeGrain.SIX_HOURS: relativedelta(hours=6),
+ TimeGrain.DAY: relativedelta(days=1),
+ TimeGrain.WEEK: relativedelta(days=7),
+ TimeGrain.WEEK_STARTING_SUNDAY: relativedelta(days=7),
+ TimeGrain.WEEK_STARTING_MONDAY: relativedelta(days=7),
+ TimeGrain.WEEK_ENDING_SATURDAY: relativedelta(days=7),
+ TimeGrain.WEEK_ENDING_SUNDAY: relativedelta(days=7),
+ TimeGrain.MONTH: relativedelta(months=1),
+ TimeGrain.QUARTER: relativedelta(months=3),
+ TimeGrain.QUARTER_YEAR: relativedelta(months=3),
+ TimeGrain.YEAR: relativedelta(years=1),
+}
+
+
+def grain_bucket_width(grain: str | None, engine: str) -> relativedelta | None:
+ """
+ How far a grain's truncation can move a timestamp, or ``None`` if unknown.
+
+ A grained filter compares the *truncated* column, so the raw bounds it
+ carries do not describe the rows it keeps. Widening both bounds by one
+ bucket recovers a predicate that is no narrower than the real one -- see
+ `_collect_partition_mirror_range` for the argument. That only works for a
+ grain whose bucket width Superset knows, which excludes anything an
+ operator supplied.
+
+ :param grain: the ISO duration from the filter's ``grain``
+ :param engine: the engine spec's ``engine``, to check per-engine overrides
+ """
+ if not grain:
+ return None
+
+ # An operator-declared grain carries whatever duration string they typed,
+ # and `TIME_GRAIN_ADDON_EXPRESSIONS` can redefine a *built-in* grain's SQL
+ # per engine -- `P1D` could be anything at all. Neither has a width we can
+ # claim to know, so both fall back to not mirroring.
+ if grain in app.config["TIME_GRAIN_ADDONS"]:
+ return None
+ if grain in app.config["TIME_GRAIN_ADDON_EXPRESSIONS"].get(engine, {}):
+ return None
+
+ return GRAIN_BUCKET_WIDTHS.get(grain)
+
+
+@dataclass(frozen=True)
+class PartitionMapping:
+ """A resolved, usable partition filter mapping."""
+
+ partition_column: str
+ mapped_column: str
+ value_transform: str
+ is_monotonic: bool
+ #: Whether the engine compares the mapped column the way ``T`` was written
+ #: against. Resolved once where the column and the engine are both in hand,
+ #: rather than re-derived at each `mirrors` call.
+ equality_is_safe: bool = True
+
+ def mirrors(self, operator: FilterOperator) -> bool:
+ return operator in mirrorable_operators(
+ self.is_monotonic, equality_is_safe=self.equality_is_safe
+ )
+
+
+def contains_value_placeholder(transform: str | None) -> bool:
+ """Whether the transform contains the ``:value`` placeholder."""
+ return bool(transform) and VALUE_PLACEHOLDER_RE.search(transform or "") is
not None
+
+
+def contains_executable_comment(transform: str | None) -> bool:
+ """Whether the transform carries a MySQL/MariaDB executable comment.
+
+ See `EXECUTABLE_COMMENT_RE`. Answered on raw text, like
+ `contains_jinja` and for the same reason: the whole point is that no
+ parser this module can reach reports the construct at all.
+ """
+ return bool(transform) and EXECUTABLE_COMMENT_RE.search(transform or "")
is not None
+
+
+def contains_jinja(transform: str | None) -> bool:
+ """Whether the transform contains a balanced Jinja block."""
+ return bool(transform) and JINJA_BLOCK_RE.search(transform or "") is not
None
+
+
+def parse_skeleton(transform: str, standin: str = _PARSE_STANDIN) -> str:
+ """
+ The transform with ``:value`` substituted out, ready for a SQL parser.
+
+ ``sanitize_clause`` / sqlglot choke on a bare ``:value`` on most dialects,
+ so the placeholder is swapped for a benign literal first -- the same trick
+ ``validate_stored_expression`` uses for Jinja blocks.
+
+ :param standin: what to substitute. `placeholder_is_executable` passes
+ `_PLACEHOLDER_STANDIN` to find out where the placeholder landed.
+ """
+ # Substituted through a function so `standin` is taken literally. As a
+ # replacement *string* `re.sub` would read backslashes in it as escapes,
+ # and `build_probe_sql` passes rendered SQL here: a filter value containing
+ # `\n` would probe a newline instead of the two characters the predicate
+ # compares, and one containing `\1` would raise outright.
+ return VALUE_PLACEHOLDER_RE.sub(lambda _: standin, transform)
+
+
+#: Prefix the transform is wrapped in before parsing. Its length is subtracted
+#: from any reported column so positions refer to what the owner actually
typed.
+_SELECT_PREFIX = "SELECT "
+
+
+def _parse_skeleton(
+ transform: str, engine: str, standin: str = _PARSE_STANDIN
+) -> SQLStatement | None:
+ """
+ Parse ``SELECT <transform>`` with the placeholder substituted out.
+
+ Returns ``None`` when the transform does not parse.
+ """
+ try:
+ return SQLStatement(
+ f"{_SELECT_PREFIX}{parse_skeleton(transform, standin)}", engine
+ )
+ except SupersetParseError:
+ return None
+
+
+def parse_error_detail(transform: str, engine: str) -> str | None:
+ """
+ Where the parser gave up on the transform.
+
+ Returns ``None`` when it parses, or when the parser offered no position.
+ Note that sqlglot parses unknown functions happily -- a misspelled function
+ name is not a parse error, it is an engine error, and surfaces only when
the
+ transform is evaluated.
+
+ The parser's own ``highlight`` is deliberately dropped: it would name the
+ ``NULL`` we substituted for ``:value``, which is not a token the owner
typed.
+ """
+ try:
+ SQLStatement(f"{_SELECT_PREFIX}{parse_skeleton(transform)}", engine)
+ except SupersetParseError as ex:
+ column = (ex.error.extra or {}).get("column")
+ if not isinstance(column, int):
+ return None
+ return str(
+ _(
+ "syntax error at position %(position)d.",
+ position=_position_in_transform(transform, column),
+ )
+ )
+ return None
+
+
+def _position_in_transform(transform: str, parsed_column: int) -> int:
+ """
+ Map a column in the parsed skeleton back to the transform as typed.
+
+ Two substitutions stand between them: the ``SELECT`` prefix, and every
+ ``:value`` that became a shorter ``NULL``. Without unwinding both, a
+ reported position drifts left by two characters per placeholder ahead of it
+ -- which is worst exactly where transforms usually break, at the end.
+ """
+ position = max(parsed_column - len(_SELECT_PREFIX), 0)
+ shift = len(":value") - len(_PARSE_STANDIN)
+ preceding = sum(
+ 1
+ for index, match in enumerate(VALUE_PLACEHOLDER_RE.finditer(transform))
+ if match.start() - index * shift < position
+ )
+ return position + preceding * shift
+
+
+def placeholder_is_executable(transform: str | None, engine: str) -> bool:
+ """
+ Whether every ``:value`` sits where the engine will evaluate it.
+
+ `contains_value_placeholder` is a regex over raw text, and `parse_skeleton`
+ substitutes before parsing, so neither notices a placeholder that is not in
+ an executable position. Two shapes reach the probe that way, and both are
+ worse than the inactive mapping a malformed transform earns:
+
+ ``':value'`` -- the placeholder inside a string literal. `build_probe_sql`
+ binds it anyway, because SQLAlchemy's own ``text()`` scan is no more
+ literal-aware than the regex, and the dialect then renders the bind
+ *including its own quotes* in the middle of the owner's quotes. A value of
+ ``" || (SELECT secret FROM vault LIMIT 1) || "`` closes the literal and
+ compiles to a working sub-query -- the very read primitive
+ `stored_expression_error` refuses when it is written openly.
+
+ ``1 -- :value`` -- the placeholder inside a comment. The probe's selections
+ are joined on one line, so the comment swallows its own ``AS v0`` alias and
+ everything after it; a single-value probe still returns one column, the
+ constant is accepted as the transform's result, and the mirror then emits
+ ``partition_col = <constant>`` for every filter value. That is wrong rows,
+ not lost pruning.
+
+ Answered structurally: substitute an identifier rather than a keyword and
+ count the column references it produced. A literal holds no column
+ reference and a comment is not parsed at all, so either case comes back
+ short of the number of placeholders the owner wrote.
+
+ Which is why a transform naming the stand-in itself is refused outright.
The
+ count is the whole gate, and a hand-written occurrence pays the difference:
+ ``concat(superset_pfm_value_standin, ':value')`` has one placeholder,
inside
+ a literal that contributes nothing, and one bare reference the owner wrote
--
+ so the counts match and the gate passes a placeholder the engine will never
+ evaluate. The probe then renders the bound value inside the owner's quotes,
+ where a value closing the literal can alias a column of any table the
+ connection can read into the stand-in's name and have the probe hand it
back
+ through the preview. There is no legitimate transform naming this
identifier,
+ so refusing it costs nothing and makes the count mean what it claims.
+ """
+ if not transform:
+ return False
+ if _PLACEHOLDER_STANDIN in transform.lower():
+ # Lowercased on both sides because `count_bare_column_references` is
+ # case-insensitive too: a dialect may normalize an unquoted identifier,
+ # so `SUPERSET_PFM_VALUE_STANDIN` would otherwise balance the count
+ # while slipping past a case-sensitive check here.
+ return False
+ expected = len(VALUE_PLACEHOLDER_RE.findall(transform))
+ if not expected:
+ return False
+ statement = _parse_skeleton(transform, engine, _PLACEHOLDER_STANDIN)
+ if statement is None:
+ return False
+ return statement.count_bare_column_references(_PLACEHOLDER_STANDIN) ==
expected
+
+
+def is_parseable(transform: str | None, engine: str) -> bool:
+ """
+ Whether the transform parses as a single select expression.
+
+ "Single" is the load-bearing word. `SELECT lower(:value), 'x'` parses just
+ as happily as `SELECT lower(:value)`, but it returns two columns per input
+ value, and the probe reads one column per input -- so an `IN` filter would
+ get a predicate built from the wrong halves of the wrong rows. Rejecting
the
+ list here is what lets the probe trust its own column count.
+ """
+ if not transform or not transform.strip():
+ return False
+ statement = _parse_skeleton(transform, engine)
+ return statement is not None and statement.count_select_expressions() == 1
+
+
+def is_bare_expression(transform: str | None, engine: str) -> bool:
+ """
+ Whether the transform is a scalar expression and nothing more.
+
+ `is_parseable` is not enough, because it counts only the projection.
+ ``secret || :value FROM vault`` holds exactly one select expression, so it
+ parses as "a single expression" -- and `build_probe_sql` then emits
+ ``SELECT secret || 'us' AS v0``, where its own alias is read as a table
+ alias on the FROM clause the transform smuggled in. The probe reads a
+ column the owner was never granted and hands it back through the predicate
+ the preview panel renders.
+
+ A transform is a scalar function of ``:value`` by definition -- the whole
+ feature rests on ``partition_col = T(mapped_col)``, which only type-checks
+ for scalar ``T`` -- so there is no legitimate transform with a clause of
+ its own, and none with a sub-query either.
+ """
+ if not transform or not transform.strip():
+ return False
+ statement = _parse_skeleton(transform, engine)
+ return (
+ statement is not None
+ and statement.is_bare_select_expression()
+ and not statement.has_subquery()
+ )
+
+
+def is_unfinished(transform: str | None, engine: str) -> bool:
+ """
+ Whether the transform is not SQL yet, as opposed to the wrong SQL.
+
+ The distinction matters because the two deserve opposite treatment. A
+ half-typed ``unix_timestamp(:value`` is what a text input produces on the
+ way to something valid: a PUT stores it and reports the mapping inactive,
+ so discarding it would mean an export could not round-trip the dataset it
+ came from. ``unix_timestamp(:value); DROP TABLE t`` is not on the way to
+ anything, and has to be refused wherever it is offered.
+
+ Both fail `is_parseable`, and both fail to parse as a single *statement* --
+ so neither of those tells them apart. Parsing as a *script* does: the
+ multi-statement form is two valid statements, while unfinished text is no
+ statement at all.
+ """
+ if not transform or not transform.strip():
+ return False
+ try:
+ SQLScript(f"{_SELECT_PREFIX}{parse_skeleton(transform)}", engine)
+ except SupersetParseError:
+ return True
+ return False
+
+
+def find_non_deterministic_functions(transform: str, engine: str) -> set[str]:
+ """
+ Names of non-deterministic functions the transform calls.
+
+ ``UNIX_TIMESTAMP`` is only reported in its zero-argument form, which means
+ "now" on Hive and Impala; the one-argument form is the canonical temporal
+ transform and stays allowed.
+ """
+ statement = _parse_skeleton(transform, engine)
+ if statement is None:
+ return set()
+
+ found = {
+ name
+ for name in NON_DETERMINISTIC_FUNCTIONS
+ if statement.check_functions_present({name})
+ }
+ return found | _find_niladic_calls(statement)
+
+
+def _find_niladic_calls(statement: SQLStatement) -> set[str]:
+ """
+ Names from ``NON_DETERMINISTIC_WHEN_NILADIC`` called with no arguments.
+
+ Note some dialects resolve the zero-argument form themselves -- Hive parses
+ ``unix_timestamp()`` straight to ``CURRENT_TIMESTAMP`` -- in which case the
+ name-based check above has already caught it. This is the backstop for the
+ dialects that do not.
+ """
+ return NON_DETERMINISTIC_WHEN_NILADIC & statement.get_niladic_functions()
+
+
+def resolve_partition_mapping(datasource: SqlaTable) -> PartitionMapping |
None:
+ """
+ Resolve the dataset's mapping, or ``None`` when nothing may be mirrored.
+
+ Every bail-out here is defensive as well as functional: save-time
validation
+ rejects most of these, but rows predating the validation can still violate
+ the invariants, and a column sync can invalidate a mapping that was fine
+ when it was written.
+
+ The transform gate is `is_transform_active`, the same function the Explore
+ indicator reads, which is `validate_transform` with the messages discarded.
+ Anything narrower here would be a second, weaker statement of the same
rule:
+ a transform calling `now()` that reached storage without passing
+ `UpdateDatasetCommand` -- through import, or through a bundle written by
hand
+ -- would be reported inactive by the editor and still mirrored by this
+ function, freezing a snapshot of probe time into the predicate with nothing
+ on screen to say so.
+ """
+ if not feature_flag_manager.is_feature_enabled(FEATURE_FLAG):
+ return None
+
+ partition_column = getattr(datasource, "partition_column", None)
+ if not partition_column:
+ return None
+
+ columns_by_name = {column.column_name: column for column in
datasource.columns}
+ if partition_column not in columns_by_name:
+ # The partition column was dropped by a column sync or at the source.
+ return None
+
+ mapped_column_name = (
+ getattr(datasource, "partition_mapped_column", None) or
datasource.main_dttm_col
+ )
+ if not mapped_column_name or mapped_column_name not in columns_by_name:
+ return None
+
+ if mapped_column_name == partition_column:
+ # Self-mapping: the mirrored predicate would duplicate the original.
+ return None
+
+ mapped_column = columns_by_name[mapped_column_name]
+ transform = getattr(mapped_column, "partition_value_transform", None)
+ if not is_transform_active(transform, datasource.database.backend):
+ return None
+
+ if has_active_advanced_data_type(mapped_column):
+ # `translate_filter` builds its own predicate shape from *translated*
+ # values, so the `(operator, value)` pair the operator matrix reasons
+ # about does not exist and mirroring would apply the wrong values.
+ return None
+
+ return PartitionMapping(
+ partition_column=str(partition_column),
+ mapped_column=str(mapped_column_name),
+ value_transform=cast(str, transform),
+ is_monotonic=bool(
+ getattr(mapped_column, "partition_transform_is_monotonic", False)
+ ),
+ equality_is_safe=equality_mirrors_safely(
+ mapped_column, datasource.database.db_engine_spec
+ ),
+ )
+
+
+def has_active_advanced_data_type(column: TableColumn) -> bool:
+ """
+ Whether the column's advanced data type is configured and switched on.
+
+ Such a column is never mirrored: ``translate_filter`` builds its own
+ predicate shape from translated values, so the ``(operator, value)`` pair
+ the operator matrix reasons about does not exist.
+ """
+ advanced_data_type = getattr(column, "advanced_data_type", None)
+ if not advanced_data_type:
+ return False
+ if not
feature_flag_manager.is_feature_enabled("ENABLE_ADVANCED_DATA_TYPES"):
+ return False
+ return advanced_data_type in app.config.get("ADVANCED_DATA_TYPES", {})
+
+
+def _denylist_engine_key(database: "Database") -> str:
+ """
+ The name ``DISALLOWED_SQL_*`` is keyed by for this database.
+
+ The engine spec's own ``engine`` first, because that is what every other
+ denylist gate uses (`_raise_for_disallowed_sql`, `sql_lab`) and what an
+ operator writing the config reads in the documentation; a spec covering
+ several SQLAlchemy backends through ``engine_aliases`` reports the one
+ canonical name under which its entry is written.
+
+ Falling back to the URL's backend, because resolving the spec loads the
+ SQLAlchemy dialect entrypoint, which imports the driver package. On a
+ deployment missing an optional driver that raises rather than merely being
+ absent, asking for the spec here would turn a dataset import into a hard
+ failure -- and for every engine without aliases the two names are the same
+ string anyway.
+ """
+ try:
+ return database.db_engine_spec.engine
+ except Exception: # pylint: disable=broad-except # noqa: BLE001
+ return database.backend
+
+
+def stored_expression_error( # noqa: C901
+ database: "Database",
+ catalog: str | None,
+ schema: str | None,
+ transform: str,
+) -> str | None:
+ """
+ Why this transform may not be stored or run, if there is a reason.
+
+ Stricter than the policy a general stored expression goes through, because
+ a value transform is a narrower thing: `build_probe_sql` binds only
+ `:value` and splices the rest of the transform in as SQL text, which the
+ engine then executes, so an ungated transform is arbitrary SQL. A dataset
+ editor without SQL Lab could store `(SELECT secret FROM protected_table
+ LIMIT 1) || :value`, or `secret || :value FROM protected_table`, and read
+ the answer back out of the emitted predicate the preview panel renders.
+
+ So the transform must be a bare scalar expression: no clause of its own and
+ no sub-query, whatever `ALLOW_ADHOC_SUBQUERY` says. That is not a
+ restriction the feature pays for -- it rests on
+ ``partition_col = T(mapped_col)``, which only type-checks for scalar ``T``
+ -- and it is what makes the table denylist and the RLS rewrite moot here,
+ since neither has a table reference left to govern.
+
+ And it must be a *read*. Shape is a separate question from effect: a bare
+ scalar expression can still write, so `SQLStatement.is_mutating` is asked
+ too -- the same question SQL Lab and the chart executor ask before letting
+ SQL run. See the gate itself for what that reaches and what it does not.
+
+ Returns the engine-agnostic reason as a string rather than raising,
+ because its four callers disagree about what to do with it: the preview
+ reports it at the field, a PUT and the legacy datasource save refuse the
+ write, the importer drops the transform and keeps the dataset, and the
+ probe simply declines to run. Raising would make three of those four
+ write a `try` around a question.
+
+ Imported inside the function rather than at module scope:
+ `connectors.sqla.models` imports this module, so the dependency only runs
+ one way at import time.
+ """
+ from superset.connectors.sqla.models import ( # pylint:
disable=import-outside-toplevel,cyclic-import
+ validate_stored_expression,
+ )
+
+ # Ahead of the parse, both because it is the cheaper question and because
+ # the parse is the cost the bound exists to contain.
+ if len(transform) > MAX_TRANSFORM_LENGTH:
+ return str(
+ _(
+ "A partition value transform cannot be longer than "
+ "%(limit)d characters.",
+ limit=MAX_TRANSFORM_LENGTH,
+ )
+ )
+
+ # Also ahead of the parse, and for a stronger reason than cost: every gate
+ # below reasons about the parse tree, and this is the one construct that
+ # makes the parse tree describe a different statement from the one the
+ # engine runs. Checked here as well as in `validate_transform` because this
+ # is the door a row written by an earlier release still comes through.
+ if contains_executable_comment(transform):
+ return str(
+ _(
+ "A partition value transform cannot contain a MySQL "
+ "executable comment, which the database executes and every "
+ "validator reads as a comment."
+ )
+ )
+
+ statement = _parse_skeleton(transform, database.backend)
+ if statement is None:
+ return str(
+ _("A partition value transform must parse as a single SQL
expression.")
+ )
+
+ # The shape gate again, even though `validate_transform` blocks the same
+ # thing at save time. This is the last door before the transform becomes
SQL
+ # an engine runs, and the only door a row written by an earlier release --
+ # or by an importer that ran with the feature flag off -- still passes
+ # through.
+ if not statement.is_bare_select_expression():
+ return str(
+ _(
+ "A partition value transform must be a single SQL expression, "
+ "with no FROM, WHERE or other clause."
+ )
+ )
+
+ # Unconditionally, not under `ALLOW_ADHOC_SUBQUERY`. A transform is a
scalar
+ # function of `:value`, so a sub-query in one has no legitimate use, and it
+ # is a read primitive whose answer the preview panel renders back to its
+ # caller. Refusing it is also what lets this function skip the table
+ # denylist and the row-level-security rewrite: with no FROM clause and no
+ # sub-query there is no table reference for either to govern.
+ if statement.has_subquery():
+ return str(_("A partition value transform cannot contain a
sub-query."))
+
+ # The read-only gate, which nothing above implies. The shape gates ask what
+ # the transform *selects*; this asks what evaluating it *does*, and a bare
+ # scalar expression can still write. `nextval(:value)` on PostgreSQL
+ # advances a sequence: one function call, no clause, no sub-query, and not
+ # in the default `DISALLOWED_SQL_FUNCTIONS` either -- so every gate above
+ # passes it and the probe then runs it. `setval` is the same shape, and
+ # `exp.Execute` and the opaque-`exp.Command` forms arrive the same way.
+ #
+ # The large-object writers (`lo_export`, `lowrite`, `lo_from_bytea`, ...)
+ # are caught here too, and the shipped denylist happens to name them as
+ # well -- but that is config an operator can replace, while this is not.
+ #
+ # `is_mutating` is the same question SQL Lab, `get_virtual_table_metadata`
+ # and the chart executor ask before letting SQL run, so a transform is held
+ # to the bar the rest of Superset already sets rather than to one invented
+ # here. Unconditional rather than gated on `Database.allow_dml`: a
transform
+ # is a scalar function of `:value` by definition, so there is no legitimate
+ # mutating one, and the probe runs on a cache schedule nobody chose -- a
+ # write would repeat every time the entry expired.
+ #
+ # It bears saying what this does not reach. A user-defined function whose
+ # body writes is a plain call no parser can tell from `lower(:value)`, and
+ # `is_mutating`'s function-name walk is PostgreSQL-only by design, since
the
+ # same names are read-only elsewhere. Neither residual is specific to this
+ # feature -- a dataset editor can already have the engine evaluate an
+ # arbitrary expression through a calculated column or a virtual dataset's
+ # SQL -- and closing them means authorizing the preview against the
+ # database, not parsing harder.
+ if statement.is_mutating():
Review Comment:
`is_mutating()` only recognises function calls, and the comment above
promises that no mutating transform gets through. On Oracle,
`orders_seq.NEXTVAL + :value` parses as a column reference, so it passes every
gate; with a numeric mapped column, a numeric partition column and sample `1`
the preview probe runs `SELECT orders_seq.NEXTVAL + 1 FROM DUAL` and advances
the sequence, again on every cache expiry, for a dataset editor with no SQL Lab
or DML grant. Could pseudo-columns such as `NEXTVAL` be refused here or added
to the denylist for Oracle?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
