powa-team / powa-web

PoWA user interface
http://powa.readthedocs.io/
73 stars 31 forks source link

Rework queries #90

Closed anayrat closed 4 years ago

anayrat commented 4 years ago

This PR bring following changes:

WHERE tstzrange(:from, :to, '[]') @> (records).ts to WHERE (records).ts <@ tstzrange(:from, :to, '[]')

I think it is more clear, it is like using: WHERE col1 = 4 instead of WHERE 4 = col1

Before: https://demo-powa.anayrat.info/server/2/overview/?from=2019-11-26T19%3A46%3A50%2B01%3A00&to=2019-11-27T19%3A46%3A50%2B01%3A00

image

After:

https://dev-powa.anayrat.info/server/2/overview/?from=2019-11-26T19%3A45%3A46%2B01%3A00&to=2019-11-27T19%3A45%3A46%2B01%3A00

image

Note there is still an issue with "general overview": image

When you zoom on it: image

rjuju commented 4 years ago

For the remaining issue, did you had a chance to look at the data, to see if the query is returning wrong data or (unlikely but still) the further processing?

anayrat commented 4 years ago

For the remaining issue, did you had a chance to look at the data, to see if the query is returning wrong data or (unlikely but still) the further processing?

I am sure data are good in history tables. The issue is in the query returning data and if I remember, I think the problem is in the processing.

rjuju commented 4 years ago

Hi @anayrat, do you have any news on this?

anayrat commented 4 years ago

This PR solve the first issue. As I said (https://github.com/powa-team/powa-web/pull/90#discussion_r352672915) I just wonder if we should put explanation here : https://github.com/powa-team/powa/blob/master/docs/components/powa-archivist/development.rst

For the second issue, I spend some time on it but it is more complicated. I think, it should be addressed in another issue in order to not block this PR.

marco44 commented 4 years ago

Mmm first objection is against yoda conditions... I don't care, it just felt more natural for me writing it this way, but if I'm the only one I don't care, everything being immutable in SQL :)

"Remove an unecessary UNION ALL and computation with min_ts and max_ts : this solved display issue with database object tab. I don't know only this tab was impacted." Ok it seems to solve the issue, but do you have an explanation why ? It's been a while since I had a look at this query :)

anayrat commented 4 years ago

"Remove an unecessary UNION ALL and computation with min_ts and max_ts : this solved display issue with database object tab. I don't know only this tab was impacted." Ok it seems to solve the issue, but do you have an explanation why ? It's been a while since I had a look at this query :)

Sorry for the delay, it took me a while to dig again :) So the main issue is in this part:

https://github.com/powa-team/powa-web/pull/90/files#diff-829d1062b24654bedfbb2ff563b6c034L837-L855

Especially in theses clauses: coalesce_range @> min_ts and coalesce_range @> max_ts. We where looking only the ranges containing the min_ts and max_ts but not between them. So I think we were lucky with other tabs, the range contained both min_ts and max_ts interval, but not with this tab.

I will open a new ticket for tst issue I mentioned at the beginning of this ticket.

anayrat commented 4 years ago

So this PR can be merged?

rjuju commented 4 years ago

Sorry, I totally forgot about this PR. Thanks for fixing (and bumping), as this is a highly wanted fix :)