Fix segment update scripts #17532
Fix segment update scripts #17532
Conversation
sgiehl
added
Critical
| foreach ($segments as $segment) { | ||
| $hash = md5(urldecode($segment['definition'])); | ||
| $migrations[] = $this->migration->db->sql("UPDATE `$segmentTable` SET `hash` = '$hash' WHERE `idsegment` = '{$segment['idsegment']}'"); | ||
| } |
There was a problem hiding this comment.
Removing this one, as otherwise all segments might be updated twice when updating from a version prior 4.3.0-b3
|
@sgiehl not fully sure I understand the problem. Updating it twice be fine I suppose as it's quick etc and this way people that copy/paste the queries and execute them manually could do this (although it should then be executed again anyway hopefully). Curious though how came it failed cause the hash column should have been added before the update query was executed. Was this not the case? Just checking in case there's a bug somewhere |
|
@tsteur the hash column was selected outside of a migration, so the migration that added the column would get executed after the query that used it was executed. This was the best fix I think. |
|
This would explain this part https://github.com/matomo-org/matomo/pull/17532/files#diff-c21583e394fbddcd7c201f880290f2b9756fbfbcce383765db917b728a06fde0L39 I think 👍 but maybe not this part? https://github.com/matomo-org/matomo/pull/17532/files#diff-acfc32e315ad85ad5c07ba2653c366883113e0a91043c2095d380a18fafaf5a7L40-L45 would have expected this to work? |
|
I assume it was just a waste to have both sets of queries since they do the same thing. |
Exactly |
|
In that case it was to show the updates it would run and then make sure it will be executed just in case either step will be missed in which case it could lead to data deletion of reports that may not be possible to be recreated etc. I reckon it could be good to add it back just to be safe as it's a very critical update that if it goes wrong reports get deleted and then if raw data deletion is configured as well the reports are gone forever or they would need to invalidate data which isn't easy either and can take long time to rearchive data. |
|
Still don't get why it would be better to execute the same sql queries twice. I have removed the one in the earlier update script. So even if someone already was on |
Description:
Updating Matomo might currently fail, as the update script for 4.3.0-b4 tries to use the
hashcolumn, which is created in update for 4.3.0-b3. As the migrations are all gathered before they are executed, the hash column does not exist and the query fails.This also happen for our updater UI test. See https://travis-ci.com/github/matomo-org/matomo/jobs/503575602#L1700
Review