aminghadersohi commented on code in PR #43870: URL: https://github.com/apache/superset/pull/43870#discussion_r4174200783
########## superset/connectors/sqla/partition_mapping.py: ########## @@ -0,0 +1,1314 @@ +# 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 (...)`` always -- ``T`` is a function +``col >=|>|<|<= v``, ``TEMPORAL_RANGE`` only if ``T`` is monotonic +``col != v``, ``NOT IN``, ``LIKE``, ... never +=========================================== ============================= + +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 re +from dataclasses import dataclass +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 SQLStatement +from superset.utils import json +from superset.utils.core import FilterOperator + +if TYPE_CHECKING: + from superset.connectors.sqla.models import SqlaTable, TableColumn + 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. +#: Every entry point enforces it -- the typed column field, the import schema +#: and the preview request -- 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. +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) + +#: 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" + +#: 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} + + +def mirrorable_operators(is_monotonic: bool) -> set[FilterOperator]: + """ + The operators whose predicates may be mirrored onto the partition column. + + :param is_monotonic: whether the owner declared the transform + order-preserving + """ + if is_monotonic: + return MIRRORABLE_ALWAYS | MIRRORABLE_IF_MONOTONIC + return set(MIRRORABLE_ALWAYS) + + +#: 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 + + def mirrors(self, operator: FilterOperator) -> bool: + return operator in mirrorable_operators(self.is_monotonic) + + +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_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) -> 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. + """ + return VALUE_PLACEHOLDER_RE.sub(_PARSE_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) -> 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)}", 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 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 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) + ), + ) + + +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 stored_expression_error( + 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. + + The same subquery, function-denylist and RLS policy every other stored + expression goes through, applied to a value transform. It matters more + here than the name suggests: `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` and read the answer back out of the emitted predicate. + + 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, + ) + + try: + validate_stored_expression(database, catalog, schema, parse_skeleton(transform)) + except SupersetSecurityException as ex: + return str(ex.error.message) + except QueryClauseValidationException as ex: + return str(ex.message) + return None Review Comment: With `ALLOW_ADHOC_SUBQUERY` on, a sub-query passes here but the RLS-injected SQL is discarded, so the probe reads RLS-hidden rows: preview of `(SELECT secret FROM vault …) || :value` emitted `part = 'zz-HIDDEN-by-RLS_x'`. `DISALLOWED_SQL_FUNCTIONS` isn't checked either. ```suggestion statement = _parse_skeleton(transform, database.backend) if statement is not None and statement.has_subquery(): # Allowed under ALLOW_ADHOC_SUBQUERY, but the probe never injects RLS. return str(_("A partition value transform cannot contain a sub-query.")) denied = app.config["DISALLOWED_SQL_FUNCTIONS"].get(database.backend, set()) if statement is not None and denied and statement.check_functions_present(denied): return str(_("A partition value transform cannot call a disallowed function.")) return None ``` ########## superset/connectors/sqla/partition_mapping.py: ########## @@ -0,0 +1,1278 @@ +# 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 (...)`` always -- ``T`` is a function +``col >=|>|<|<= v``, ``TEMPORAL_RANGE`` only if ``T`` is monotonic +``col != v``, ``NOT IN``, ``LIKE``, ... never +=========================================== ============================= + +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 re +from dataclasses import dataclass +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 SQLStatement +from superset.utils import json +from superset.utils.core import FilterOperator + +if TYPE_CHECKING: + from superset.connectors.sqla.models import SqlaTable, TableColumn + 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. +#: Every entry point enforces it -- the typed column field, the import schema +#: and the preview request -- 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. +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) + +#: 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" + +#: 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} + + +def mirrorable_operators(is_monotonic: bool) -> set[FilterOperator]: + """ + The operators whose predicates may be mirrored onto the partition column. + + :param is_monotonic: whether the owner declared the transform + order-preserving + """ + if is_monotonic: + return MIRRORABLE_ALWAYS | MIRRORABLE_IF_MONOTONIC + return set(MIRRORABLE_ALWAYS) + + +#: 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 + + def mirrors(self, operator: FilterOperator) -> bool: + return operator in mirrorable_operators(self.is_monotonic) + + +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_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) -> 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. + """ + return VALUE_PLACEHOLDER_RE.sub(_PARSE_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) -> 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)}", 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 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 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): Review Comment: Fixed at `115c194f58` for the default config: import drops it regardless of the flag and the probe declines it (removing either gate fails 6 import / 3 probe tests). The `ALLOW_ADHOC_SUBQUERY` case is a new comment on `stored_expression_error`. ########## tests/integration_tests/datasets/api_tests.py: ########## @@ -802,6 +802,7 @@ def test_info_security_dataset(self): "can_get_or_create_dataset", "can_warm_up_cache", "can_get_drill_info", + "can_partition_mapping_preview", Review Comment: Fixed in `69517c2e4d`; every Python leg is green at `115c194f58`. ########## superset-frontend/src/components/Datasource/components/DatasourceEditor/components/PartitionFilterMapping/PartitionColumnFields.tsx: ########## @@ -0,0 +1,274 @@ +/** + * 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. + */ +import { useMemo } from 'react'; +import { t } from '@apache-superset/core/translation'; +import { css, useTheme } from '@apache-superset/core/theme'; +import { Alert } from '@apache-superset/core/components'; +import { + Button, + Flex, + Icons, + InfoTooltip, + Label, + Select, + Tooltip, + Typography, +} from '@superset-ui/core/components'; +import { + mappedColumnIsImplicit, + mappingIsActive, + partitionMappingErrors, + resolveMappedColumn, + suggestedMappedColumn, +} from './utils'; +import type { + PartitionMappingColumn, + PartitionMappingDatasource, + PartitionMappingIssue, +} from './types'; + +interface PartitionColumnFieldsProps { + datasource: PartitionMappingDatasource; + columns: PartitionMappingColumn[]; + onPartitionColumnChange: (columnName: string | null) => void; + /** Open the given column's row expand in the Columns table. */ + onNavigateToColumn: (columnName: string) => void; +} + +/** + * "Partition column" and the computed "Maps to partition" (wireframes 1a, 1g). + * + * "Maps to partition" is deliberately read-only. It reflects + * `partition_mapped_column ?? main_dttm_col`, and a peer dropdown would let it + * drift from the default datetime column silently. The only way to override it + * is from the target column's own row, where a transform has to be supplied + * alongside. + */ +export default function PartitionColumnFields({ + datasource, + columns, + onPartitionColumnChange, + onNavigateToColumn, +}: PartitionColumnFieldsProps) { + const theme = useTheme(); + + const options = useMemo( + () => + columns.map(column => ({ + value: column.column_name, + label: column.column_name, + customLabel: ( + <Flex align="center" gap={theme.sizeUnit}> + <span>{column.column_name}</span> + {column.type && <Label>{column.type}</Label>} + </Flex> + ), + })), + [columns, theme.sizeUnit], + ); + + const mappedColumn = resolveMappedColumn(datasource); + const isImplicit = mappedColumnIsImplicit(datasource); + const isActive = mappingIsActive(datasource, columns); + const { partition_column: partitionColumn } = datasource; + + // `field` is what the issues carry it for: a message about the partition + // column belongs under the partition column, not only in the Save button's + // tooltip, where an owner has to guess which of the two selects is at fault. + const issues = useMemo( + () => partitionMappingErrors(datasource, columns), + [datasource, columns], + ); + const issueFor = (field: PartitionMappingIssue['field']) => + issues.find(issue => issue.field === field)?.message; + + // "Map a different column instead" normally opens the currently-mapped + // column's row, which is where the picker lives. When the mapped column *is* + // the partition column -- the self-mapping the backend rejects, and reachable + // in one click because the picker offers every column -- that row renders no + // mapping section at all, so the one guided way out of the broken state led + // nowhere. Offer a column that is not the partition column instead. + const differentColumnTarget = + mappedColumn && mappedColumn !== partitionColumn + ? mappedColumn + : (suggestedMappedColumn(columns, partitionColumn) ?? mappedColumn); + + return ( + <Flex vertical gap={theme.sizeUnit} data-test="partition-column-fields"> + <Flex align="center" gap={theme.sizeUnit}> + <Typography.Text>{t('Partition column')}</Typography.Text> + <InfoTooltip + tooltip={t( + 'The physical column the engine partitions on. Filters on the mapped column are mirrored onto it so the engine can prune partitions.', + )} + /> + </Flex> + <Select + ariaLabel={t('Partition column')} + options={options} + value={partitionColumn ?? undefined} + onChange={value => onPartitionColumnChange((value as string) ?? null)} + onClear={() => onPartitionColumnChange(null)} + placeholder={t('None')} + allowClear + data-test="partition-column-select" + /> + {issueFor('partition_column') && ( + <Typography.Text type="danger" data-test="partition-column-error"> + {issueFor('partition_column')} + </Typography.Text> + )} + <Typography.Text type="secondary"> + {t( + "Column used for partition pruning on this table. Selecting one hides it from Explore's dimension and filter pickers by default.", + )} + </Typography.Text> + + {partitionColumn && ( + <Flex vertical gap={theme.sizeUnit} data-test="maps-to-partition"> + <Flex align="center" gap={theme.sizeUnit}> + <Typography.Text type="secondary"> + {t('Maps to partition')} + </Typography.Text> + <InfoTooltip + tooltip={t( + 'The column whose filters are mirrored. It follows the default datetime column unless a different column holds the mapping.', + )} + /> + </Flex> + + {mappedColumn ? ( + <> + <Flex align="center" gap={theme.sizeUnit}> + <Label>{mappedColumn}</Label> + {isImplicit && ( + <> + <Typography.Text type="secondary"> + {t('Default datetime column')} + </Typography.Text> + <Tooltip + title={t( + 'Set from the default datetime column above, so re-pointing that column moves the mapping with it.', + )} + > + <Icons.LockOutlined + iconSize="s" + iconColor={theme.colorTextTertiary} + /> + </Tooltip> + </> + )} + </Flex> + {issueFor('partition_mapped_column') && ( + <Typography.Text + type="danger" + data-test="partition-mapped-column-error" + > + {issueFor('partition_mapped_column')} + </Typography.Text> + )} + <Typography.Text type="secondary"> + {t( + 'Filters on this column are mirrored onto the partition column.', + )}{' '} + <Button + buttonStyle="link" + onClick={() => onNavigateToColumn(differentColumnTarget)} Review Comment: Fixed in `2fc5ac8c89`; `lint-frontend` (oxlint + tsc) is green at `115c194f58`. ########## superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterOption/AdhocFilterOption.test.tsx: ########## @@ -108,3 +108,115 @@ test('should render the Popover on clicking the right caret', async () => { await userEvent.click(rightCaret); expect(screen.getByRole('tooltip')).toBeInTheDocument(); }); + +// A monotonic transform, so ranges mirror alongside equality. +const MONOTONIC_MAPPING = { + partition_column: 'dt_epoch', + mapped_column: 'value', + active: true, + is_monotonic: true, + mirrorable_operators: ['<', '<=', '==', '>', '>=', 'IN', 'TEMPORAL_RANGE'], +}; + +const withMapping = ( + mapping: Record<string, unknown> | null, + adhocFilter = simpleAdhocFilter, +) => + setup({ + ...mockedProps, + adhocFilter, + datasource: { partition_filter_mapping: mapping }, + }); + +const glyph = () => screen.queryByTestId('partition-pruning-indicator'); + +test('a filter on the mapped column carries the partition pruning glyph', () => { + // Wireframe 1d: the chart author never configured any of this and only sees + // an explanation of why the query got faster. + render(withMapping(MONOTONIC_MAPPING)); + + expect(glyph()).toBeInTheDocument(); +}); + +test('a filter on any other column carries no glyph', () => { + render( + withMapping({ ...MONOTONIC_MAPPING, mapped_column: 'some_other_column' }), + ); + + expect(glyph()).not.toBeInTheDocument(); +}); + +test('an inactive mapping leaves the chip alone', () => { + render(withMapping({ ...MONOTONIC_MAPPING, active: false })); + + expect(glyph()).not.toBeInTheDocument(); +}); + +test('a negated filter on the mapped column carries no glyph', () => { + // The query path never mirrors a negation -- the transform need not be + // injective, so `value != 10` would mirror to a predicate that drops rows the + // original filter keeps. Labelling it would promise a speed-up the SQL does + // not contain. + render( + withMapping( + MONOTONIC_MAPPING, + simpleAdhocFilter.duplicateWith({ operator: '!=' }), + ), + ); + + expect(glyph()).not.toBeInTheDocument(); +}); + +test('a range filter waits on the monotonicity declaration', () => { + render( + withMapping({ + ...MONOTONIC_MAPPING, + is_monotonic: false, + mirrorable_operators: ['==', 'IN'], + }), + ); + + expect(glyph()).not.toBeInTheDocument(); +}); + +test('an equality filter mirrors under a non-monotonic transform', () => { + render( + withMapping( + { + ...MONOTONIC_MAPPING, + is_monotonic: false, + mirrorable_operators: ['==', 'IN'], + }, + simpleAdhocFilter.duplicateWith({ operator: '==' }), + ), + ); + + expect(glyph()).toBeInTheDocument(); +}); + +test('a free-form SQL filter on the mapped column carries no glyph', () => { + // It lands in `extras.where`, where the backend sees no operator to mirror. + render( + withMapping( + MONOTONIC_MAPPING, + new AdhocFilter({ + expressionType: ExpressionTypes.Sql, + sqlExpression: 'value > 10', + clause: Clauses.Where, + }), + ), + ); + + expect(glyph()).not.toBeInTheDocument(); +}); + +test('a filter with no comparator yet carries no glyph', () => { + render( + withMapping( + MONOTONIC_MAPPING, + simpleAdhocFilter.duplicateWith({ comparator: '' }), + ), + ); + + expect(glyph()).not.toBeInTheDocument(); +}); Review Comment: Fixed in `ff786951f8`; all `sharded-jest-tests` shards are green at `115c194f58`. ########## superset-frontend/src/components/Datasource/components/DatasourceEditor/components/PartitionFilterMapping/utils.ts: ########## @@ -0,0 +1,510 @@ +/** + * 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. + */ +import { t } from '@apache-superset/core/translation'; +import type { + PartitionMappingColumn, + PartitionMappingDatasource, + PartitionMappingIssue, + PartitionRowState, +} from './types'; + +/** + * Jinja delimiters. + * + * A transform is evaluated in a different context and at a different time from + * the chart query, so a template would not render the same way -- the backend + * rejects one outright. Shared so the check that declines to preview a Jinja + * transform and the check that blocks saving it cannot drift apart. + */ +const JINJA_PATTERN = /\{\{|\{%|\{#/; + +/** + * The column whose filters are mirrored. + * + * `partition_mapped_column` is an explicit override; `null` means "follow the + * default datetime column", so re-pointing that column moves the mapping with + * it. Mirrors `SqlaTable.partition_filter_mapping_summary` on the backend. + */ +export function resolveMappedColumn( + datasource: PartitionMappingDatasource, +): string | null { + if (!datasource.partition_column) { + return null; + } + return datasource.partition_mapped_column || datasource.main_dttm_col || null; +} + +/** Whether the mapped column came from `main_dttm_col` rather than an override. */ +export function mappedColumnIsImplicit( + datasource: PartitionMappingDatasource, +): boolean { + return Boolean( + datasource.partition_column && + !datasource.partition_mapped_column && + datasource.main_dttm_col, + ); +} + +/** + * Whether the mapping will actually mirror anything. + * + * Drives the choice between the green "filters will automatically apply" + * message and the warning that nothing is mirrored yet, so "non-blank text in + * the box" is not a good enough test: a transform with no `:value` to + * substitute, or one carrying Jinja, is as inert as an empty one and the + * backend's own summary reports it inactive. Claiming otherwise tells an owner + * their queries are pruning when they are not. + * + * Shares `transformCanPreview`'s conditions rather than restating them, and is + * necessary rather than sufficient for the same reason: an unparseable + * expression or a non-deterministic function needs a SQL parser to spot, so the + * server stays the authority -- `valueTransformIssues` surfaces what it says at + * the transform field itself. + */ +export function mappingIsActive( + datasource: PartitionMappingDatasource, + columns: PartitionMappingColumn[], +): boolean { + const mappedColumnName = resolveMappedColumn(datasource); + if (!mappedColumnName || mappedColumnName === datasource.partition_column) { + return false; + } + const mappedColumn = columns.find( + column => column.column_name === mappedColumnName, + ); + return transformCanPreview( Review Comment: Fixed in `ff786951f8`; `lint-frontend` is green at `115c194f58`. ########## superset/migrations/versions/2026-09-23_00-01_a7f3c2e91d84_add_partition_filter_mapping.py: ########## @@ -0,0 +1,101 @@ +# 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. +"""add partition filter mapping + +Adds the four columns behind the ``PARTITION_FILTER_MAPPING`` feature: + +- ``tables.partition_column`` -- the physical column the engine partitions on +- ``tables.partition_mapped_column`` -- explicit override for the column whose + filters are mirrored; NULL means "follow ``main_dttm_col``" +- ``table_columns.partition_value_transform`` -- the ``:value`` expression +- ``table_columns.partition_transform_is_monotonic`` -- gates range operators + +The Continuum shadow tables get the same columns so dataset version history and +restore keep working. + +Revision ID: a7f3c2e91d84 +Revises: 95d8a99c822e +Create Date: 2026-08-31 22:30:00.000000 + +""" + +import sqlalchemy as sa + +from superset.migrations.shared.utils import add_columns, drop_columns + +# revision identifiers, used by Alembic. +revision = "a7f3c2e91d84" +down_revision = "95d8a99c822e" Review Comment: Fixed: `down_revision` is `884a2115ebd3`, still master's only head, and `enforce-single-migration-head` is green at `115c194f58`. -- 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]
