New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix event value is truncated #15408
Fix event value is truncated #15408
Conversation
FYI Using decimal it causes the values to appear like this: https://travis-ci.org/matomo-org/matomo/jobs/638207041#L5670 We can workaround this by using eg |
<nb_visits>0</nb_visits> | ||
<nb_actions>0</nb_actions> | ||
<nb_visits_converted>0</nb_visits_converted> | ||
<nb_visits>4</nb_visits> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It didn't work as the comparison custom_float = '345.678'
doesn't return any results when the field is a float. The number seems not to be stored accurate enough to be able to compare it correctly. When changing the column to DOUBLE
it gets converted to 345.6780090332031
.
Not sure if it would make sense to truncate some minor fraction digits after changing the type 🤔
Using a DOUBLE
the number is stored accurate enough to be compared correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we maybe additionally run a script like UPDATE log_link_visit_action SET custom_float = TRUNCATE(custom_float, 5)
after it was converted to double? That should throw away all fraction digits after the fifth one 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sgiehl from what I see mysql stores also new values like this.
and truncate seems to ensure there's always that many zeros too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sgiehl not sure I understand this fully? If we just use DOUBLE
then this shouldn't be needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we currently have a value like 345.678
and the field is converted to DOUBLE
, the new value will be something like 345.6780090332031
. That's the reason why I suggested to run something like a TRUNCATE, so we have a similar value as before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which MySQL version is that? This wasn't happening for me on MySQL 8. Or maybe I'm not checking correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mysql 5.7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, can reproduce it on 10.2-MariaDB
. I just created 3 rows with values 3, 4.1 and 4.12. After converting to float I got this:
After executing your suggested query I got this:
So it's still changing the values quite a bit.
It seems there might not be a safe way to change the field? I wonder if it's worth it or maybe we rather create an FAQ for people needing more accuracy and only try to change it for new installs.
@mattab there doesn't seem to be an easy way to change the field for this column. I reckon we could really rather only change it for new installs maybe and create an FAQ for all other cases. any thoughts?
Doesn't According to https://dev.mysql.com/doc/refman/8.0/en/floating-point-types.html it uses 8 instead of 4 bytes. But nevertheless, the numbers will get stored more accurate... |
That was compared to the decimal alternative, not compared to float. |
@sgiehl as discussed changing the type only for new installs and created FAQ for existing installs https://matomo.org/faq/how-to/how-do-i-fix-a-truncated-event-value/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now. Seems all failing tests are because the branch is a bit outdated...
fix #14934
should require 2 bytes for the right part, and about 7 byes for the left part.