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

Mysql column type breaking change on PdoSessionHandler #34491

Closed
tseho opened this issue Nov 21, 2019 · 8 comments
Closed

Mysql column type breaking change on PdoSessionHandler #34491

tseho opened this issue Nov 21, 2019 · 8 comments

Comments

@tseho
Copy link
Contributor

tseho commented Nov 21, 2019

Symfony version(s) affected: 4.4

Description

I think there is a BC break in #33169. On mysql, the lifetime column has been for the last few years a MEDIUMINT column.

The new version with time() + $maxlifetime instead of $maxlifetime is higher than the MEDIUMINT mysql limit.

When upgrading from symfony 4.3 to 4.4 with an existing database, we have the error SQLSTATE[22003]: Numeric value out of range: 1264 Out of range value for column 'sess_lifetime' at row 1

I saw the creation of the session table has been fixed 5 days ago in #34410 but that does not fix existing databases.

Is it normal we need to create manually a sql migration for a breaking database schema change, which is not really mentioned in UPGRADE-4.4.md ?

@s-duval
Copy link

s-duval commented Nov 22, 2019

Exactly already mentioned that bug in #34479 and was prematurely closed. That's a pretty major issue as sessions are commonly used in applications and the fact that a DB migration is required in order to work

@tseho
Copy link
Contributor Author

tseho commented Nov 22, 2019

I did miss your issue when posting mine because it was already closed.

@nicolas-grekas
Copy link
Member

Can we issue an ALTER TABLE when an exception with code 1264 is thrown? That'd be the best isn't it?
PR welcome.

@s-duval
Copy link

s-duval commented Nov 25, 2019

@nicolas-grekas Indeed I think that's the only way to fix this issue to keep backward compatibility between 4.3 and 4.4

@tseho
Copy link
Contributor Author

tseho commented Nov 25, 2019

@nicolas-grekas I did a quick POC about it last night, it's really straightforward but I'm conflicted about it.
I think it's a bad idea to run ALTER TABLE on any production database when the error occurs. It will mess with existing database migrations.
Meanwhile, I proposed #34582 for at least updating the UPGRADE-4.4.md.

@nicolas-grekas
Copy link
Member

I did a quick POC about it last night, it's really straightforward but I'm conflicted about it.

up to submitting what you've done so far? then we can discuss about the impact on the PR?

@tseho
Copy link
Contributor Author

tseho commented Nov 25, 2019

sure, I will submit it tonight.

nicolas-grekas added a commit that referenced this issue Nov 28, 2019
…BC BREAK in 4.4 (tseho)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[HttpFoundation] Update CHANGELOG for PdoSessionHandler BC BREAK in 4.4

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #34491
| License       | MIT

As explained in #34491, there was a BC BREAK between 4.3 and 4.4, when using `PdoSessionHandler` with MySQL, where the column `sess_lifetime` was modified from `MEDIUMINT` to `INTEGER UNSIGNED`.

This PR updates `UPGRADE-4.4.md` with a suggested query for updating the database accordingly.

Commits
-------

eda4d68 [HttpFoundation] Update CHANGELOG for PdoSessionHandler BC BREAK in 4.4
@mvanduijker
Copy link

Might be good to add to the changelog that this change will invalidate all your current sessions.
Before upgrading and after doing the migration of the sess_lifetime column to unsigned int, you probably have to run a query like (not tested):

UPDATE sessions SET sess_lifetime = sess_time + sess_lifetime;

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

No branches or pull requests

6 participants