Skip to content

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

Closed
wants to merge 1 commit into from
Closed

Conversation

devnexen
Copy link
Member

@devnexen devnexen commented Jun 8, 2025

No description provided.

@devnexen devnexen marked this pull request as ready for review June 8, 2025 13:45
@@ -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], [
Copy link
Member

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.

Copy link
Member

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));
Copy link
Member

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?

@@ -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);
Copy link
Member

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?

Copy link
Member

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.
Copy link
Member

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.

@devnexen devnexen force-pushed the pdo_sqlite_refactor branch from afcf17f to 7f80836 Compare June 9, 2025 12:22
allow to check if a statement is still running before reusage.

close phpGH-18804
@devnexen devnexen force-pushed the pdo_sqlite_refactor branch from 7f80836 to 55a9b03 Compare June 9, 2025 12:22
@devnexen devnexen closed this in 53231a8 Jun 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants