rebenitez1802 commented on code in PR #45557:
URL: https://github.com/apache/superset/pull/45557#discussion_r4378874502
##########
superset/connectors/sqla/partition_mapping.py:
##########
@@ -3385,6 +3397,65 @@ def _reads_as_temporal(value: str, db_engine_spec:
type[BaseEngineSpec]) -> bool
return False
+def _temporal_literal_sql(partition_column: TableColumn, value: date) -> str |
None:
+ """
+ A probed date or timestamp as the engine's own literal for the column.
+
+ The probe answers a temporal transform with a `date` or a `datetime`, and
+ SQLAlchemy's literal processor writes either one as plain quoted text --
+ ``part_date >= '2026-01-01'`` -- whatever the dialect. Trino and Presto
+ have no implicit coercion from ``varchar`` to ``date`` or ``timestamp``, so
+ they refuse that comparison outright, and because the mirror is ``AND``-ed
+ onto the chart query the whole chart fails rather than its pruning.
+
+ `BaseEngineSpec.convert_dttm` is how every other temporal bound in a chart
+ query is written (see `ExploreMixin.dttm_sql_literal`), so the mirror
+ writes its values the same way: ``DATE '2026-01-01'`` on Trino,
+ ``CAST('2026-01-01' AS DATE)`` on Hive.
+
+ `None` where that cannot be done faithfully, and the caller declines:
+
+ * The engine has no literal for the column's type. An engine Superset has
+ no rendering for is one it has not confirmed, the same answer
+ `_reads_as_temporal` gives for text.
+ * The literal throws part of the value away -- SQLite's, for one, stops at
+ the second. A bound floored past what the probe answered is a mirror
+ narrower than the filter, which drops rows. Detected the way
+ `ExploreMixin._engine_literal_resolution` detects it, by rendering a
+ second instant and comparing the text: a value one microsecond earlier
+ writes differently from this one exactly when nothing of it is lost.
+ A column holding only days is exempt, because flooring to the day is
+ the column's own resolution and every comparison against it reads the
+ same either way.
+ """
+ instant = (
+ value if isinstance(value, datetime) else datetime.combine(value,
time.min)
+ )
+ spec = partition_column.db_engine_spec
+ column_type = partition_column.type or ""
+ db_extra = partition_column.db_extra
+ rendered = spec.convert_dttm(column_type, instant, db_extra=db_extra)
+ if rendered is None:
+ return None
+ holds_days_only = isinstance(spec.get_sqla_column_type(column_type),
sa.Date)
+ if not holds_days_only and rendered == spec.convert_dttm(
+ column_type, instant - timedelta(microseconds=1), db_extra=db_extra
+ ):
+ return None
+ return rendered
Review Comment:
🟡 Medium — A `datetime.min` probe answer fails the chart instead of skipping
the mirror
`instant - timedelta(microseconds=1)` raises `OverflowError` when the probe
answers `datetime.min` or `date.min` for a column that isn't a DATE. That's
reachable: psycopg2 reads Postgres `'-infinity'` as `datetime(1, 1, 1)`.
Nothing upstream catches it, so the chart query fails and the partition-mapping
preview returns 500; before this commit, Postgres mirrored that value as
`part_ts >= '0001-01-01 00:00:00'` without error. Catching the overflow skips
the mirror instead, and the preview shows the type message. The same lines are
still unguarded on #43870 (`partition_mapping.py:3712-3716`), and this
replacement applies there as-is. Worth adding a regression test for
`datetime.min` on a TIMESTAMP key.
```suggestion
if holds_days_only:
return rendered
try:
earlier = instant - timedelta(microseconds=1)
except OverflowError:
# `datetime.min` (psycopg2's '-infinity') has no earlier instant to
# compare against, so the literal's precision cannot be confirmed.
return None
if rendered == spec.convert_dttm(column_type, earlier,
db_extra=db_extra):
return None
return rendered
```
--
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]
