aminghadersohi commented on code in PR #43870:
URL: https://github.com/apache/superset/pull/43870#discussion_r4227028394
##########
superset/models/helpers.py:
##########
@@ -4292,36 +4389,661 @@ def dttm_sql_literal(self, dttm: datetime, col:
"TableColumn") -> str:
return f"""'{dttm.strftime("%Y-%m-%d %H:%M:%S.%f")}'"""
- def get_time_filter( # pylint: disable=too-many-arguments # noqa: C901
+ def _mirror_probe_value(self, dttm: datetime, col:
Optional["TableColumn"]) -> Any:
+ """
+ ``dttm`` in the mapped column's own stored representation.
+
+ The predicate this mirror stands in for compares the column against
+ `dttm_sql_literal(dttm, col)`, so the transform has to be probed with
+ the same value. On a column stored as an epoch integer, ``T`` is a
+ function of an integer: probing it with a `datetime` either returns
+ something unrelated to the partition keys or raises, and a raise costs
+ the dataset its pruning silently.
+
+ Only the ``python_date_format`` branches of `dttm_sql_literal` are
+ reproduced, because only those produce a *value*. `convert_dttm` yields
+ engine-specific SQL text, which cannot be bound as a parameter; its own
+ divergence from the probe is handled by widening the bounds instead
+ (`_engine_literal_resolution`).
+
+ In `dttm_sql_literal`'s own order, though: it asks ``convert_dttm``
+ first and reaches the format only when the engine answers nothing. A
+ column can carry both -- a TIMESTAMP with a ``%Y%m%d``
+ ``python_date_format``, or an entry in
+ ``python_date_format_by_column_name`` -- and applying the format
+ unconditionally probed a value the real predicate never compares.
+ ``event_time >= '2026-01-01 00:00:00'`` mirrored as
+ ``partition_col >= '20260101'``, and a January 15 row keyed
+ ``2026-01-15`` sorts below that string, so the chart lost a row the
+ filter keeps.
+
+ Where the engine does answer, the `datetime` is returned unchanged and
+ the widening path takes over -- which is exactly what already happens
+ for every temporal column that carries no format at all.
+ """
+ if col is None:
+ return dttm
+
+ if col.type and self.db_engine_spec.convert_dttm(
+ col.type, dttm, db_extra=self.db_extra
+ ):
+ # The format is unreachable in `dttm_sql_literal` for this column,
+ # so reproducing it here would describe a different bound.
+ return dttm
+
+ tf = col.python_date_format
+ if not tf and self.db_extra:
+ tf = self.db_extra.get("python_date_format_by_column_name",
{}).get(
+ col.column_name
+ )
+ if not tf:
+ return dttm
+
+ if tf in EPOCH_FORMATS:
+ dttm_tz_aware = dttm
+ if dttm_tz_aware.tzinfo is None:
+ dttm_tz_aware = dttm_tz_aware.replace(tzinfo=timezone.utc)
+ return int(dttm_tz_aware.timestamp()) * EPOCH_FORMATS[tf]
+ return dttm.strftime(tf)
+
+ def _engine_literal_resolution(
+ self, dttm: datetime, col: Optional["TableColumn"]
+ ) -> LiteralResolution:
+ """
+ How much of ``dttm`` survives into this engine's literal for ``col``.
+
+ The real predicate compares `dttm_sql_literal(dttm, col)`, so whatever
+ that literal throws away is a bound the engine never sees. A mirror
+ derived from the full precision then describes a *narrower* range than
+ the predicate it stands in for -- and narrower means it drops rows the
+ filter keeps.
+
+ Two things get thrown away in practice:
+
+ - the sub-second part. SQLite renders a timestamp with
+ ``timespec="seconds"``.
+ - the time of day entirely. Every engine spec with a ``types.Date``
+ branch renders ``dttm.date().isoformat()``, so Presto, Trino,
+ BigQuery and some two dozen others answer `DATE '2026-01-01'` for a
+ bound of 10:00 -- and `T(DATE '2026-01-01')` is below the mirror's
+ own lower bound, so the whole first day is dropped.
+
+ Detected rather than enumerated per engine, because the branch depends
+ on the column's type as well as the spec: instants differing only in
the
+ part under test render as the same text exactly when the engine has
+ thrown that part away.
+
+ The day test uses two fixed times of day rather than ``dttm`` and one
+ alternative, so a bound that happens to sit on the hour the alternative
+ uses cannot read as "no time at all". Neither probe sits at midnight on
+ purpose: SQLite renders a DATE column at exactly midnight as a bare
+ date, so a midnight probe would differ from its alternative for a
+ reason that has nothing to do with resolution.
+
+ Likewise both sub-second probes carry a non-zero remainder. Comparing
+ against ``dttm`` truncated to the second looks like the obvious test
and
+ is wrong for the same formatting-boundary reason.
+
+ A non-temporal column has no answer here, and asking anyway gets a
wrong
+ one. SQLite's ``convert_dttm`` renders for ``types.String`` as well as
+ for the date types, so a ``VARCHAR`` mapped column reads as
+ second-resolution -- and a filter value that happens to *look* like an
+ instant then has its bound rounded as though the engine had truncated
+ it. On a VARCHAR ``code`` mapped onto a VARCHAR ``p`` by ``:value``,
+ ``code < '2026-07-06T10:08:11.500000'`` was mirrored as
+ ``p <= '2026-07-06 10:08:12.000000'``; a space sorts before ``T``, so a
+ row holding ``2026-07-06T09:00:00`` fell outside the mirror while
+ matching the filter.
+
+ The guard lives here rather than in each caller so this and
+ `_column_literal_resolution` cannot disagree about which column is
+ coarse -- which is the same reason that helper delegates to this one.
+ """
+ if col is None or not col.type or not col.is_dttm:
Review Comment:
Keying on `is_dttm` also skips rounding for a `DATE` column not flagged
temporal: `get_time_filter` still renders a 10:00 TEMPORAL_RANGE bound as `DATE
'2026-01-01'`, but the probe now gets 10:00, so the mirror drops that first
day. The generic type keeps the VARCHAR fix:
```suggestion
if (
col is None
or not col.type
or col.type_generic != GenericDataType.TEMPORAL
):
```
##########
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: 00fab727cd0a
Review Comment:
`Revises:` still names the old parent.
```suggestion
Revises: e4a7c2b9d1f3
```
--
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]
