statistics module
Update #7000
    Failed: PDOException: SQLSTATE[42000]: Syntax error or access violation: 1071 Specified key was too long; max key length is 1000 bytes: ALTER TABLE {accesslog} CHANGE `sid` `sid` VARCHAR(128) NOT NULL DEFAULT '' COMMENT 'Browser session ID of user that visited page.'; Array ( ) in db_change_field() (line 2957 of /disk2/www/drupal7/includes/database/database.inc).

In D6 the sid column is varchar(64), the update process converts to varchar(128)
line 150 of statistics.install: the exact query is:
ALTER TABLE accesslog CHANGE `sid` `sid` VARCHAR(128) NOT NULL DEFAULT '';

The problem seems to be that in D6 there is an index accesslog_path_sid, accesslog_uid_sid and kid. These need to be dropped as part of the upgrade, before the sid column is increased.

Solution: see #23
ALTER TABLE accesslog DROP INDEX accesslog_path_sid;
ALTER TABLE accesslog DROP INDEX accesslog_uid_sid;

CommentFileSizeAuthor
#8 statistics-upgrade-1261056-6.patch1.34 KBboran
#8 statistics-test-1261056-6.patch5.93 KBboran
#5 statistics_1261056.patch1.34 KBboran
#3 statistics.patch1.11 KBboran
#1 statistics_1261056.patch1.34 KBboran

Comments

boran’s picture

boran commented
Issue summary: View changes

typo

boran’s picture

boran commented
StatusFileSize
new statistics_1261056.patch1.34 KB

Patch attached

boran’s picture

boran commented

-

boran’s picture

boran commented
Version: 8.x-dev » 7.x-dev
Priority: Major » Normal
Status: Needs work » Active
StatusFileSize
new statistics.patch1.11 KB

initial simple test to go with the above patch.
Testing of the test has not yet been done :-)

Update: please ignore the attachment, it is wrong

aspilicious’s picture

aspilicious commented
Version: 7.x-dev » 8.x-dev
Priority: Normal » Major
Status: Active » Needs work

Marking this major if its indeed an upgrade path fail.
I can't open your patch in #1 and I don't understand what the test does.

boran’s picture

boran commented
Version: 7.x-dev » 8.x-dev
Priority: Normal » Major
Status: Active » Needs work
StatusFileSize
new statistics_1261056.patch1.34 KB

Attaching the patch again.
"git diff --no-prefix statistics.install" was used to generate it, source was 7.x git dev.

Agreed, this probably affect any upgrade where the statistics module is enabled.

The test: creates a sample D6 table with a few rows, so that the upgrade procedure can be tested. It should go in modules/simpletest/tests/upgrade.
Ideally the test would fail before applying the patch, and succeed after adding the patch.
However, be patient please, this is the first test I've written. For example, I cannot get simpletest to see this new test, see http://drupal.org/node/1261260.

Update: the test attached to the comment above is wrong, update coming..

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, statistics_1261056.patch, failed testing.

boran’s picture

boran commented
StatusFileSize
new statistics-test-1261056-6.patch5.93 KB
new statistics-upgrade-1261056-6.patch1.34 KB

See attached: update of the fix and the test.

a) statistics.install patch:
dropped 'no-prefix' flag (created patches as instructed in http://drupal.org/node/1054616), add comment number.

b) simple test for statistics: as the attachment in comment #3 did not have the correct files.
(I still have the practical problem http://drupal.org/node/1261260, not related to this module)

boran’s picture

boran commented
Status: Needs work » Needs review
aspilicious’s picture

aspilicious commented

Your test doesn't work as the upgrade doesn't fail

boran’s picture

boran commented

I dont quite understand.

a) the code patch, looking at the test bot:

[06:36:03] Main branch [8.x] checkout [complete].
...
 Output: [error: patch failed: modules/statistics/statistics.install:142
error: modules/statistics/statistics.install: patch does not apply].
......
 Output: [error: a/modules/statistics/statistics.install: No such file or directory].

Why is it applied against 8.x and not 7.x? 8.x does not have the relevant code needs to be fixed.
I'm using 7.x of course, since Im working on upgrading 6>7.
==> changed the issue version back to 7.x. Will the bot automatically restart against 7.x?

b) The test patch: it passed the bot, but I have as noted above the problem of getting simpletest to see changes to its upgrade directory. Have you actually installed the test patch and can you see it in simple test?

aspilicious’s picture

aspilicious commented
Version: 8.x-dev » 7.x-dev

No time to look into this now. Will look at it in the near future, if no ones else beats me.
You can press the retest button (placed behind the patch) when changing versions.

Yeah this is a 7.x issue srry for that

boran’s picture

boran commented

#8: statistics-upgrade-1261056-6.patch queued for re-testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Issue tags: +Needs backport to D7

Fixing tag.

catch’s picture

catch
he/him
Primary language English
commented

#8: statistics-test-1261056-6.patch queued for re-testing.

boran’s picture

boran commented

Webchick: why the "needs backport"? the patch is already for D7.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Don't ask me why it was tagged, I just know it was using the wrong one. :)

xjm’s picture

xjm
she/her
Primary language English
commented

Hm I think webchick needs a coffee :)

Something else confuses me though. Shouldn't the test-only patch in #8 be failing?

catch’s picture

catch
he/him
Primary language English
commented

Yeah that ought to fail if the bug is reproducible.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Needs review » Needs work

Looks like the test needs work then.

marcingy’s picture

marcingy commented
Status: Needs work » Postponed (maintainer needs more info)

The test does not add the index that is causing the problem. I am actually confused where the index was even added to the table
as the d6 indexes are

'indexes' => array(
    'accesslog_timestamp' => array('timestamp'),
    'uid' => array('uid'),
),

and drupal 5

KEY accesslog_timestamp (timestamp),

and 4.7

update_sql("CREATE INDEX {accesslog}_timestamp_idx ON {accesslog} (timestamp);");

The indexes in question do not look like indexes created by core unless it is from a version that pre-dates drupal 4.7

boran’s picture

boran commented
Issue summary: View changes

attach patch

boran’s picture

boran commented

The plot thickens! zut.

Having gone though several of my sites, those indices are visible on some, not on others, so, its not a core issue. It must have been added by a contrib module.
So the above patch is not the solution, I'd suggest just adding to the summary of this issue, that if there error pops up, drop the indices and retry the upgrade?

ALTER TABLE accesslog DROP INDEX accesslog_path_sid;
ALTER TABLE accesslog DROP INDEX accesslog_uid_sid;

(corrected according to #23)

emarchak’s picture

emarchak commented
Status: Postponed (maintainer needs more info) » Closed (works as designed)

The correct syntax to #22 should be 

ALTER TABLE accesslog DROP INDEX accesslog_path_sid;
ALTER TABLE accesslog DROP INDEX accesslog_uid_sid;
emarchak’s picture

emarchak commented
Issue summary: View changes

add solution

vOOd’s picture

vOOd commented

Ive just upgraded my drupal D6 to D7 and Im facing this issue, but I dont understand how to fix it with your solution (alter table etc..).

Could someone please explain me step by step how to solve this problem. thanks

vOOd’s picture

vOOd commented

Ok I managed to execute those 2 lines with phpmyadmin and it now updates successfully.

xjm’s picture

xjm
she/her
Primary language English
commented

If update.php fails with this error:

  1. Restore your backup of the database from right before you attempted the upgrade.
  2. At a MySQL command prompt, or in phpmyadmin, or etc., run these mysql commands:
    ALTER TABLE accesslog DROP INDEX accesslog_path_sid;
ALTER TABLE accesslog DROP INDEX accesslog_uid_sid;
  3. Re-run update.php.
kylebrowning’s picture

kylebrowning commented

#22 worked for me.

Virgil’s picture

Virgil commented

Thank you xjm! #26 nailed it.

don@robertson.net.nz’s picture

don@robertson.net.nz commented

Check to make sure your MySQL database is Innodb and not MyIsam - have a look at http://highervisibilitywebsites.com/convert-your-mysql-database-myisam-i...

don@robertson.net.nz’s picture

don@robertson.net.nz commented
Issue summary: View changes

Update summary

AlternatiefIdee’s picture

AlternatiefIdee commented
Issue summary: View changes

I have this problem that looks like above but say it's on line 3020, is the solution then still the same ?
for me this looks not simple I never tried to work with MySQL database before.
so how do I have to do this ?

Update #7000
• Failed: PDOException: SQLSTATE[42000]: Syntax error or access violation: 1071 Specified key was too long; max key length is 1000 bytes: ALTER TABLE {accesslog} CHANGE `sid` `sid` VARCHAR(128) NOT NULL DEFAULT '' COMMENT 'Browser session ID of user that visited page.'; Array ( ) in db_change_field() (line 3020 of /domains/websitename/DEFAULT/includes/database/database.inc).