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

fix: now() throws error when using SQLite3 DBDriver #9228

Closed
wants to merge 1 commit into from

Conversation

grimpirate
Copy link
Contributor

@grimpirate grimpirate commented Oct 16, 2024

Description
DatabaseHandler uses the now() function for timestamp generation which is MySQL specific. This means that when using the DatabaseHandler for sessions and using the SQLite3 driver the functions that use now() will throw an exception. The proposed fix checks which DBDriver is being used, and if it's SQLite3 uses the datetime('now') function instead of now().

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

Use config instead of settings

Use CURRENT_TIMESTAMP

Strict parameter type

Lint fix

Lint fix

Removed unneeded functions/variables
@michalsn
Copy link
Member

@grimpirate
Copy link
Contributor Author

SQLite3 implements advisory locking and there's also exclusive locking mode. I don't understand/know enough about the problems that can occur in a session without the appropriate type of locks, and will defer to your expertise. I would only ask if it's worth investigating further or if that's already been done and the conclusion is SQLite3 couldn't possibly handle sessions at all (it's difficult in my mind to reconcile since the file-based session mechanism works)?

@michalsn
Copy link
Member

SQLite3 would require additional coding to manage and check lock statuses, as SQLite3 does not natively support advisory locks like MySQL and PostgreSQL do.

SQLite3 only offers file-level locking, if you need advisory locks, you would have to implement them manually. This usually involves creating a separate table to track lock statuses and using transactions to manage and check those locks.

In theory, we could use exclusive locking with WAL mode, but this would still have a huge impact on application performance and responsiveness for multiple simultaneous requests.

As for file-based handler - it doesn't have the problems that SQLite3 faces.

@grimpirate
Copy link
Contributor Author

Thank you for your time and responses michalsn, I appreciate the insight.

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

Successfully merging this pull request may close these issues.

2 participants