From 101c230a216aa53f93d035738500f1f03de12a8a Mon Sep 17 00:00:00 2001 From: Doxie Date: Thu, 5 Nov 2026 23:29:54 +0700 Subject: [PATCH 1/3] fix: fromSub alias on a model, find and pluck on a union - fromSub() on a model now keeps the alias in tableAlias. The soft delete check and the chunk() order use it, not the model table. - find(), findOrFail(), pluck(), pluckMap() and valueOf() throw a StateError on a union() query. Before, they gave a wrong row or an SQL error. --- packages/dox-orm/CHANGELOG.md | 3 ++ packages/dox-orm/lib/src/get.dart | 20 ++++++++- packages/dox-orm/lib/src/select.dart | 5 +++ packages/dox-orm/test/bug_fixes_test.dart | 52 +++++++++++++++++++++++ 4 files changed, 79 insertions(+), 1 deletion(-) diff --git a/packages/dox-orm/CHANGELOG.md b/packages/dox-orm/CHANGELOG.md index 508095e..db00c94 100644 --- a/packages/dox-orm/CHANGELOG.md +++ b/packages/dox-orm/CHANGELOG.md @@ -1,5 +1,8 @@ ## Unreleased +- Fix: `fromSub()` on a model now uses the alias. Before, the soft delete check and the `chunk()` order used the model table, like `blog.deleted_at`, so `Blog().fromSub(sub, 'b').get()` failed with `no such column: blog.deleted_at`. Now they use `b.deleted_at` and `b.uid`. +- Fix: `find()` and `findOrFail()` on a `union()` query now throw a `StateError`. Before, the id check went to the first part only and `LIMIT 1` to the whole union, so they could give a wrong row. Use `get()` on a union. `getFirst()` and `firstOrFail()` work as before. +- Fix: `pluck()`, `pluckMap()` and `valueOf()` on a `union()` query now throw a clear `StateError`. Before, only the first part got the select, so the database gave an SQL error. - Fix: many to many `sync()` and `toggle()` now report in `detached` only the rows that the database really deleted. Before, on a column that compares text without case, with stored `'ABC'` and `'abc'`, `sync(['ABC'])` reported `detached: ['abc']`, but the database kept both rows. - Fix: many to many `sync()` and `toggle()` now check all text ids that match a stored row only by case in one query. Before, they ran one query for each id. - Fix: `restore()` now sets `updated_at` too, like Laravel, when the model has timestamps. It uses a custom `updated_at` column, and sets nothing when the model has no timestamps. This is true for `blog.restore()` (it also sets the `updatedAt` field) and for a query like `Blog().where('id', 1).restore()`. diff --git a/packages/dox-orm/lib/src/get.dart b/packages/dox-orm/lib/src/get.dart index 4aa9d70..0240433 100644 --- a/packages/dox-orm/lib/src/get.dart +++ b/packages/dox-orm/lib/src/get.dart @@ -38,7 +38,10 @@ mixin Get implements SharedMixin { /// `find(5, null)`) throws an [ArgumentError]. /// This cannot be use with other query such as, where, join, delete. /// It works with lockForUpdate() and sharedLock(). + /// It throws a [StateError] on a union() query: the id would only be + /// checked in the first part. Use get() then. Future find(dynamic arg1, [dynamic arg2 = missingArg]) { + _checkNoUnion('find'); bool idOnly = identical(arg2, missingArg); if (!idOnly && arg1 is! String) { throw ArgumentError.value( @@ -590,7 +593,10 @@ mixin Get implements SharedMixin { } /// select only [columns], as `dox_pluck0`, `dox_pluck1`... + /// it throws a [StateError] on a union() query: only the first part + /// would get the select. Future>> _pluckRows(List columns) async { + _checkNoUnion('pluck(), pluckMap() and valueOf'); if (queryBuilder.getRawQuery().isNotEmpty) { throw StateError('pluck() and valueOf() can not be used with rawQuery()'); } @@ -710,6 +716,18 @@ mixin Get implements SharedMixin { } } + /// [method] adds its where or select to the first part of a union + /// only, so it gives a wrong row or an sql error. it throws a + /// [StateError] then. + void _checkNoUnion(String method) { + if (queryBuilder.hasUnion) { + throw StateError( + '$method() can not be used on a union() query. ' + 'Use get() and read the rows.', + ); + } + } + static void _checkChunkSize(int size) { if (size < 1) { throw ArgumentError.value(size, 'size', 'must be 1 or more'); @@ -721,7 +739,7 @@ mixin Get implements SharedMixin { String _qualified(String column) { /// the ORDER BY of a union takes only the plain output name if (column.contains('.') || queryBuilder.hasUnion) return column; - return '$tableName.$column'; + return '${tableAlias ?? tableName}.$column'; } /// the value of [key] in a row: a map, or a model (its original row) diff --git a/packages/dox-orm/lib/src/select.dart b/packages/dox-orm/lib/src/select.dart index 70cd422..e44b582 100644 --- a/packages/dox-orm/lib/src/select.dart +++ b/packages/dox-orm/lib/src/select.dart @@ -36,6 +36,11 @@ mixin Select implements SharedMixin { QueryBuilder fromSub(QueryBuilder query, String alias) { _fromSub = '(${helper.subQuery(query)}) AS ${helper.wrap(alias)}'; tableName = alias; + + /// a model reads its table name from its class, so the line above + /// does nothing there. the alias field keeps [alias] for the + /// columns that need the table in front, like `b.deleted_at`. + tableAlias = alias; return queryBuilder; } diff --git a/packages/dox-orm/test/bug_fixes_test.dart b/packages/dox-orm/test/bug_fixes_test.dart index e835fcb..3761606 100644 --- a/packages/dox-orm/test/bug_fixes_test.dart +++ b/packages/dox-orm/test/bug_fixes_test.dart @@ -430,6 +430,58 @@ void main() async { ); expect(() => Blog().findOrFail(5, null), throwsArgumentError); }); + + Future> threeBlogs() async { + List blogs = []; + for (String status in ['x', 'y', 'z']) { + Blog blog = Blog() + ..title = status + ..description = status + ..status = status; + await blog.save(); + blogs.add(blog); + } + return blogs; + } + + test('fromSub on a model uses the alias', () async { + List blogs = await threeBlogs(); + await blogs[1].delete(); + QueryBuilder> sub() => + QueryBuilder.table('blog').select('uid, title, deleted_at'); + + /// the soft delete check uses `b.deleted_at`, not `blog.deleted_at` + List rows = await Blog().fromSub(sub(), 'b').orderBy('uid').get(); + expect(rows.map((Blog b) => b.title), ['x', 'z']); + expect(await Blog().fromSub(sub(), 'b').count(), 2); + + /// chunk() orders by `b.uid` + List titles = []; + await Blog().fromSub(sub(), 'b').chunk(1, (List page) { + titles.addAll(page.map((Blog b) => b.title)); + return true; + }); + expect(titles, ['x', 'z']); + }); + + test('find, pluck and valueOf throw on a union', () async { + await threeBlogs(); + QueryBuilder> q() => + QueryBuilder.table('blog') + .where('status', 'zzz') + .union(QueryBuilder.table('blog').where('status', 'y')); + expect(() => q().find(1), throwsStateError); + expect(() => q().find('title', 'x'), throwsStateError); + expect(() => q().findOrFail(1), throwsStateError); + expect(() => q().pluck('title'), throwsStateError); + expect(() => q().pluckMap('title', 'uid'), throwsStateError); + expect(() => q().valueOf('title'), throwsStateError); + + /// getFirst() and firstOrFail() put LIMIT 1 on the whole union, so + /// they give the right row + expect((await q().getFirst())?['status'], 'y'); + expect((await q().firstOrFail())['status'], 'y'); + }); }); group('Schema fixes |', () { From 3613359628ad5166f2f72ba6aa6c66bbbbb53ec6 Mon Sep 17 00:00:00 2001 From: Doxie Date: Fri, 6 Nov 2026 07:03:17 +0700 Subject: [PATCH 2/3] fix: firstOrNew, firstOrCreate and updateOrCreate throw on a union Their where went to the first part of a union only, so they could find or write the wrong row. They now throw a StateError, like find(). The check is one shared checkNoUnion() in union.dart. --- packages/dox-orm/CHANGELOG.md | 2 +- packages/dox-orm/lib/src/get.dart | 17 +++-------------- packages/dox-orm/lib/src/insert.dart | 12 ++++++++---- packages/dox-orm/lib/src/model.dart | 3 +++ packages/dox-orm/lib/src/union.dart | 12 ++++++++++++ packages/dox-orm/test/bug_fixes_test.dart | 16 +++++++++++++++- 6 files changed, 42 insertions(+), 20 deletions(-) diff --git a/packages/dox-orm/CHANGELOG.md b/packages/dox-orm/CHANGELOG.md index db00c94..aaaaa37 100644 --- a/packages/dox-orm/CHANGELOG.md +++ b/packages/dox-orm/CHANGELOG.md @@ -1,7 +1,7 @@ ## Unreleased - Fix: `fromSub()` on a model now uses the alias. Before, the soft delete check and the `chunk()` order used the model table, like `blog.deleted_at`, so `Blog().fromSub(sub, 'b').get()` failed with `no such column: blog.deleted_at`. Now they use `b.deleted_at` and `b.uid`. -- Fix: `find()` and `findOrFail()` on a `union()` query now throw a `StateError`. Before, the id check went to the first part only and `LIMIT 1` to the whole union, so they could give a wrong row. Use `get()` on a union. `getFirst()` and `firstOrFail()` work as before. +- Fix: `find()`, `findOrFail()`, `firstOrNew()`, `firstOrCreate()` and `updateOrCreate()` on a `union()` query now throw a `StateError`. Before, their where went to the first part only and `LIMIT 1` to the whole union, so they could give a wrong row, and `firstOrCreate()` and `updateOrCreate()` could write the wrong row. Use `get()` on a union. `getFirst()` and `firstOrFail()` work as before. - Fix: `pluck()`, `pluckMap()` and `valueOf()` on a `union()` query now throw a clear `StateError`. Before, only the first part got the select, so the database gave an SQL error. - Fix: many to many `sync()` and `toggle()` now report in `detached` only the rows that the database really deleted. Before, on a column that compares text without case, with stored `'ABC'` and `'abc'`, `sync(['ABC'])` reported `detached: ['abc']`, but the database kept both rows. - Fix: many to many `sync()` and `toggle()` now check all text ids that match a stored row only by case in one query. Before, they ran one query for each id. diff --git a/packages/dox-orm/lib/src/get.dart b/packages/dox-orm/lib/src/get.dart index 0240433..302b2b2 100644 --- a/packages/dox-orm/lib/src/get.dart +++ b/packages/dox-orm/lib/src/get.dart @@ -8,6 +8,7 @@ import 'limit.dart'; import 'order_by.dart'; import 'where.dart'; import 'shared_mixin.dart'; +import 'union.dart'; /// the select sql of [qb], as get() runs it. it is a function, not a /// query method, so it can not clash with the name of a model field. @@ -41,7 +42,7 @@ mixin Get implements SharedMixin { /// It throws a [StateError] on a union() query: the id would only be /// checked in the first part. Use get() then. Future find(dynamic arg1, [dynamic arg2 = missingArg]) { - _checkNoUnion('find'); + checkNoUnion(queryBuilder, 'find'); bool idOnly = identical(arg2, missingArg); if (!idOnly && arg1 is! String) { throw ArgumentError.value( @@ -596,7 +597,7 @@ mixin Get implements SharedMixin { /// it throws a [StateError] on a union() query: only the first part /// would get the select. Future>> _pluckRows(List columns) async { - _checkNoUnion('pluck(), pluckMap() and valueOf'); + checkNoUnion(queryBuilder, 'pluck(), pluckMap() and valueOf'); if (queryBuilder.getRawQuery().isNotEmpty) { throw StateError('pluck() and valueOf() can not be used with rawQuery()'); } @@ -716,18 +717,6 @@ mixin Get implements SharedMixin { } } - /// [method] adds its where or select to the first part of a union - /// only, so it gives a wrong row or an sql error. it throws a - /// [StateError] then. - void _checkNoUnion(String method) { - if (queryBuilder.hasUnion) { - throw StateError( - '$method() can not be used on a union() query. ' - 'Use get() and read the rows.', - ); - } - } - static void _checkChunkSize(int size) { if (size < 1) { throw ArgumentError.value(size, 'size', 'must be 1 or more'); diff --git a/packages/dox-orm/lib/src/insert.dart b/packages/dox-orm/lib/src/insert.dart index 2e8705f..2db9ecc 100644 --- a/packages/dox-orm/lib/src/insert.dart +++ b/packages/dox-orm/lib/src/insert.dart @@ -1,6 +1,7 @@ import 'package:dox_orm/dox_orm.dart'; import 'shared_mixin.dart'; +import 'union.dart'; import 'update.dart'; mixin Insert implements SharedMixin { @@ -281,7 +282,7 @@ mixin Insert implements SharedMixin { Map where, [ Map values = const {}, ]) async { - T? found = await _whereAll(where).getFirst(); + T? found = await _whereAll(where, 'firstOrCreate').getFirst(); if (found != null) return found; resetSubstitutionValues(); return await queryBuilder.create({...where, ...values}); @@ -298,7 +299,7 @@ mixin Insert implements SharedMixin { Map where, [ Map values = const {}, ]) async { - T? found = await _whereAll(where).getFirst(); + T? found = await _whereAll(where, 'firstOrNew').getFirst(); return found ?? _fromData({...where, ...values}, saved: false); } @@ -318,7 +319,7 @@ mixin Insert implements SharedMixin { Map where, Map values, ) async { - T? found = await _whereAll(where).getFirst(); + T? found = await _whereAll(where, 'updateOrCreate').getFirst(); if (found == null) { resetSubstitutionValues(); return await queryBuilder.create({...where, ...values}); @@ -343,7 +344,10 @@ mixin Insert implements SharedMixin { } /// this query with `where(key, value)` for each pair of [where] - QueryBuilder _whereAll(Map where) { + /// it throws a [StateError] on a union() query: the where would go to + /// the first part only. + QueryBuilder _whereAll(Map where, String method) { + checkNoUnion(queryBuilder, method); for (MapEntry e in where.entries) { queryBuilder.where(e.key, e.value); } diff --git a/packages/dox-orm/lib/src/model.dart b/packages/dox-orm/lib/src/model.dart index d5e73dc..7e744ed 100644 --- a/packages/dox-orm/lib/src/model.dart +++ b/packages/dox-orm/lib/src/model.dart @@ -1,6 +1,7 @@ import 'dart:async'; import '../dox_orm.dart'; +import 'union.dart'; class Model extends QueryBuilder { static final Map Function()> _morphModels = @@ -1105,6 +1106,7 @@ class Model extends QueryBuilder { Map where, [ Map values = const {}, ]) async { + checkNoUnion(this, 'firstOrNew'); for (MapEntry e in where.entries) { this.where(e.key, e.value); } @@ -1136,6 +1138,7 @@ class Model extends QueryBuilder { Map where, Map values, ) async { + checkNoUnion(this, 'updateOrCreate'); for (MapEntry e in where.entries) { this.where(e.key, e.value); } diff --git a/packages/dox-orm/lib/src/union.dart b/packages/dox-orm/lib/src/union.dart index 93a721a..9080b02 100644 --- a/packages/dox-orm/lib/src/union.dart +++ b/packages/dox-orm/lib/src/union.dart @@ -60,3 +60,15 @@ mixin Union implements SharedMixin { return queryBuilder; } } + +/// [method] adds its where or select to the first part of a union only, +/// so it gives a wrong row or an sql error. it throws a [StateError] +/// then. +void checkNoUnion(QueryBuilder q, String method) { + if (q.hasUnion) { + throw StateError( + '$method() can not be used on a union() query. ' + 'Use get() and read the rows.', + ); + } +} diff --git a/packages/dox-orm/test/bug_fixes_test.dart b/packages/dox-orm/test/bug_fixes_test.dart index 3761606..8eb38f0 100644 --- a/packages/dox-orm/test/bug_fixes_test.dart +++ b/packages/dox-orm/test/bug_fixes_test.dart @@ -464,7 +464,7 @@ void main() async { expect(titles, ['x', 'z']); }); - test('find, pluck and valueOf throw on a union', () async { + test('find, pluck and first-or helpers throw on a union', () async { await threeBlogs(); QueryBuilder> q() => QueryBuilder.table('blog') @@ -476,6 +476,20 @@ void main() async { expect(() => q().pluck('title'), throwsStateError); expect(() => q().pluckMap('title', 'uid'), throwsStateError); expect(() => q().valueOf('title'), throwsStateError); + Map where = {'title': 'x'}; + Map values = {'body': 'b'}; + expect(() => q().firstOrNew(where), throwsStateError); + expect(() => q().firstOrCreate(where, values), throwsStateError); + expect(() => q().updateOrCreate(where, values), throwsStateError); + + /// a model has its own firstOrNew() and updateOrCreate() + Blog blogs() => Blog() + ..where('status', 'zzz') + ..union(Blog().where('status', 'y')); + expect(() => blogs().firstOrNew(where), throwsStateError); + expect(() => blogs().firstOrCreate(where, values), throwsStateError); + expect(() => blogs().updateOrCreate(where, values), throwsStateError); + expect(await Blog().count(), 3); /// getFirst() and firstOrFail() put LIMIT 1 on the whole union, so /// they give the right row From ba197daafdfe2863761ae65896fd3e1f20a436ce Mon Sep 17 00:00:00 2001 From: Doxie Date: Fri, 6 Nov 2026 07:39:32 +0700 Subject: [PATCH 3/3] fix: writes after fromSub use the real table for soft deletes update(), delete() and restore() run on the model table. After the fromSub() alias fix, their soft delete check used the alias, so they failed with no such column: b.deleted_at. Writes now use the table name. Reads keep the alias. --- packages/dox-orm/CHANGELOG.md | 2 +- packages/dox-orm/lib/src/model.dart | 5 +++-- packages/dox-orm/lib/src/utils/helper.dart | 2 +- packages/dox-orm/lib/src/where.dart | 8 ++++++-- packages/dox-orm/test/bug_fixes_test.dart | 17 +++++++++++++++++ 5 files changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/dox-orm/CHANGELOG.md b/packages/dox-orm/CHANGELOG.md index aaaaa37..50bef6e 100644 --- a/packages/dox-orm/CHANGELOG.md +++ b/packages/dox-orm/CHANGELOG.md @@ -1,6 +1,6 @@ ## Unreleased -- Fix: `fromSub()` on a model now uses the alias. Before, the soft delete check and the `chunk()` order used the model table, like `blog.deleted_at`, so `Blog().fromSub(sub, 'b').get()` failed with `no such column: blog.deleted_at`. Now they use `b.deleted_at` and `b.uid`. +- Fix: `fromSub()` on a model now uses the alias. Before, the soft delete check and the `chunk()` order used the model table, like `blog.deleted_at`, so `Blog().fromSub(sub, 'b').get()` failed with `no such column: blog.deleted_at`. Now they use `b.deleted_at` and `b.uid`. `update()`, `delete()` and `restore()` after `fromSub()` still run on the model table, as before. - Fix: `find()`, `findOrFail()`, `firstOrNew()`, `firstOrCreate()` and `updateOrCreate()` on a `union()` query now throw a `StateError`. Before, their where went to the first part only and `LIMIT 1` to the whole union, so they could give a wrong row, and `firstOrCreate()` and `updateOrCreate()` could write the wrong row. Use `get()` on a union. `getFirst()` and `firstOrFail()` work as before. - Fix: `pluck()`, `pluckMap()` and `valueOf()` on a `union()` query now throw a clear `StateError`. Before, only the first part got the select, so the database gave an SQL error. - Fix: many to many `sync()` and `toggle()` now report in `detached` only the rows that the database really deleted. Before, on a column that compares text without case, with stored `'ABC'` and `'abc'`, `sync(['ABC'])` reported `detached: ['abc']`, but the database kept both rows. diff --git a/packages/dox-orm/lib/src/model.dart b/packages/dox-orm/lib/src/model.dart index 7e744ed..045b29b 100644 --- a/packages/dox-orm/lib/src/model.dart +++ b/packages/dox-orm/lib/src/model.dart @@ -611,8 +611,9 @@ class Model extends QueryBuilder { withTrash(); /// only deleted rows, so every database counts the same rows (MySQL - /// counts only changed rows, Postgres and SQLite every matched row) - whereNotNull('${tableAlias ?? tableName}.deleted_at'); + /// counts only changed rows, Postgres and SQLite every matched row). + /// the UPDATE runs on the real table, so no alias of fromSub(). + whereNotNull('$tableName.deleted_at'); /// super.update(): deleted_at is not checked against fillable return await super.update(_restoreValues()) > 0; diff --git a/packages/dox-orm/lib/src/utils/helper.dart b/packages/dox-orm/lib/src/utils/helper.dart index 8d72e0a..b3a0b28 100644 --- a/packages/dox-orm/lib/src/utils/helper.dart +++ b/packages/dox-orm/lib/src/utils/helper.dart @@ -142,7 +142,7 @@ class QueryBuilderHelper { String join = queryBuilder.getJoinQuery(); String orderBy = queryBuilder.getOrderByQuery(); String limit = queryBuilder.getLimitQuery(); - String where = queryBuilder.getWhereQuery(); + String where = queryBuilder.getWhereQuery(write: true); if (join.isEmpty && orderBy.isEmpty && limit.isEmpty) { return where; } diff --git a/packages/dox-orm/lib/src/where.dart b/packages/dox-orm/lib/src/where.dart index 8ac4a8f..dbc0557 100644 --- a/packages/dox-orm/lib/src/where.dart +++ b/packages/dox-orm/lib/src/where.dart @@ -63,16 +63,20 @@ mixin Where implements SharedMixin { static final RegExp _leadingType = RegExp(r'^(AND|OR) '); - String getWhereQuery() { + /// the WHERE part. [write] is true for an UPDATE or DELETE: they run + /// on the real table, so the soft delete check uses the table name, + /// not the alias of fromSub(). + String getWhereQuery({bool write = false}) { queryBuilder.applyGlobalScopes(); List? wheres = _wheres; + String table = write ? tableName : tableAlias ?? tableName; /// wrap user conditions in brackets so an OR can not skip the /// soft delete check. onlyTrashed() keeps its check also when /// forceDelete() turns on withTrashed. if (isSoftDeletes && (trashedOnly || !withTrashed)) { String softDelete = - "${helper.wrap('${tableAlias ?? tableName}.deleted_at')} " + "${helper.wrap('$table.deleted_at')} " "${trashedOnly ? 'IS NOT NULL' : 'IS NULL'}"; if (wheres == null) { return " WHERE $softDelete"; diff --git a/packages/dox-orm/test/bug_fixes_test.dart b/packages/dox-orm/test/bug_fixes_test.dart index 8eb38f0..a16dcb7 100644 --- a/packages/dox-orm/test/bug_fixes_test.dart +++ b/packages/dox-orm/test/bug_fixes_test.dart @@ -464,6 +464,23 @@ void main() async { expect(titles, ['x', 'z']); }); + test('update and delete after fromSub on a model', () async { + List blogs = await threeBlogs(); + int id = blogs[0].uid!; + Blog q() => Blog() + ..fromSub(QueryBuilder.table('blog').select('uid'), 'b') + ..where('uid', id); + + /// writes run on the real table, so the soft delete check uses + /// `blog.deleted_at`, not `b.deleted_at` + expect(await q().update({'title': 'new'}), 1); + expect((await Blog().find(id))?.title, 'new'); + expect(await q().delete(), 1); + expect(await Blog().find(id), isNull); + expect(await q().restore(), isTrue); + expect((await Blog().find(id))?.title, 'new'); + }); + test('find, pluck and first-or helpers throw on a union', () async { await threeBlogs(); QueryBuilder> q() =>