aminghadersohi commented on code in PR #43870:
URL: https://github.com/apache/superset/pull/43870#discussion_r4254203475
##########
docs/static/resources/openapi.json:
##########
@@ -4253,6 +4411,7 @@
"type": "object"
},
"DashboardRestApi.get_list": {
+ "additionalProperties": false,
Review Comment:
This regeneration adds `"additionalProperties": false` to ~380 schemas and
turns nullable `allOf` into `anyOf`; CI's `superset update-api-docs` emits
neither, so `check-openapi-spec-drift` and
`test_dashboard_list_uses_generated_response_schema` fail on all 4 DB legs. No
suggestion: generated file.
##########
superset/connectors/sqla/partition_mapping.py:
##########
@@ -0,0 +1,3041 @@
+# 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")
+
+#: The opening and closing delimiters of each kind of Jinja block. The probe
+#: would render these in a different context at a different time from the chart
+#: query, so a transform holding one is rejected at save time.
+JINJA_BLOCK_DELIMITERS = (("{{", "}}"), ("{%", "%}"), ("{#", "#}"))
+
+#: 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?!")
+
+#: A PostgreSQL Unicode-escaped identifier, ``U&"..."``.
+#:
+#: The second construct that breaks the premise above, and in the same way:
+#: ``U&"\\0071uery_to_xml"(...)`` parses as an anonymous function whose name is
+#: the *literal* ``\\0071uery_to_xml``, so the function denylist is matching a
+#: name the engine never sees -- PostgreSQL decodes the escape and runs
+#: ``query_to_xml``, handing a dataset owner rows from outside their RLS scope
+#: through the preview's ``emitted_predicate``.
+#:
+#: Normalizing the escapes and re-checking would be the other repair, and is
+#: worse: it means reimplementing one engine's identifier decoding well enough
+#: to be a security boundary. Refusing the construct is exact, and costs
+#: nothing real -- a transform is a scalar function of ``:value``, and there is
+#: no reason to name a function or column through an escape sequence.
+#:
+#: Refused on every engine, like the executable comment, so a transform stored
+#: on SQLite cannot detonate when the dataset is re-pointed at PostgreSQL. The
+#: ``&`` may be separated from the ``U`` by nothing at all, but whitespace or a
+#: comment between the prefix and the string is not accepted by PostgreSQL
+#: either, so the strict form is the whole surface.
+#:
+#: Matched in raw text, so the prefix is refused inside a string literal too.
+#: That is the same trade the executable comment makes and has the same
+#: justification: a gate whose contract is to fire *before* the parse cannot
+#: ask the parse whether the text was quoted. ``concat(:value, 'u&"x"')`` is
+#: refused, which costs an owner nothing anyone writes.
+UNICODE_ESCAPED_IDENTIFIER_RE = re.compile(r'\bU&"', re.IGNORECASE)
+
+#: 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_unicode_escaped_identifier(transform: str | None) -> bool:
+ """Whether the transform names something through a ``U&"..."`` escape.
+
+ See `UNICODE_ESCAPED_IDENTIFIER_RE`. Answered on raw text for the same
+ reason as the comment above: the parse reports a name the engine does not
+ use, which is exactly what makes the denylist blind to it.
+ """
+ return (
+ bool(transform)
+ and UNICODE_ESCAPED_IDENTIFIER_RE.search(transform or "") is not None
+ )
+
+
+#: Sequence pseudo-columns, which advance a sequence when they are *read*.
+#:
+#: ``orders_seq.NEXTVAL`` parses as a qualified column reference, not a
+#: function call, so neither `SQLStatement.is_mutating` -- which walks function
+#: names -- nor the function denylist can see it. The probe would then run
+#: ``SELECT orders_seq.NEXTVAL + 1 FROM DUAL`` and advance the sequence, again
+#: on every cache expiry, for a dataset editor holding neither SQL Lab nor a
+#: DML grant. Reading one is a write however the parser classifies it.
+#:
+#: The function spelling (``nextval('s')`` on PostgreSQL) is a different shape
+#: and is caught as a function; this is only about the suffix form.
+SEQUENCE_PSEUDO_COLUMNS = frozenset({"nextval", "currval"})
+
+
+def references_sequence_pseudo_column(statement: SQLStatement) -> bool:
+ """Whether the statement reads a sequence pseudo-column.
+
+ See `SEQUENCE_PSEUDO_COLUMNS`. Answered on the parse rather than on raw
+ text, because the name has to be in column position for this to be the
+ construct: a column or alias merely *called* ``nextval`` is not it.
+ """
+ return bool(statement.get_column_names() & SEQUENCE_PSEUDO_COLUMNS)
Review Comment:
The function spellings still pass: MySQL/MariaDB `NEXTVAL(s)`/`SETVAL(s,
1)`, DuckDB `nextval('s')` and MSSQL `NEXT VALUE FOR s` all get `None` from
`stored_expression_error`, because `is_mutating` walks function names only on
PostgreSQL.
```suggestion
return bool(
statement.get_column_names() & SEQUENCE_PSEUDO_COLUMNS
or statement.get_disallowed_functions({"nextval", "setval",
"next_value_for"})
)
```
##########
superset/config.py:
##########
@@ -2569,6 +2601,29 @@ def engine_context_manager( # pylint:
disable=unused-argument
"getMacro",
"getSetting",
},
+ # Oracle functions that run a query handed to them as text. Same blind spot
+ # as PostgreSQL's XML and full-text families above: the statement they
+ # execute is a string, so the expression has no FROM clause and no
+ # sub-query of its own for a table-reference gate to see, and the rows come
+ # back without the dataset grants or row-level security the caller is
+ # subject to.
+ #
+ # Named bare rather than package-qualified: `DBMS_XMLGEN.GETXML(...)`
+ # parses as an anonymous function called `GETXML`, so the package prefix
+ # never reaches the matcher. `getclobval` and `extract` are deliberately
+ # absent -- they read an XMLType the caller already has rather than
+ # building one from a query.
+ "oracle": {
+ "getxml",
+ "getxmltype",
+ "newcontext",
+ "newcontextfromhierarchy",
+ # Reads a table by path string, e.g.
+ # ``DBURITYPE('/HR/EMPLOYEES/ROW[ID=1]/SALARY').getclob()``, which is
+ # the same blind spot as the above with no function argument to
+ # inspect -- and ``PUBLIC`` holds execute on it by default.
+ "dburitype",
Review Comment:
`URIFACTORY.GETURI('/oradb/HR/EMPLOYEES/ROW[ID=1]/SALARY').getclob()` builds
the same DBURIType through the factory and passes the gate, as do the sibling
types `XDBURITYPE` and `HTTPURITYPE`.
```suggestion
"dburitype",
# The same reader through its factory, and its sibling URI types.
"geturi",
"xdburitype",
"httpuritype",
```
--
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]
