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


##########
superset/db_engine_specs/base.py:
##########
@@ -1110,6 +1110,14 @@ def get_timestamp_expr(
             time_expr = time_expr.replace("{col}", cls.epoch_to_dttm())
         elif pdf == "epoch_ms":
             time_expr = time_expr.replace("{col}", cls.epoch_ms_to_dttm())
+        elif pdf == "%Y":
+            # a bare four-digit year (e.g. the `year` column on the 
`video_game_sales`
+            # example dataset) has no native date type to lean on; without 
this the
+            # column value is passed straight into the grain function below, 
which
+            # every engine interprets as something other than a calendar year 
(SQLite
+            # reads a bare integer as a Julian day number, for instance), 
silently
+            # producing NULL for every row.
+            time_expr = time_expr.replace("{col}", cls.year_to_dttm())

Review Comment:
   Good catch, fixed. Wrapped the `year_to_dttm()` call in a try/except, so 
engines that have not implemented it fall through to the prior behavior instead 
of raising.



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