@tsteur opened this Pull Request on May 27th 2020 Member

fix https://github.com/matomo-org/matomo/issues/12575

Similar to https://github.com/matomo-org/matomo/pull/15408 not using DECIMAL as it would change the output like this: https://travis-ci.org/github/matomo-org/matomo/jobs/691567334#L714-L742 . Tested this in https://github.com/matomo-org/matomo/commit/025410dd66fb52a5ff62d19d35e1ffb24bea1d01

Instead using double which should at least not round the big numbers like it used to in https://github.com/matomo-org/matomo/issues/14934#issue-501165270

Just like in https://github.com/matomo-org/matomo/pull/15408 only doing this for new installs.

For existing installs created this FAQ https://matomo.org/faq/how-to/how-do-i-fix-a-truncated-or-rounded-revenue-or-price-value/

For the log_conversion.revenue* dimensions I could change the type directly in the dimension as it would have caused an update for existing users. I therefore moved it directly to the MySQL schema so it's good for new installs but doesn't affect existing users. Since the plugin cannot be uninstalled I can't think of a benefit of having it in the dimension directly.

@tsteur commented on May 27th 2020 Member

@sgiehl not sure it's worth it? Most installs might already have a value set in case they use the feature and even if we could migrate it, not too many people will run into this issue? Would avoid several full table scans on this columns.

@sgiehl commented on May 28th 2020 Member

Was just a thought. Btw. do we need to add those columns to an (old) update script? If someone updates from an very old version, where those columns didn't exist yet, the updater won't do that automatically anymore, as the dimensions don't trigger that anymore. 🤔

@tsteur commented on May 28th 2020 Member

Good point @sgiehl . Didn't think of this. Added logic for this. Tested it locally when all columns were present and when some were missing. Worked for me.

@robocoder commented on June 2nd 2020 Contributor

I thought #12575 was because float can't represent exact decimal numbers. Wouldn't that still be an issue with double?


@tsteur commented on June 2nd 2020 Member

Hi @robocoder yes it would be still an issue indeed. Ideally would use decimal. The problem was it changes the output in some reports like instead of 5.23 it was returning 5.2300 eg if it is like decimal(16,4). We could obviously work around this in PHP but figured the benefit of decimal might not be worth it? Haven't tested how segments would behave. Eg whether a segment value of 5.23 would also match 5.2300 etc.

This Pull Request was closed on June 2nd 2020
Powered by GitHub Issue Mirror