Skip to content
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

Should we limit key values to varchar(50)? #41

Open
MichaelRoosz opened this issue Apr 17, 2022 · 4 comments
Open

Should we limit key values to varchar(50)? #41

MichaelRoosz opened this issue Apr 17, 2022 · 4 comments

Comments

@MichaelRoosz
Copy link
Contributor

When running
core:convert-to-utf8mb4
I got for table log_visit:
[Zend_Db_Adapter_Exception] SQLSTATE[42000]: Syntax error or access violation: 1118 Row size too large. The maximum row size for the used table type, not counting BLOBs, is 65535. This includes storage overhead, check the manual. You have to change some columns to TEXT or BLOBs

My setup has 20 custom variables and 15 custom dimensions in scope visit.
To fix this, I decided to convert some of the custom_var_k* columns to varchar(100) as 100 chars for the key still seem to be long enough.

I think this change would make sense for 99% of matomo users and would give an easier upgrade path for big Matomo 3.x setups

@MichaelRoosz MichaelRoosz changed the title Should we limit key values to varchar(100)? Should we limit key values to varchar(50)? Apr 17, 2022
@MichaelRoosz
Copy link
Contributor Author

MichaelRoosz commented Apr 17, 2022

Update: the situation is even worse in log_conversion, so I changed the custom_var_k* columns now to varchar(50) which still fells big enough. My log_conversion table currently has 295 millions rows and only 27 of those used a custom_var_k* value with length > 50, and all of those were because of corrupted / wrong data.

@sgiehl
Copy link
Member

sgiehl commented Apr 25, 2022

HI @MichaelRoosz Thanks for creating this issue. Having so many custom variables and custom dimensions might not be a typical set up. At least I have not yet heard about the row size issue. I guess it's hard to predict how long the columns need to be. Some users might not need 50 chars, some might even exceed the 200. Guess we should at least document the issue / behavior, so others are able to troubleshoot. Besides that the best solution that comes to my mind would be to have it configurable how long those columns should be. Maybe even for each custom variable that is newly created. We could that display that in the admin UI. A quick solution would be a config value, that is applied at least for newly created variables maybe.

@MichaelRoosz
Copy link
Contributor Author

MichaelRoosz commented Apr 25, 2022

Hello @sgiehl , sounds good!
Maybe let us start with the quick and easy solution by making https://github.com/matomo-org/plugin-CustomVariables/blob/4.x-dev/CustomVariables.php#L65 get the size for the name and value columns from a config.ini.php setting?

For default values we could keep the curent name/200 value/200 limits, our we could reduce the name column a bit, maybe name/50, value/200? What do you prefer?

Also, because the max 200 char limit is enforced also in other libraries, I think it would be good to add some documentation that going above 200 may not work.

@sgiehl
Copy link
Member

sgiehl commented Apr 25, 2022

@MichaelRoosz Feel free to create a simple PR for that. The config.ini.php setting should only be used if it is available and use 200 as default otherwise. As this is a plugin, we can't add the default to global.ini.php in core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants