New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
modify SegmentQueryDecorator so extra comment information can be saved #17337
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@flamisz I tried this locally and I see the site, period, segment hash, etc. on some queries, I don't see it on every one. Eg here are some I saw:
// no period/site
'SELECT /*+ MAX_EXECUTION_TIME(7200000) */ /* Core */ /* trigger = CronArchive */ count(distinct log_visit.idvisitor) AS `1`, count(distinct log_visit.user_id) AS `39` FROM matomo_log_visit AS log_visit WHERE log_visit.visit_last_action_time >= ? AND log_visit.visit_last_action_time <= ? AND log_visit.idsite IN (?)
// has period/site
SELECT /*+ MAX_EXECUTION_TIME(7200000) */ /* 2015-05-11,2015-05-18 sites 2 segmenthash 90a5a511e1974bca37613b6daec137ba */ /* Core */ /* trigger = CronArchive */ count(distinct log_visit.idvisitor) AS `1`, count(distinct log_visit.user_id) AS `39` FROM matomo_logtmpsegmenta997edd368ac688e7aef25686690a7fc AS logtmpsegmenta997edd368ac688e7aef25686690a7fc INNER JOIN matomo_log_visit AS log_visit ON log_visit.idvisit = logtmpsegmenta997edd368ac688e7aef25686690a7fc.idvisit
The first one's probably not super important, but I also don't see it in MediaAnalytics queries:
SELECT /* MediaAnalytics */ /* trigger = CronArchive */ log_media.resource as label, count(log_media.idvisit) as nb_impressions, count(distinct log_media.idvisitor) as nb_unique_visitors_impressions FROM matomo_log_media AS log_media WHERE log_media.server_time >= ? AND log_media.server_time <= ? AND log_media.idsite IN (?) AND log_media.media_type = 1 GROUP BY label ORDER BY nb_impressions
SELECT /* MediaAnalytics */ /* trigger = CronArchive */ log_media.resource as label, count(log_media.idvisit) as nb_plays, count(distinct log_media.idvisitor) as nb_unique_visitors_plays, sum(if(log_media.media_length > 2 AND log_media.media_progress >= (log_media.media_length - 2), 1, 0)) as nb_finishes, sum(log_media.time_to_initial_play) as sum_time_to_play, sum(if(log_media.time_to_initial_play is null, 0, 1)) as nb_plays_with_tip, sum(log_media.watched_time) as sum_time_watched, sum(log_media.media_progress) as sum_time_progress, sum(log_media.media_length) as sum_media_length, sum(if(log_media.media_length > 0, 1, 0)) as nb_plays_with_ml, sum(log_media.fullscreen) as sum_fullscreen_plays FROM matomo_log_media AS log_media WHERE log_media.server_time >= ? AND log_media.server_time <= ? AND log_media.idsite IN (?) AND log_media.watched_time > 1 AND log_media.media_type = 1 GROUP BY label ORDER BY nb_plays SELECT /* MediaAnalytics */ /* trigger = CronArchive */ log_media.resource as parentLabel, log_media.watched_time as label, count(log_media.watched_time) as nb_plays FROM matomo_log_media AS log_media WHERE log_media.server_time >= ? AND log_media.server_time <= ? AND log_media.idsite IN (?) AND log_media.watched_time > 1 AND log_media.media_type = 1 GROUP BY log_media.resource, log_media.watched_time
I'm guessing other premium plugins might also not have this header. It would definitely be helpful to see this header on the more compute intensive archiving queries.
hi @diosmosis, can you please check now. I updated the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the code looks good to me, and it works. some tests are now failing though, once they're fixed it'll be good to merge.
Description:
fixes #17330
fixes #17327
Review