mistercrunch commented on code in PR #39344:
URL: https://github.com/apache/superset/pull/39344#discussion_r3197088958


##########
superset/models/helpers.py:
##########
@@ -1881,10 +1881,20 @@ def join_offset_dfs(
             time_grain
         )
 
-        if join_column_producer and not time_grain:
-            raise QueryObjectValidationError(
-                _("Time Grain must be specified when using Time Shift.")
+        if not time_grain:
+            has_relative_offset = any(
+                not (
+                    self.is_valid_date_range(offset)
+                    and feature_flag_manager.is_feature_enabled(
+                        "DATE_RANGE_TIMESHIFTS_ENABLED"
+                    )
+                )
+                for offset in offset_dfs
             )
+            if has_relative_offset:

Review Comment:
   Addressed in commit c08706e4 — validation now checks whether any join key is 
temporal before requiring time grain, so non-timeseries comparisons (e.g., 
table chart grouped by text dimension) work without time grain.



##########
tests/unit_tests/utils/test_core.py:
##########
@@ -1688,3 +1690,10 @@ def test_sanitize_url_blocks_dangerous():
     """Test that dangerous URL schemes are blocked."""
     assert sanitize_url("javascript:alert('xss')") == ""
     assert sanitize_url("data:text/html,<script>alert(1)</script>") == ""
+
+
+def test_extract_dataframe_dtypes_with_duplicate_columns():

Review Comment:
   Added return type annotation in b89364531f.



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