rusackas commented on code in PR #34513:
URL: https://github.com/apache/superset/pull/34513#discussion_r2801764939


##########
superset/db_engine_specs/postgres.py:
##########
@@ -489,8 +490,30 @@ class PostgresEngineSpec(BasicParametersMixin, 
PostgresBaseEngineSpec):
             ENUM(),
             GenericDataType.STRING,
         ),
+        (
+            re.compile(r"^interval", re.IGNORECASE),
+            INTERVAL(),
+            GenericDataType.NUMERIC,
+        ),
     )
 
+    # PostgreSQL INTERVAL values need normalization for chart rendering.
+    # 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"). String representations pass through unchanged.
+    column_type_mutators: dict[types.TypeEngine, Callable[[Any], Any]] = {
+        INTERVAL: lambda v: (
+            v.total_seconds() * 1000
+            if hasattr(v, "total_seconds")
+            else float(v) * 1000
+            if isinstance(v, (int, float))
+            else 0
+            if v is None
+            else v  # Pass through string representations and other types
+        ),
+    }

Review Comment:
   Applied in fab40b6b69. Great catches on all three issues:
   
   1. **NULL semantics** - Now returns `None` instead of `0` to preserve 
correct NULL behavior
   2. **bool handling** - Added explicit `not isinstance(v, bool)` check since 
bool is a subclass of int
   3. **Mixed-type columns** - Unconvertible types now return `None` instead of 
passing through
   
   Also extracted the lambda to a named `_normalize_interval` method for better 
testability as suggested.
   
   Note: I skipped the `fetch_data` integration test because PostgreSQL's 
`get_datatype` uses psycopg2 type codes (integers) rather than string type 
names, making it difficult to mock without the actual psycopg2 type system. The 
unit tests provide good coverage of the mutator logic itself.



-- 
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]

Reply via email to