-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Pdo sqlite refactor #18804
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
Pdo sqlite refactor #18804
Conversation
@@ -1923,7 +1923,7 @@ dnl | |||
dnl Common setup macro for SQLite library. | |||
dnl | |||
AC_DEFUN([PHP_SETUP_SQLITE], [ | |||
PKG_CHECK_MODULES([SQLITE], [sqlite3 >= 3.7.7], [ | |||
PKG_CHECK_MODULES([SQLITE], [sqlite3 >= 3.7.17], [ |
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.
Strictly speaking this requires an upgrading entry.
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.
Similarly, your newly added constants need upgrading entries
$db->prepare("INSERT INTO test_sqlite_stmt_getattribute VALUES ('interleaved'), ('statements')")->execute(); | ||
$st = $db->prepare("SELECT * FROM test_sqlite_stmt_getattribute"); | ||
|
||
var_dump($st->getAttribute(PDO::SQLITE_ATTR_BUSY_STATEMENT)); |
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.
This uses the old-style constant instead of the new one you introduced. Shouldn't both be tested?
ext/pdo_sqlite/pdo_sqlite.c
Outdated
@@ -421,6 +417,7 @@ PHP_MINIT_FUNCTION(pdo_sqlite) | |||
REGISTER_PDO_CLASS_CONST_LONG("SQLITE_OPEN_CREATE", (zend_long)SQLITE_OPEN_CREATE); | |||
REGISTER_PDO_CLASS_CONST_LONG("SQLITE_ATTR_READONLY_STATEMENT", (zend_long)PDO_SQLITE_ATTR_READONLY_STATEMENT); | |||
REGISTER_PDO_CLASS_CONST_LONG("SQLITE_ATTR_EXTENDED_RESULT_CODES", (zend_long)PDO_SQLITE_ATTR_EXTENDED_RESULT_CODES); | |||
REGISTER_PDO_CLASS_CONST_LONG("SQLITE_ATTR_BUSY_STATEMENT", (zend_long)PDO_SQLITE_ATTR_BUSY_STATEMENT); |
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.
These are old-style constants instead of the subclass specific ones. Do we still want to add new ones to here instead of only continuing to evolve the subclasses?
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.
Yes, we should only add new ones on the subclasses. Reminds me that I wanted to slate the overloaded PDO instances with methods for deprecation in 8.5
UPGRADING
Outdated
@@ -452,6 +452,9 @@ PHP 8.5 UPGRADE NOTES | |||
. DECIMAL_COMPACT_SHORT. | |||
. DECIMAL_COMPACT_LONG. | |||
|
|||
- PDO_Sqlite: | |||
. ATTR_BUSY_STATEMENT. |
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.
This is a class constant, not a global constant.
afcf17f
to
7f80836
Compare
allow to check if a statement is still running before reusage. close phpGH-18804
7f80836
to
55a9b03
Compare
No description provided.