Add an optional mysql-timezone input for the database container - #53
Open
snake14 wants to merge 1 commit into
Open
Add an optional mysql-timezone input for the database container#53snake14 wants to merge 1 commit into
snake14 wants to merge 1 commit into
Conversation
Defaults to UTC, so existing callers are unaffected. A non-UTC value reproduces the condition where SQL reading dates out of Matomo's UTC-stored DATETIME columns returns a shifted result, which CI cannot currently reach.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds an optional
mysql-timezoneinput for the database container. It defaults toUTC, which is what CI has always run, so every existing caller is unaffected and nothing runs differently until someone opts in.Matomo stores datetimes in UTC, but SQL that reads a date back out of those columns can depend on the database server's timezone — Matomo never pins it, so it is whatever the server defaults to. Because CI is UTC, that whole class of bug passes here and only appears for users whose database server is not on UTC.
That is not theoretical. Cohorts derived each visitor's cohort via
UNIX_TIMESTAMP(log_visit.visit_first_action_time), which reads a UTC-storedDATETIMEin the session's zone, so every cohort was shifted by that offset — for years, with CI green throughout. It was found by accident on a developer machine inPacific/Auckland(fix: innocraft/plugin-Cohorts#134).The input is passed as
TZto the container rather than applied viaSET GLOBAL time_zone, so the server reports the zone through@@system_time_zonewhile@@global.time_zonestaysSYSTEM— how such a server is really configured, and a path that a numeric offset set from inside a test never exercises.Verified against
mysql:5.7, the default image:@@system_time_zone@@global.time_zoneNOW()vsUTC_TIMESTAMP()TZ=UTC)UTCSYSTEMTZ=Pacific/AucklandNZSTSYSTEMDeliberately not wired into any caller or matrix here. A timezone axis would double every plugin's
PluginTestsmatrix, and turning it on broadly would surface real but unrelated failures in whichever PR happened to trigger it. The suggested next step is one manual or scheduled run over core and a few plugins to measure the blast radius first.Checklist