Skip to content

[HttpFoundation] [PDO] Don't fetch time when reading the session #43120

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 21, 2021
Merged

[HttpFoundation] [PDO] Don't fetch time when reading the session #43120

merged 1 commit into from
Sep 21, 2021

Conversation

IonBazan
Copy link
Contributor

Q A
Branch? 6.0
Bug fix? no
New feature? no
Deprecations? no
Tickets
License MIT
Doc PR

Following up #33169 (comment)

BTW is there a reason we are using FETCH_NUM instead of FETCH_ASSOC for more readability?

@carsonbot carsonbot added this to the 6.0 milestone Sep 21, 2021
@carsonbot carsonbot changed the title [HttpFoundation][PDO] Don't fetch time when reading the session [HttpFoundation] [PDO] Don't fetch time when reading the session Sep 21, 2021
@stof
Copy link
Member

stof commented Sep 21, 2021

BTW is there a reason we are using FETCH_NUM instead of FETCH_ASSOC for more readability?

Currently, the SQL does not guarantee fixed names for the fetched columns in FETCH_ASSOC mode as it does not alias them and the field names in the database are configurable. So this would make the code more complex.

@stof
Copy link
Member

stof commented Sep 21, 2021

Looks like this breaks some tests. Maybe they were relying on the BC layer by storing the lifetime instead of the expiry ?

@IonBazan
Copy link
Contributor Author

Thanks for explanation, @stof.
I will look into the test issues. Converting to draft for now.

@IonBazan IonBazan marked this pull request as draft September 21, 2021 10:56
@IonBazan IonBazan marked this pull request as ready for review September 21, 2021 11:12
@fabpot fabpot force-pushed the pdo-session-cleanup branch from 6111389 to fb9508f Compare September 21, 2021 13:00
@fabpot
Copy link
Member

fabpot commented Sep 21, 2021

Thank you @IonBazan.

@fabpot fabpot merged commit ebe0bdd into symfony:6.0 Sep 21, 2021
@IonBazan IonBazan deleted the pdo-session-cleanup branch September 22, 2021 03:33
@Tobion Tobion mentioned this pull request Oct 11, 2021
Merged
fabpot added a commit that referenced this pull request Oct 12, 2021
@fabpot
minor #43395 [HttpFoundation] remove legacy session lifetime logic (T…
0e258bd 
…obion)

This PR was merged into the 6.0 branch.

Discussion
----------

[HttpFoundation] remove legacy session lifetime logic

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       |
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

This part was only relevant for keeping bc with the old lifetime data. Has been forgotten to be removed in #41334 and #43120

Commits
-------

54a578b [HttpFoundation] remove legacy session lifetime logic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@stof stof stof approved these changes

Assignees
No one assigned
Labels
HttpFoundation Status: Reviewed
Projects
None yet
Milestone
6.0
Development

Successfully merging this pull request may close these issues.
4 participants
@IonBazan @stof @fabpot @carsonbot