From 542b8cb6523c0daca50397f1e07fa5f2608cc518 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Sat, 17 Oct 2026 16:27:55 +1300 Subject: [PATCH] fix(join): keep only the newest over-long join labels per adapter 8.3.7 recorded every over-long `alias.column` label in a static map that was never pruned. Join aliases are the caller's, so a Swoole worker that serves joins under ever-new long aliases kept every label it was asked for, for its whole life. The map is now per adapter and keeps the newest 1024 labels, oldest evicted first. Resetting it per read is not safe: find() can call getDocument() through getJunctionName() and SQLite's full-text lookup before it remaps its rows, which would drop the outer read's labels. Eviction only drops the oldest, a read's own labels are the newest when its rows are remapped, and an adapter runs one statement at a time, so a read never loses its own. The bound is the memory a worker can afford, not a limit on queries: a read needs at most a few labels. Co-Authored-By: Claude Opus 5.5 --- src/Database/Adapter/SQL.php | 22 +++-- tests/unit/Adapter/JoinLabelRetentionTest.php | 95 +++++++++++++++++++ 2 files changed, 110 insertions(+), 7 deletions(-) create mode 100644 tests/unit/Adapter/JoinLabelRetentionTest.php diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 75f946e59..7112adf53 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -97,12 +97,16 @@ abstract class SQL extends Adapter implements Feature\Connection, Feature\RawQue private const string JOIN_LABEL_PREFIX = '$join_'; /** - * The `alias.column` label each digest label stands for. A digest names one label for good, so - * the map only grows with the joined columns a process reads. + * The `alias.column` label each digest label stands for, oldest first. Join aliases are the + * caller's, so the map keeps only the newest JOIN_LABELS_KEPT labels instead of every label a + * long-lived worker was ever asked for. A read's labels are the newest when its rows are + * remapped, and an adapter runs one statement at a time, so none of them is evicted first. * * @var array */ - private static array $joinLabels = []; + private array $joinLabels = []; + + private const int JOIN_LABELS_KEPT = 1024; /** * MariaDB and MySQL keep a column label of up to 256 characters. @@ -4245,7 +4249,11 @@ private function joinLabel(string $alias, string $column): string } $digest = self::JOIN_LABEL_PREFIX.\md5($label); - self::$joinLabels[$digest] = $label; + unset($this->joinLabels[$digest]); + $this->joinLabels[$digest] = $label; + if (\count($this->joinLabels) > self::JOIN_LABELS_KEPT) { + unset($this->joinLabels[\array_key_first($this->joinLabels)]); + } return $digest; } @@ -6444,10 +6452,10 @@ private function remapRow(array &$row): void unset($row[$key]); continue; } - if (isset(self::$joinLabels[$key])) { - $row[self::$joinLabels[$key]] = $row[$key]; + if (isset($this->joinLabels[$key])) { + $row[$this->joinLabels[$key]] = $row[$key]; unset($row[$key]); - $key = self::$joinLabels[$key]; + $key = $this->joinLabels[$key]; } if (\str_starts_with($key, self::FOJ_ORDER_ALIAS_PREFIX)) { unset($row[$key]); diff --git a/tests/unit/Adapter/JoinLabelRetentionTest.php b/tests/unit/Adapter/JoinLabelRetentionTest.php new file mode 100644 index 000000000..bec998357 --- /dev/null +++ b/tests/unit/Adapter/JoinLabelRetentionTest.php @@ -0,0 +1,95 @@ +adapter = new class (new PDO('sqlite::memory:')) extends SQLite { + protected const int MAX_LABEL_LENGTH = 8; + }; + $this->database = new Database($this->adapter, new Cache(new NoCache())); + $this->database + ->setDatabase(self::NAMESPACE) + ->setNamespace(self::NAMESPACE) + ->setAuthorization(new Authorization()); + $this->database->create(); + + foreach (['customers' => [Attribute::string('name', size: 64)], 'orders' => [Attribute::string('customerId', size: 64)]] as $id => $attributes) { + $this->database->createCollection(Collection::create( + id: $id, + attributes: $attributes, + permissions: [Permission::create(Role::any()), Permission::read(Role::any())], + documentSecurity: false, + )); + } + + $this->database->createDocument('customers', new Document(['$id' => 'alice', 'name' => 'Alice'])); + $this->database->createDocument('orders', new Document(['$id' => 'o1', 'customerId' => 'alice'])); + } + + public function testOverlongJoinLabelsAreNotKeptForEveryAliasAWorkerReads(): void + { + for ($read = 0; $read < self::READS; $read++) { + $alias = 'customer_'.$read; + $orders = $this->database->find('orders', [ + Query::join('customers', $alias, [Query::on('customerId', $alias.'.$id')]), + Query::select(['$id', $alias.'.name']), + ]); + + $this->assertCount(1, $orders); + $this->assertSame('Alice', $orders[0]->getAttribute($alias.'.name'), 'The overlong label of read '.$read.' must be read back'); + } + + $labels = (new ReflectionProperty(SQL::class, 'joinLabels'))->getValue($this->adapter); + $this->assertIsArray($labels); + $this->assertLessThanOrEqual(self::KEPT, \count($labels), 'Every overlong label ever read is still held'); + } + + public function testAReadKeepsItsLabelsAfterOlderOnesAreEvicted(): void + { + for ($read = 0; $read < self::KEPT; $read++) { + $this->database->find('orders', [ + Query::join('customers', 'older_'.$read, [Query::on('customerId', 'older_'.$read.'.$id')]), + Query::select(['$id', 'older_'.$read.'.name']), + ]); + } + + $orders = $this->database->find('orders', [ + Query::join('customers', 'first', [Query::on('customerId', 'first.$id')]), + Query::join('customers', 'second', [Query::on('customerId', 'second.$id')]), + Query::select(['$id', 'first.name', 'second.name']), + ]); + + $this->assertSame('Alice', $orders[0]->getAttribute('first.name')); + $this->assertSame('Alice', $orders[0]->getAttribute('second.name')); + } +}