-
Notifications
You must be signed in to change notification settings - Fork 49
Introduce an unsafe flag for using the new driver with SQLite < 3.37.0
#256
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self::MINIMUM_SQLITE_VERSION | ||
) | ||
); | ||
if ( defined( 'WP_SQLITE_UNSAFE_ENABLE_UNSUPPORTED_VERSIONS' ) && WP_SQLITE_UNSAFE_ENABLE_UNSUPPORTED_VERSIONS ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should still keep a (lower than above) minimum sqlite version check here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea! On PHP 7.2, they seem to package 3.27.2
by default:
docker run --rm php:7.2 -r "print_r((new PDO('sqlite::memory'))->query('SELECT SQLITE_VERSION();')->fetch());"
And given that 3.27.1
and 3.27.2
were bugfix releases, I think we can go with 3.27.0
(February 2019).
…PPORTED_VERSIONS"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks to Jan, I was able to test this PR. I confirm that enabling the correct constants made the WordPress site work on SQLite 3.34.1.
Modified constants:
define ( 'WP_SQLITE_AST_DRIVER', true );
define ( 'WP_SQLITE_UNSAFE_ENABLE_UNSUPPORTED_VERSIONS', true );
- And setting the correct
DB_NAME
: `define ( 'DB_NAME', 'database_name_here' );
I wonder if defining the WP_SQLITE_UNSAFE_ENABLE_UNSUPPORTED_VERSIONS
constant is necessary, or if it should be enabled automatically when the SQLite version is older and AST is enabled.
With this simple implementation, it's probably necessary (as per the |
Adds a
WP_SQLITE_UNSAFE_ENABLE_UNSUPPORTED_VERSIONS
flag for use with older versions of SQLite.I came to the conclusion that a special flag with the word
UNSAFE
might be the best solution here.By default, the new driver would still throw the following error:
But with the new flag, it could enable accessing a database created with 3.37.0 on older SQLite versions.
An important consideration or TODO:
In this form, it would serve a basic use case, which is opening an existing database on an older SQLite for previews, including writing data to the database.
However, any new
CREATE TABLE
statement would fail because of the use of theSTRICT
keyword (General error: 1 near "STRICT": syntax error
). We could address that by omitting that keyword in this mode, but maybe let's first determine whether there are use cases for it.Resolves #252.