Skip to content

Commit

Permalink
Unique keys have a 767 max byte length as well. Thanks Phil!
Browse files Browse the repository at this point in the history
  • Loading branch information
@wilsonge
wilsonge committed Feb 16, 2016
1 parent bf7cdea commit fcb6d90
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
4 changes: 2 additions & 2 deletions administrator/components/com_admin/sql/updates/mysql/3.5.0-2015-07-01.sql
View file
Expand Up @@ -5,9 +5,9 @@ ALTER TABLE `#__menu` DROP KEY `idx_client_id_parent_id_alias_language`, ADD UNI

ALTER TABLE `#__redirect_links` DROP KEY `idx_link_old`, ADD UNIQUE KEY `idx_link_old` (`old_url`(100));

ALTER TABLE `#__session` MODIFY `session_id` varchar(400) NOT NULL DEFAULT '';
ALTER TABLE `#__session` MODIFY `session_id` varchar(191) NOT NULL DEFAULT '';

ALTER TABLE `#__user_keys` MODIFY `series` varchar(400) NOT NULL;
ALTER TABLE `#__user_keys` MODIFY `series` varchar(191) NOT NULL;

-- Convert all tables to UTF-8 Multibyte (utf8mb4)
ALTER TABLE `#__assets` CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;
Expand Down
4 changes: 2 additions & 2 deletions installation/sql/mysql/joomla.sql
View file
Expand Up @@ -1551,7 +1551,7 @@ CREATE TABLE IF NOT EXISTS `#__schemas` (
--

CREATE TABLE IF NOT EXISTS `#__session` (
`session_id` varchar(400) NOT NULL DEFAULT '',
`session_id` varchar(191) NOT NULL DEFAULT '',
`client_id` tinyint(3) unsigned NOT NULL DEFAULT 0,
`guest` tinyint(4) unsigned DEFAULT 1,
`time` varchar(14) DEFAULT '',
Expand Down Expand Up @@ -1885,7 +1885,7 @@ CREATE TABLE IF NOT EXISTS `#__user_keys` (
`id` int(10) unsigned NOT NULL AUTO_INCREMENT,
`user_id` varchar(255) NOT NULL,
`token` varchar(255) NOT NULL,
`series` varchar(400) NOT NULL,
`series` varchar(191) NOT NULL,
`invalid` tinyint(4) NOT NULL,
`time` varchar(200) NOT NULL,
`uastring` varchar(255) NOT NULL,
Expand Down

9 comments on commit fcb6d90

@zero-24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wilsonge

Is there a reason to do this only for that two points?

There are much more places see:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_admin/sql/updates/mysql/3.5.0-2015-07-01.sql#L82-L89

and https://raw.githubusercontent.com/joomla/joomla-cms/staging/installation/sql/mysql/joomla.sql

search for varchar(400). (8 places)

@wilsonge
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

varchar(400) is fine (and indeed for alias') required to avoid truncation. UNLESS there is a index (in which case it is limited to 767 bytes which in UTF8MB4 is 191 characters - hence the reversion for these specific fields. Unless people come accross other issues during install like #9132 then we should be ok :)

@zero-24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. Thanks for checking.

@andrepereiradasilva
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wilsonge
I updated my joomla test instaltion by just replacing the joomla files (sincronizing them with git) has i do always.

I get several db warnings.
I press fix and i get an error.

An error has occurred.
 1071 Specified key was too long; max key length is 1000 bytes SQL=ALTER TABLE `#__menu` MODIFY `alias` varchar(400) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL COMMENT 'The SEF alias of the menu item.';

Also this ones are still in database warning:

Table 'kg1u7_menu' does not have column 'alias' with type 'varchar(400)'. (From file 3.5.0-2015-07-01.sql.)
Table 'kg1u7_newsfeeds' does not have column 'alias' with type 'varchar(400)'. (From file 3.5.0-2015-07-01.sql.)
Table 'kg1u7_tags' does not have column 'alias' with type 'varchar(400)'. (From file 3.5.0-2015-07-01.sql.)
Table 'kg1u7_ucm_content' does not have column 'core_alias' with type 'varchar(400)'. (From file 3.5.0-2015-07-01.sql.)

I don't know if it's related to this change...

@infograf768
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same problem here. My db is already set to varchar(191) on my test site.
Shall we not not make a conditional to the series of ALTER TABLE for future updates (from 3.5.0 to 3.5.1)?

@infograf768
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may have found the solution:

Change
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_admin/sql/updates/mysql/3.5.0-2015-07-01.sql#L86

to
ALTER TABLE#__menuMODIFYaliasvarchar(400) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL DEFAULT '' COMMENT 'The SEF alias of the menu item.';

@andrepereiradasilva
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, that may be the problem, i.e., the lack of default.

@mbabker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks more like a hack than a fix to me. Taken at a global context, it implies that menu data would be OK without an alias set. Then again, honestly the SQL schema is in terrible shape globally and the application has far too many dependencies on the data that gets inserted during installation, but fixing that is something far beyond scope here.

@wilsonge
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem is I need to set the index's of the alias's to be max 100 chars then it doesn't exist 767

Please sign in to comment.