rusackas commented on PR #42082: URL: https://github.com/apache/superset/pull/42082#issuecomment-5006008573
@Abdulrehman-PIAIC80387 I'm not sure this change actually does anything, but would appreciate you checking my math here. Before the fix, `inner_from_dttm` was set to `query_object_clone.from_dttm` right after `from_dttm` itself was reassigned to the shifted value, so `inner_from_dttm == from_dttm` already. Every consumer I can find (the `inner_from_dttm or from_dttm` fallback in `helpers.py`, and the `inner_from_dttm is None or (inner_from_dttm == from_dttm and ...)` short-circuit in `semantic_layers/mapper.py`) treats that case identically to `inner_from_dttm` being `None`. So flipping the assignment to `None` shouldn't change any generated SQL versus what runs today. Did you diff the generated SQL or query object before/after for the `365 days ago` + `month` grain repro, or did this come from matching the pattern in the date-range branch? If I'm right, `test_relative_offset_leaves_inner_dttm_none` wouldn't have caught it either, since it only regexes the source text of `processing_time_offsets` rather than running it and checking the actual output. Maybe we can just add tests that cover all the bases to prove me right or wrong... holler if I can help with that. -- 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]
