villebro commented on a change in pull request #15279:
URL: https://github.com/apache/superset/pull/15279#discussion_r677228313
##########
File path: superset/utils/core.py
##########
@@ -115,6 +115,8 @@
DTTM_ALIAS = "__timestamp"
+TIME_COMPARISION = "__"
Review comment:
> I have some reservation on running multiple queries for each period
after offset----it basically means an additional 1x query time for each new
offset---plus the overheads of transferring potentially duplicate rows with
overlapping periods.
TL;DR: if we want to maintain feature parity with the current time offset
functionality in `viz.py` (which issues multiple queries), we need to do that
here, too.
@ktmud This was my initial reaction, too, and I assumed we could just add
the offsets by based on the initial dataframe. However, after studying how this
currently works, I noticed that we need to issue separate queries for each
offset, as they will retrieve data for different time ranges. Take this
screenshot from @zhaoyongjie 's comment above:
Here you can see that the "1 year ago" offset in year 1980 is in fact the
data for 1979, which isn't visible in the original series. If we wanted to
support arbitrary offsets based on just one query response, we would need to
know the maximum offset, query based on that, and then truncate the original
series to it's original time ranges etc.
--
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]
