SQL type FLOAT should not be used for economical data #15994
Conversation
tsteur
added
the
Pull Request WIP
tsteur
added
the
not-in-changelog
tsteur
added
Needs Review
|
@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. |
|
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. 🤔 |
|
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. |
|
I thought #12575 was because float can't represent exact decimal numbers. Wouldn't that still be an issue with double? https://dev.mysql.com/doc/refman/5.7/en/fixed-point-types.html |
|
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 |
fix #12575
Similar to #15408 not using
DECIMALas it would change the output like this: https://travis-ci.org/github/matomo-org/matomo/jobs/691567334#L714-L742 . Tested this in 025410dInstead using
doublewhich should at least not round the big numbers like it used to in #14934 (comment)Just like in #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.