Database: stop leaking query state between statements on a model

src/Database/Barry/Builder.php

@@ -46,12 +46,14 @@ public function eager(string|array $relations): Builder
      */
     public function get(array $columns = []): Model|Collection|null
     {
-        $data = parent::get($columns);
-
-        // Read and reset the eager loads now: query() memoizes a shared Builder
-        // instance, so the list must not leak into the next query on this model.
-        $eager_loads = $this->eager_loads;
-        $this->eager_loads = [];
+        // Read and reset the eager loads even when the query throws, so the
+        // list never reaches the next query on this builder.
+        try {
+            $data = parent::get($columns);
+        } finally {
+            $eager_loads = $this->eager_loads;
+            $this->eager_loads = [];
+        }
 
         if (is_null($data)) {
             return null;

src/Database/Barry/Model.php

@@ -236,19 +236,15 @@ public function getConnection(): ?string
      */
     public static function query(): Builder
     {
-        if (
-            static::$builder instanceof Builder
-            && static::$builder->getModel() == static::class
-        ) {
-            if (DB::getConnectionName() == static::$builder->getAdapterName()) {
-                return static::$builder;
-            }
-        }
-
-        // Reflection action
-        $reflection = new ReflectionClass(static::class);
+        // A new builder on every call: one
+        // builder reused for the whole process replayed what a failed or
+        // half-built query left in it into the next query on the model. Only
+        // the class's default properties are cached. static::$builder is
+        // still assigned below: create() and writeRows() read it.
+        static $default_properties = [];
 
-        $properties = $reflection->getDefaultProperties();
+        $properties = $default_properties[static::class]
+            ??= (new ReflectionClass(static::class))->getDefaultProperties();
 
         if (!isset($properties['table']) || $properties['table'] == null) {
             $parts = explode('\\', static::class);
@@ -356,10 +352,9 @@ public static function first(): array|object|null
      */
     public static function retrieveBy(string $column, mixed $value): Collection
     {
-        $model = new static();
-        $model->where($column, $value);
-
-        return $model->get();
+        // One chain on one builder: query() no longer shares a builder between
+        // separate calls.
+        return static::query()->where($column, $value)->get();
     }
 
     /**
@@ -404,14 +399,16 @@ public static function retrieve(
         $id = (array) $id;
 
         $model = new static();
-        $model->select($select);
-        $model->whereIn($model->primary_key, $id);
+
+        // One chain on one builder: query() no longer shares a builder between
+        // separate calls.
+        $query = static::query()->select($select)->whereIn($model->primary_key, $id);
 
         if (count($id) != 1) {
-            return $model->get();
+            return $query->get();
         }
 
-        return $model->first();
+        return $query->first();
     }
 
     /**
@@ -591,14 +588,14 @@ private function writeRows(Builder $builder): int
         // Insert information in the database
         $row_affected = $builder->insert($this->attributes);
 
-        // We get a last insertion id value
-        if (static::$builder->getAdapterName() == 'pgsql') {
+        // We get a last insertion id value from the builder that inserted
+        if ($builder->getAdapterName() == 'pgsql') {
             if ($this->auto_increment) {
                 $sequence = $this->table . "_" . $this->primary_key . '_seq';
-                $primary_key_value = static::$builder->getPdo()->lastInsertId($sequence);
+                $primary_key_value = $builder->getPdo()->lastInsertId($sequence);
             }
         } else {
-            $primary_key_value = static::$builder->getPdo()->lastInsertId();
+            $primary_key_value = $builder->getPdo()->lastInsertId();
         }
 
         if ((int)$primary_key_value == 0) {

src/Database/QueryBuilder.php

@@ -1048,6 +1048,22 @@ public function max(string $column): int|float
      * @return mixed
      */
     private function aggregate($aggregate, $column): mixed
+    {
+        try {
+            return $this->runAggregate($aggregate, $column);
+        } finally {
+            $this->resetState();
+        }
+    }
+
+    /**
+     * The aggregate itself; aggregate() resets the builder around it.
+     *
+     * @param  $aggregate
+     * @param  string $column
+     * @return mixed
+     */
+    private function runAggregate($aggregate, $column): mixed
     {
         $column = $this->assertSafeIdentifier($column, 'aggregate', true);
         $sql = 'select ' . $aggregate . '(' . $column . ') from ' . $this->table;
@@ -1228,9 +1244,14 @@ public function last(): ?object
 
         $where_data_binding = $this->where_data_binding;
 
+        // count() resets the whole builder: keep what the final select needs.
+        $kept = [$this->select, $this->order, $this->limit, $this->as, $this->lock_for_update, $this->shared_lock];
+
         // We count all.
         $count = $this->count();
 
+        [$this->select, $this->order, $this->limit, $this->as, $this->lock_for_update, $this->shared_lock] = $kept;
+
         $this->where = $where;
 
         $this->where_data_binding = $where_data_binding;
@@ -1319,6 +1340,45 @@ public function take(int $limit): QueryBuilder
      * @throws
      */
     public function get(array $columns = []): array|object|null
+    {
+        try {
+            return $this->runGet($columns);
+        } finally {
+            $this->resetState();
+        }
+    }
+
+    /**
+     * Clear every clause, binding and flag a statement consumes, so nothing a
+     * statement left behind (failed or not) reaches the next one built on
+     * this builder.
+     *
+     * @return void
+     */
+    private function resetState(): void
+    {
+        $this->select = null;
+        $this->where = null;
+        $this->where_data_binding = [];
+        $this->join = null;
+        $this->limit = null;
+        $this->group = null;
+        $this->having = null;
+        $this->having_data_binding = [];
+        $this->order = null;
+        $this->as = null;
+        $this->first = false;
+        $this->lock_for_update = false;
+        $this->shared_lock = false;
+    }
+
+    /**
+     * The select itself; get() resets the builder around it.
+     *
+     * @param  array $columns
+     * @return array|object|null
+     */
+    private function runGet(array $columns): array|object|null
     {
         if (count($columns) > 0) {
             $this->select($columns);
@@ -1421,6 +1481,21 @@ public function jump(int $offset = 0): QueryBuilder
      * @return int
      */
     public function update(array $data = []): int
+    {
+        try {
+            return $this->runUpdate($data);
+        } finally {
+            $this->resetState();
+        }
+    }
+
+    /**
+     * The update itself; update() resets the builder around it.
+     *
+     * @param  array $data
+     * @return int
+     */
+    private function runUpdate(array $data): int
     {
         $columns = array_keys($data);
 
@@ -1470,6 +1545,20 @@ public function remove(string $column, mixed $comparator = '=', $value = null):
      * @return int
      */
     public function delete(): int
+    {
+        try {
+            return $this->runDelete();
+        } finally {
+            $this->resetState();
+        }
+    }
+
+    /**
+     * The delete itself; delete() resets the builder around it.
+     *
+     * @return int
+     */
+    private function runDelete(): int
     {
         $sql = 'delete from ' . $this->table;
 
@@ -1522,6 +1611,24 @@ public function decrement(string $column, int $step = 1): int
      * @return int
      */
     private function incrementAction(string $column, int $step = 1, string $direction = '+')
+    {
+        // The bindings were never cleared here before, even on success.
+        try {
+            return $this->runIncrementAction($column, $step, $direction);
+        } finally {
+            $this->resetState();
+        }
+    }
+
+    /**
+     * The increment itself; incrementAction() resets the builder around it.
+     *
+     * @param  string $column
+     * @param  int    $step
+     * @param  string $direction
+     * @return int
+     */
+    private function runIncrementAction(string $column, int $step, string $direction): int
     {
         $column = $this->assertSafeIdentifier($column, 'incrementAction');
         $sql = 'update ' . $this->table . ' set ' . $column . ' = ' . $column . ' ' . $direction . ' ' . $step;