From 8335a95f41e6f409a9cef08e1c7464fa730e0eb3 Mon Sep 17 00:00:00 2001 From: Franck DAKIA Date: Sun, 14 Jun 2026 22:37:45 +0000 Subject: [PATCH 1/2] refactor(console): add index to stub models --- src/Console/stubs/model/cache.stub | 1 + src/Console/stubs/model/notification.stub | 2 ++ src/Console/stubs/model/queue.stub | 2 ++ src/Console/stubs/model/session.stub | 1 + src/Database/Barry/Concerns/Relationship.php | 14 +++++++------- src/Database/QueryBuilder.php | 2 +- ...est__test_generate_cache_migration_stubs__1.txt | 1 + ...st_generate_notification_migration_stubs__1.txt | 2 ++ ...est__test_generate_queue_migration_stubs__1.txt | 2 ++ ...t__test_generate_session_migration_stubs__1.txt | 1 + 10 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/Console/stubs/model/cache.stub b/src/Console/stubs/model/cache.stub index a127c7a4..120570f8 100644 --- a/src/Console/stubs/model/cache.stub +++ b/src/Console/stubs/model/cache.stub @@ -14,6 +14,7 @@ class {className} extends Migration $table->addString('key_name', ['primary' => true, 'size' => 500]); $table->addText('data'); $table->addDatetime('expire', ['nullable' => true]); + $table->addIndex('expire'); $table->addTimestamps(); }); } diff --git a/src/Console/stubs/model/notification.stub b/src/Console/stubs/model/notification.stub index 22d6a68b..7f78fc42 100644 --- a/src/Console/stubs/model/notification.stub +++ b/src/Console/stubs/model/notification.stub @@ -15,6 +15,8 @@ class {className} extends Migration $table->addString('type'); $table->addString('concern_id'); $table->addString('concern_type'); + $table->addIndex('concern_id'); + $table->addIndex('concern_type'); $table->addText('data'); $table->addDatetime('read_at', ['nullable' => true]); $table->addTimestamps(); diff --git a/src/Console/stubs/model/queue.stub b/src/Console/stubs/model/queue.stub index fbb9eb7f..f81f1e0b 100644 --- a/src/Console/stubs/model/queue.stub +++ b/src/Console/stubs/model/queue.stub @@ -22,6 +22,8 @@ class {className} extends Migration $table->addDatetime('available_at'); $table->addDatetime('reserved_at', ["nullable" => true, "default" => null]); $table->addDatetime('created_at'); + $table->addIndex('queue'); + $table->addIndex('status'); }); } diff --git a/src/Console/stubs/model/session.stub b/src/Console/stubs/model/session.stub index ab20acea..b23fae18 100644 --- a/src/Console/stubs/model/session.stub +++ b/src/Console/stubs/model/session.stub @@ -15,6 +15,7 @@ class {className} extends Migration $table->addTimestamp('time'); $table->addText('data'); $table->addString('ip'); + $table->addIndex('time'); }); } diff --git a/src/Database/Barry/Concerns/Relationship.php b/src/Database/Barry/Concerns/Relationship.php index 5a3c3fa2..4ef649c4 100644 --- a/src/Database/Barry/Concerns/Relationship.php +++ b/src/Database/Barry/Concerns/Relationship.php @@ -40,13 +40,6 @@ public function belongsTo( return new BelongsTo($related_model, $this, $foreign_key, $local_key); } - /** - * Get the table key - * - * @return string - */ - abstract public function getKey(): string; - /** * The belongs to many relative * @@ -128,4 +121,11 @@ public function hasOne( return new HasOne($related_model, $this, $foreign_key, $primary_key); } + + /** + * Get the table key + * + * @return string + */ + abstract public function getKey(): string; } diff --git a/src/Database/QueryBuilder.php b/src/Database/QueryBuilder.php index 347e685d..7b54926c 100644 --- a/src/Database/QueryBuilder.php +++ b/src/Database/QueryBuilder.php @@ -1587,7 +1587,7 @@ private function execute(string $sql, array $bindings = [], bool $write = true): $this->triggerQueryEvent($sql, $ended_at - $start_at, $bindings); } catch (\Exception $e) { throw new QueryBuilderException( - 'Error executing query: ' . $e->getMessage() . ' | Query: ' . $this->last_query, + 'message: ' . $e->getMessage() . '; query: ' . $this->last_query, $this->last_query, E_ERROR, ); diff --git a/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_cache_migration_stubs__1.txt b/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_cache_migration_stubs__1.txt index a0feccd0..76b5506c 100644 --- a/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_cache_migration_stubs__1.txt +++ b/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_cache_migration_stubs__1.txt @@ -14,6 +14,7 @@ class FakeCacheMigration extends Migration $table->addString('key_name', ['primary' => true, 'size' => 500]); $table->addText('data'); $table->addDatetime('expire', ['nullable' => true]); + $table->addIndex('expire'); $table->addTimestamps(); }); } diff --git a/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_notification_migration_stubs__1.txt b/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_notification_migration_stubs__1.txt index 310f0591..a37cc486 100644 --- a/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_notification_migration_stubs__1.txt +++ b/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_notification_migration_stubs__1.txt @@ -15,6 +15,8 @@ class FakeNotificationTableMigration extends Migration $table->addString('type'); $table->addString('concern_id'); $table->addString('concern_type'); + $table->addIndex('concern_id'); + $table->addIndex('concern_type'); $table->addText('data'); $table->addDatetime('read_at', ['nullable' => true]); $table->addTimestamps(); diff --git a/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_queue_migration_stubs__1.txt b/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_queue_migration_stubs__1.txt index a9b27e94..a2257d82 100644 --- a/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_queue_migration_stubs__1.txt +++ b/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_queue_migration_stubs__1.txt @@ -22,6 +22,8 @@ class QueueTableMigration extends Migration $table->addDatetime('available_at'); $table->addDatetime('reserved_at', ["nullable" => true, "default" => null]); $table->addDatetime('created_at'); + $table->addIndex('queue'); + $table->addIndex('status'); }); } diff --git a/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_session_migration_stubs__1.txt b/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_session_migration_stubs__1.txt index 46ab892c..f6239c93 100644 --- a/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_session_migration_stubs__1.txt +++ b/tests/Console/__snapshots__/GeneratorDeepTest__test_generate_session_migration_stubs__1.txt @@ -15,6 +15,7 @@ class FakeSessionMigration extends Migration $table->addTimestamp('time'); $table->addText('data'); $table->addString('ip'); + $table->addIndex('time'); }); } From 7c250de53aa59c44117565871aa6e0fd0dd5f5a1 Mon Sep 17 00:00:00 2001 From: Franck DAKIA Date: Sun, 14 Jun 2026 23:16:02 +0000 Subject: [PATCH 2/2] fix(database): fix eager by skip null foreign key values --- src/Database/Barry/Relation.php | 125 ++++++++----- .../EagerLoadingNullForeignKeyTest.php | 172 ++++++++++++++++++ tests/Database/Stubs/UuidItemModelStub.php | 38 ++++ tests/Database/Stubs/UuidOwnerModelStub.php | 26 +++ 4 files changed, 312 insertions(+), 49 deletions(-) create mode 100644 tests/Database/Relation/EagerLoadingNullForeignKeyTest.php create mode 100644 tests/Database/Stubs/UuidItemModelStub.php create mode 100644 tests/Database/Stubs/UuidOwnerModelStub.php diff --git a/src/Database/Barry/Relation.php b/src/Database/Barry/Relation.php index bc9ab610..5a365f24 100644 --- a/src/Database/Barry/Relation.php +++ b/src/Database/Barry/Relation.php @@ -16,36 +16,42 @@ abstract class Relation * @var bool */ protected static bool $has_constraints = true; + /** * Indicate whether the relationships use a pivot table.*. * * @var bool */ protected static bool $has_pivot = false; + /** * The foreign key of the parent model. * * @var string */ protected string $foreign_key; + /** * The associated key on the parent model. * * @var string */ protected string $local_key; + /** * The parent model instance * * @var Model */ protected Model $parent; + /** * The related model instance * * @var Model */ protected Model $related; + /** * The Bow Query builder * @@ -53,6 +59,14 @@ abstract class Relation */ protected QueryBuilder $query; + /** + * Whether no parent exposed a key during eager loading, in which case the + * relation resolves to nothing without querying the database. + * + * @var bool + */ + protected bool $eager_has_no_keys = false; + /** * Relation Contractor * @@ -84,6 +98,34 @@ public function __construct(Model $related, Model $parent) */ abstract public function addConstraints(): void; + /** + * Get the results of the relationship. + * + * @return mixed + */ + abstract public function getResults(): mixed; + + /** + * The parent attribute whose value is matched against the related models. + * + * @return string + */ + abstract protected function eagerParentKey(): string; + + /** + * The related column queried when eager loading the relation. + * + * @return string + */ + abstract protected function eagerRelatedKey(): string; + + /** + * Whether the relation resolves to many related models. + * + * @return bool + */ + abstract protected function eagerIsMany(): bool; + /** * Get the parent model. * @@ -104,24 +146,6 @@ public function getRelated(): Model return $this->related; } - /** - * _Call - * - * @param string $method - * @param array $args - * @return mixed - */ - public function __call(string $method, array $args = []) - { - $result = call_user_func_array([$this->query, $method], (array)$args); - - if ($result === $this->query) { - return $this; - } - - return $result; - } - /** * Create a new row of the related * @@ -135,34 +159,6 @@ public function create(array $attributes): Model return $this->related->create($attributes); } - /** - * Get the results of the relationship. - * - * @return mixed - */ - abstract public function getResults(): mixed; - - /** - * The parent attribute whose value is matched against the related models. - * - * @return string - */ - abstract protected function eagerParentKey(): string; - - /** - * The related column queried when eager loading the relation. - * - * @return string - */ - abstract protected function eagerRelatedKey(): string; - - /** - * Whether the relation resolves to many related models. - * - * @return bool - */ - abstract protected function eagerIsMany(): bool; - /** * Run the given callback with relation constraints disabled. * @@ -197,9 +193,17 @@ public function addEagerConstraints(array $parents): void fn ($value) => !is_null($value) ))); - // Fall back to an impossible match when no parent exposes a key so the - // query stays well-formed and returns nothing. - $this->query->whereIn($this->eagerRelatedKey(), count($keys) > 0 ? $keys : [0]); + // With no keys to match, skip the value-based constraint entirely. + // Injecting a placeholder value (such as 0) is rejected by strongly + // typed columns — e.g. a PostgreSQL uuid primary key raises + // "invalid input syntax for type uuid: 0". getEager() short-circuits. + if (count($keys) === 0) { + $this->eager_has_no_keys = true; + + return; + } + + $this->query->whereIn($this->eagerRelatedKey(), $keys); } /** @@ -209,6 +213,11 @@ public function addEagerConstraints(array $parents): void */ public function getEager(): Collection { + // No parent exposed a key, so there is nothing to fetch. + if ($this->eager_has_no_keys) { + return new Collection([]); + } + $results = $this->query->get(); return $results instanceof Collection ? $results : new Collection([]); @@ -240,4 +249,22 @@ public function match(array $parents, Collection $results, string $name): void ); } } + + /** + * _Call + * + * @param string $method + * @param array $args + * @return mixed + */ + public function __call(string $method, array $args = []) + { + $result = call_user_func_array([$this->query, $method], (array)$args); + + if ($result === $this->query) { + return $this; + } + + return $result; + } } diff --git a/tests/Database/Relation/EagerLoadingNullForeignKeyTest.php b/tests/Database/Relation/EagerLoadingNullForeignKeyTest.php new file mode 100644 index 00000000..54b1b106 --- /dev/null +++ b/tests/Database/Relation/EagerLoadingNullForeignKeyTest.php @@ -0,0 +1,172 @@ +connection($name)->dropIfExists("uuid_items", false); + $migration->connection($name)->dropIfExists("uuid_owners", false); + } catch (\Exception $e) { + // Ignore errors during cleanup + } + } + } + + private function executeMigration(string $name): void + { + $migration = new MigrationExtendedStub(); + $migration->connection($name)->dropIfExists("uuid_items", false); + $migration->connection($name)->dropIfExists("uuid_owners", false); + + $migration->connection($name)->create("uuid_owners", function (Table $table) { + $table->addUuid("id", ["primary" => true]); + $table->addString("name"); + }, false); + + $migration->connection($name)->create("uuid_items", function (Table $table) { + $table->addUuid("id", ["primary" => true]); + $table->addString("name"); + $table->addUuid("owner_id", ["nullable" => true]); + }, false); + } + + /** + * Reproduces the reported crash: eager loading a belongsTo where every + * foreign key is null must return null relations, not raise a type error. + * + * @dataProvider connectionNames + */ + public function test_eager_belongs_to_with_all_null_foreign_keys(string $name) + { + $this->executeMigration($name); + + Database::connection($name)->statement( + "INSERT INTO uuid_owners (id, name) VALUES " + . "('11111111-1111-1111-1111-111111111111', 'alice')" + ); + Database::connection($name)->statement( + "INSERT INTO uuid_items (id, name, owner_id) VALUES " + . "('aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa', 'first', NULL), " + . "('bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb', 'second', NULL)" + ); + + $items = UuidItemModelStub::connection($name)->eager('owner')->get(); + + $this->assertInstanceOf(Collection::class, $items); + $this->assertCount(2, $items->all()); + + foreach ($items as $item) { + $this->assertNull($item->owner); + } + } + + /** + * A mix of null and non-null foreign keys must still match the present ones + * and leave the null ones unresolved. + * + * @dataProvider connectionNames + */ + public function test_eager_belongs_to_with_some_null_foreign_keys(string $name) + { + $this->executeMigration($name); + + Database::connection($name)->statement( + "INSERT INTO uuid_owners (id, name) VALUES " + . "('11111111-1111-1111-1111-111111111111', 'alice')" + ); + Database::connection($name)->statement( + "INSERT INTO uuid_items (id, name, owner_id) VALUES " + . "('aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa', 'owned', " + . "'11111111-1111-1111-1111-111111111111'), " + . "('bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb', 'orphan', NULL)" + ); + + $items = UuidItemModelStub::connection($name)->eager('owner')->get(); + + $byName = []; + foreach ($items as $item) { + $byName[$item->name] = $item; + } + + $this->assertInstanceOf(UuidOwnerModelStub::class, $byName['owned']->owner); + $this->assertEquals('alice', $byName['owned']->owner->name); + $this->assertNull($byName['orphan']->owner); + } + + /** + * Root-cause guard (no database server required): when every parent key is + * null the eager query must NOT carry a value-based `in (...)` predicate. + * The previous `[0]` sentinel produced `where id in (?)` bound to 0, which a + * PostgreSQL uuid column rejects. + */ + public function test_addEagerConstraints_injects_no_placeholder_when_all_keys_null() + { + // Use the in-memory sqlite connection so no server is needed. + Database::connection('sqlite'); + + $parents = [ + new UuidItemModelStub(['id' => 'a', 'name' => 'first', 'owner_id' => null]), + new UuidItemModelStub(['id' => 'b', 'name' => 'second', 'owner_id' => null]), + ]; + + $relation = Relation::noConstraints(fn () => $parents[0]->owner()); + $relation->addEagerConstraints($parents); + + $sql = strtolower($relation->toSql()); + + $this->assertStringNotContainsString(' in (', $sql); + $this->assertStringNotContainsString('where', $sql); + } +} diff --git a/tests/Database/Stubs/UuidItemModelStub.php b/tests/Database/Stubs/UuidItemModelStub.php new file mode 100644 index 00000000..3179885d --- /dev/null +++ b/tests/Database/Stubs/UuidItemModelStub.php @@ -0,0 +1,38 @@ +belongsTo(UuidOwnerModelStub::class, "owner_id"); + } +} diff --git a/tests/Database/Stubs/UuidOwnerModelStub.php b/tests/Database/Stubs/UuidOwnerModelStub.php new file mode 100644 index 00000000..35fed9ad --- /dev/null +++ b/tests/Database/Stubs/UuidOwnerModelStub.php @@ -0,0 +1,26 @@ +