From 2e60190bd99f7363fa0c83550346fbf8fd6b9e5e Mon Sep 17 00:00:00 2001 From: nadorator Date: Thu, 22 Oct 2026 17:29:20 +0200 Subject: [PATCH 1/2] [#17689] - read back the generated id when the identity is 0 on adapters without sequences Since #16920, doLowInsert() reuses any non-empty identity value instead of calling lastInsertId(), to avoid the PostgreSQL currval() error from #16436. On MySQL, 0 is not an explicit id: it is replaced by the next auto-increment value, so a model whose identity defaults to 0 kept 0 after the insert. The value on the model is still reused on adapters with sequences and for any explicit non-zero id (same value and type as before); 0 now goes through lastInsertId() on adapters without sequences. Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- CHANGELOG-5.0.md | 1 + phalcon/Mvc/Model.zep | 11 +++-- tests/database/Mvc/Model/CreateTest.php | 66 +++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 4 deletions(-) diff --git a/CHANGELOG-5.0.md b/CHANGELOG-5.0.md index eb6a3697df..509d3222b2 100644 --- a/CHANGELOG-5.0.md +++ b/CHANGELOG-5.0.md @@ -22,6 +22,7 @@ All notable changes are documented here. The format is based on [Keep a Changelo ### Fixed - `Phalcon\Db\Adapter\Pdo\Postgresql::describeReferences()` returning wrong column pairs for composite foreign keys. [#17680](https://github.com/phalcon/cphalcon/issues/17680) +- `Phalcon\Mvc\Model::doLowInsert()` not reading back the generated id on MySQL when the identity property is `0`; `0` is no longer reused as an explicit id on adapters without sequences. [#17689](https://github.com/phalcon/cphalcon/issues/17689) - `Phalcon\Mvc\View\Engine\Volt\Compiler::compile()` rendering serialized blocks for the second child of a layout when `stat` is `false`. [#17674](https://github.com/phalcon/cphalcon/issues/17674) - `Phalcon\Mvc\View\Engine\Volt\Compiler::compileCall()` silently dropping `{% call %}` blocks instead of throwing an exception. [#17676](https://github.com/phalcon/cphalcon/issues/17676) - PHP 8.6 deprecation for session handlers without `create_sid()`: `Phalcon\Session\Adapter\AbstractAdapter` and `Phalcon\Session\Adapter\Noop` now implement `SessionIdInterface` and `create_sid()`. [#17687](https://github.com/phalcon/cphalcon/issues/17687) diff --git a/phalcon/Mvc/Model.zep b/phalcon/Mvc/Model.zep index 7e379de1b2..d5e89e90fd 100644 --- a/phalcon/Mvc/Model.zep +++ b/phalcon/Mvc/Model.zep @@ -4690,7 +4690,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, unsetDefaultValues = [], value = null, values = []; - bool useExplicitIdentity; + bool useExplicitIdentity, useSequences; let bindSkip = Column::BIND_SKIP, manager = this->modelsManager, @@ -4880,9 +4880,10 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, /** * We check if the model have sequences */ - let sequenceName = null; + let sequenceName = null, + useSequences = (bool) connection->supportSequences(); - if connection->supportSequences() { + if useSequences { if method_exists(this, "getSequenceName") { let sequenceName = this->{"getSequenceName"}(); } else { @@ -4903,10 +4904,12 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, * used, so calling lastInsertId() would fail on PostgreSQL because * currval() requires nextval() to have been called in the session. * Reuse the value already present on the model in that case. + * Without sequences, 0 is not an explicit value: MySQL replaces it + * with a generated id, which only lastInsertId() returns. */ fetch value, this->{attributeField}; - if value !== null && value !== "" { + if value !== null && value !== "" && (useSequences || value != 0) { let lastInsertedId = value; } else { let lastInsertedId = connection->lastInsertId(sequenceName); diff --git a/tests/database/Mvc/Model/CreateTest.php b/tests/database/Mvc/Model/CreateTest.php index 1a257d9409..01edd133d4 100644 --- a/tests/database/Mvc/Model/CreateTest.php +++ b/tests/database/Mvc/Model/CreateTest.php @@ -18,6 +18,7 @@ use Phalcon\Tests\Support\Migrations\InvoicesMigration; use Phalcon\Tests\Support\Models\Invoices; use Phalcon\Tests\Support\Traits\DiTrait; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use function date; @@ -37,6 +38,17 @@ public function setUp(): void (new InvoicesMigration($connection)); } + /** + * @return array + */ + public static function zeroIdentityProvider(): array + { + return [ + 'int' => [0], + 'string' => ['0'], + ]; + } + /** * @author Phalcon Team * @since 2020-02-01 @@ -150,6 +162,30 @@ public function testMvcModelCreateAutoIncrementAfterExplicitIdPgsql(): void $this->assertGreaterThan(77, (int) $second->inv_id); } + /** + * An explicit identity value is kept as set on the model (same value and + * type) on MySQL and SQLite. + * + * @issue https://github.com/phalcon/cphalcon/issues/17689 + * @author Phalcon Team + * @since 2026-10-22 + */ + #[Group('mysql')] + #[Group('sqlite')] + public function testMvcModelCreateWithExplicitIdMysqlSqlite(): void + { + $invoice = new Invoices(); + $invoice->inv_id = 77; + $invoice->inv_cst_id = 2; + $invoice->inv_status_flag = 1; + $invoice->inv_title = uniqid('inv-'); + $invoice->inv_total = 50.00; + $invoice->inv_created_at = date('Y-m-d H:i:s'); + + $this->assertNotFalse($invoice->create()); + $this->assertSame(77, $invoice->inv_id); + } + /** * Inserting with an explicit primary key on PostgreSQL must not throw a * "currval of sequence not yet defined in this session" error. @@ -175,4 +211,34 @@ public function testMvcModelCreateWithExplicitIdPgsql(): void $this->assertNotFalse($result); $this->assertSame(77, (int) $invoice->inv_id); } + + /** + * On MySQL, an identity left at 0 is replaced by a generated id; the + * model must read it back with lastInsertId() instead of keeping 0. + * Assumes the default sql_mode (no NO_AUTO_VALUE_ON_ZERO). + * + * @issue https://github.com/phalcon/cphalcon/issues/17689 + * @author Phalcon Team + * @since 2026-10-22 + */ + #[DataProvider('zeroIdentityProvider')] + #[Group('mysql')] + public function testMvcModelCreateWithZeroIdentityMysql(int | string $zero): void + { + $invoice = new Invoices(); + $invoice->inv_id = $zero; + $invoice->inv_cst_id = 2; + $invoice->inv_status_flag = 1; + $invoice->inv_title = uniqid('inv-'); + $invoice->inv_total = 50.00; + $invoice->inv_created_at = date('Y-m-d H:i:s'); + + $this->assertNotFalse($invoice->create()); + + $stored = Invoices::findFirst(); + + $this->assertNotNull($stored); + $this->assertGreaterThan(0, (int) $stored->inv_id); + $this->assertEquals($stored->inv_id, $invoice->inv_id); + } } From 877dc97bc5e40495c0f516ddd11d751c1004cbb2 Mon Sep 17 00:00:00 2001 From: nadorator Date: Mon, 26 Oct 2026 15:53:16 +0100 Subject: [PATCH 2/2] [#17689] - address review: comment wording, SQLite zero test, changelog - Model.zep: reword the comment; without sequences lastInsertId() returns the stored id, MySQL can replace 0 with a generated id. - CreateTest: add testMvcModelCreateWithZeroIdentitySqlite (0 is stored as given and read back as the string '0'), assertSame on the MySQL zero test, union type spacing. - CHANGELOG: shorten the entry. Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- CHANGELOG-5.0.md | 2 +- phalcon/Mvc/Model.zep | 4 ++-- tests/database/Mvc/Model/CreateTest.php | 32 ++++++++++++++++++++++++- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/CHANGELOG-5.0.md b/CHANGELOG-5.0.md index 509d3222b2..347c1128b2 100644 --- a/CHANGELOG-5.0.md +++ b/CHANGELOG-5.0.md @@ -22,7 +22,7 @@ All notable changes are documented here. The format is based on [Keep a Changelo ### Fixed - `Phalcon\Db\Adapter\Pdo\Postgresql::describeReferences()` returning wrong column pairs for composite foreign keys. [#17680](https://github.com/phalcon/cphalcon/issues/17680) -- `Phalcon\Mvc\Model::doLowInsert()` not reading back the generated id on MySQL when the identity property is `0`; `0` is no longer reused as an explicit id on adapters without sequences. [#17689](https://github.com/phalcon/cphalcon/issues/17689) +- `Phalcon\Mvc\Model::doLowInsert()` not reading back the generated id on MySQL when the identity property is `0`. [#17689](https://github.com/phalcon/cphalcon/issues/17689) - `Phalcon\Mvc\View\Engine\Volt\Compiler::compile()` rendering serialized blocks for the second child of a layout when `stat` is `false`. [#17674](https://github.com/phalcon/cphalcon/issues/17674) - `Phalcon\Mvc\View\Engine\Volt\Compiler::compileCall()` silently dropping `{% call %}` blocks instead of throwing an exception. [#17676](https://github.com/phalcon/cphalcon/issues/17676) - PHP 8.6 deprecation for session handlers without `create_sid()`: `Phalcon\Session\Adapter\AbstractAdapter` and `Phalcon\Session\Adapter\Noop` now implement `SessionIdInterface` and `create_sid()`. [#17687](https://github.com/phalcon/cphalcon/issues/17687) diff --git a/phalcon/Mvc/Model.zep b/phalcon/Mvc/Model.zep index d5e89e90fd..be412570d8 100644 --- a/phalcon/Mvc/Model.zep +++ b/phalcon/Mvc/Model.zep @@ -4904,8 +4904,8 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, * used, so calling lastInsertId() would fail on PostgreSQL because * currval() requires nextval() to have been called in the session. * Reuse the value already present on the model in that case. - * Without sequences, 0 is not an explicit value: MySQL replaces it - * with a generated id, which only lastInsertId() returns. + * Without sequences, lastInsertId() returns the stored id. MySQL + * can replace 0 with a generated id. */ fetch value, this->{attributeField}; diff --git a/tests/database/Mvc/Model/CreateTest.php b/tests/database/Mvc/Model/CreateTest.php index 01edd133d4..5a5ed55dda 100644 --- a/tests/database/Mvc/Model/CreateTest.php +++ b/tests/database/Mvc/Model/CreateTest.php @@ -223,7 +223,7 @@ public function testMvcModelCreateWithExplicitIdPgsql(): void */ #[DataProvider('zeroIdentityProvider')] #[Group('mysql')] - public function testMvcModelCreateWithZeroIdentityMysql(int | string $zero): void + public function testMvcModelCreateWithZeroIdentityMysql(int|string $zero): void { $invoice = new Invoices(); $invoice->inv_id = $zero; @@ -240,5 +240,35 @@ public function testMvcModelCreateWithZeroIdentityMysql(int | string $zero): voi $this->assertNotNull($stored); $this->assertGreaterThan(0, (int) $stored->inv_id); $this->assertEquals($stored->inv_id, $invoice->inv_id); + $this->assertSame((int) $stored->inv_id, (int) $invoice->inv_id); + } + + /** + * On SQLite, an identity of 0 is stored as given; the model reads it + * back with lastInsertId(), which returns it as a string. + * + * @issue https://github.com/phalcon/cphalcon/issues/17689 + * @author Phalcon Team + * @since 2026-10-26 + */ + #[DataProvider('zeroIdentityProvider')] + #[Group('sqlite')] + public function testMvcModelCreateWithZeroIdentitySqlite(int|string $zero): void + { + $invoice = new Invoices(); + $invoice->inv_id = $zero; + $invoice->inv_cst_id = 2; + $invoice->inv_status_flag = 1; + $invoice->inv_title = uniqid('inv-'); + $invoice->inv_total = 50.00; + $invoice->inv_created_at = date('Y-m-d H:i:s'); + + $this->assertNotFalse($invoice->create()); + + $stored = Invoices::findFirst(); + + $this->assertNotNull($stored); + $this->assertSame(0, (int) $stored->inv_id); + $this->assertSame('0', $invoice->inv_id); } }