@sgiehl opened this Pull Request on May 7th 2021 Member

Description:

Updating Matomo might currently fail, as the update script for 4.3.0-b4 tries to use the hash column, 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

  • [ ] Functional review done
  • [ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@tsteur commented on May 9th 2021 Member

@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

@diosmosis commented on May 9th 2021 Member

@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.

@tsteur commented on May 10th 2021 Member
@diosmosis commented on May 10th 2021 Member

I assume it was just a waste to have both sets of queries since they do the same thing.

@sgiehl commented on May 10th 2021 Member

I assume it was just a waste to have both sets of queries since they do the same thing.

Exactly

@tsteur commented on May 10th 2021 Member

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.

@sgiehl commented on May 11th 2021 Member

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 4.3.0-b3, the updates will be performed again if needed 🤷

This Pull Request was closed on May 7th 2021
Powered by GitHub Issue Mirror