rusackas commented on PR #38617:
URL: https://github.com/apache/superset/pull/38617#issuecomment-5039604909

   Thanks @nitishagar, catching up on this since my June comment.
   
   The DATE_FORMAT approach @betodealmeida suggested got folded in (commit 
f09424e). Nice bonus that it also fixes the SECOND/MINUTE truncation bug, not 
just HOUR. The subquery-proxy test coverage CodeAnt asked for back in March is 
there too, `test_compile_timegrain_expression_preserves_date_truncation` now 
compiles through the subquery path and checks the truncation survives.
   
   Two things still open though. Copilot's note on the `is` identity check in 
`test_starrocks.py` hasn't been addressed, asserting on the rendered HOUR 
expression instead of object identity would be more robust. And the docstring 
on `test_time_grain_expressions_inherit_mysql` still says "DATE() preserved" 
but the assertion is now checking a `DATE_FORMAT(...)` string with no `DATE()` 
in it at all, just a leftover from before the pivot.
   
   (CodeAnt's untyped-lambda nit on that same file is pre-existing style, not 
new here, wouldn't block on that one.)
   
   Once those two are cleared up I think this is good to go. Thanks for 
sticking with it!


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