diff --git a/superset/db_engine_specs/postgres.py b/superset/db_engine_specs/postgres.py index 51eb8b8f129..1d7e374ef89 100644 --- a/superset/db_engine_specs/postgres.py +++ b/superset/db_engine_specs/postgres.py @@ -136,6 +136,34 @@ def parse_options(connect_args: dict[str, Any]) -> dict[str, str]: return {token[0]: token[1] for token in tokens} +def _normalize_interval(v: Any) -> Optional[float]: + """Convert PostgreSQL INTERVAL values to milliseconds. + + psycopg2 and psycopg3 always return INTERVAL values as datetime.timedelta + objects. We convert to milliseconds so users can apply the built-in + "DURATION" number format for human-readable display (e.g., + "1d 2h 30m 45s") and so the values participate cleanly in numeric + aggregations in bar/pie charts. + + Returns None for the NULL case (preserves NULL semantics) and for any + unexpected non-timedelta type (avoids producing a mixed-type column + when an unfamiliar driver surfaces something other than timedelta). + """ + if v is None: + return None + if hasattr(v, "total_seconds"): + return v.total_seconds() * 1000 + # Defensive: psycopg2/3 should always hand us a timedelta. If a future + # driver doesn't, surface the surprise in the logs rather than silently + # dropping the value so operators can diagnose it. + logger.warning( + "Cannot normalize PostgreSQL INTERVAL value of type %s to numeric; " + "returning None.", + type(v).__name__, + ) + return None + + class PostgresBaseEngineSpec(BaseEngineSpec): """Abstract class for Postgres 'like' databases""" @@ -534,28 +562,8 @@ class PostgresEngineSpec(BasicParametersMixin, PostgresBaseEngineSpec): ), ) - @staticmethod - def _normalize_interval(v: Any) -> Any: - """Convert PostgreSQL INTERVAL values to milliseconds. - - psycopg2 returns timedelta objects which we convert to milliseconds for - numeric operations in bar/pie charts. Using milliseconds allows users to - apply the built-in "DURATION" number format for human-readable display - (e.g., "1d 2h 30m 45s"). - - Returns None for values that cannot be converted to preserve NULL semantics - and avoid mixed-type columns. - """ - if v is None: - return None - if hasattr(v, "total_seconds"): - return v.total_seconds() * 1000 - if isinstance(v, (int, float)) and not isinstance(v, bool): - return float(v) * 1000 - return None # Can't convert to numeric — treat as missing - column_type_mutators: dict[types.TypeEngine, Callable[[Any], Any]] = { - INTERVAL: _normalize_interval.__func__, # type: ignore[attr-defined] + INTERVAL: _normalize_interval, } @classmethod diff --git a/tests/unit_tests/db_engine_specs/test_postgres.py b/tests/unit_tests/db_engine_specs/test_postgres.py index d65159694d7..e2dbb1b4f9c 100644 --- a/tests/unit_tests/db_engine_specs/test_postgres.py +++ b/tests/unit_tests/db_engine_specs/test_postgres.py @@ -376,30 +376,27 @@ def test_interval_type_mutator() -> None: """ mutator = spec.column_type_mutators[INTERVAL] - # Test timedelta conversion (most common case from psycopg2) - # Result is in milliseconds for compatibility with DURATION formatter + # Timedelta conversion — the only path psycopg2/psycopg3 actually + # exercises. Result is in milliseconds for compatibility with the + # DURATION formatter. td = timedelta(days=1, hours=2, minutes=30, seconds=45) - assert mutator(td) == 95445000.0 # Total ms: (1*86400 + 2*3600 + 30*60 + 45) * 1000 + assert mutator(td) == 95445000.0 # (1*86400 + 2*3600 + 30*60 + 45) * 1000 - # Test zero duration + # Zero duration assert mutator(timedelta(0)) == 0.0 - # Test negative interval + # Negative interval assert mutator(timedelta(days=-1)) == -86400000.0 - # Test numeric values (assumed to be seconds) are converted to milliseconds - assert mutator(12345) == 12345000.0 - assert mutator(123.45) == 123450.0 - - # Test None preserves NULL semantics (not converted to 0) + # None preserves NULL semantics (not converted to 0) assert mutator(None) is None - # Test bool is not treated as numeric (bool is subclass of int in Python) - assert mutator(True) is None - assert mutator(False) is None - - # Test unconvertible types return None to avoid mixed-type columns + # Unexpected non-timedelta types fall through to the defensive + # `return None` (and emit a warning) rather than producing a + # mixed-type column. assert mutator("1 day 02:30:45") is None assert mutator("P1DT2H30M45S") is None + assert mutator(12345) is None + assert mutator(True) is None assert mutator([1, 2, 3]) is None assert mutator({"days": 1}) is None