perf(orm): use EXISTS instead of COUNT subquery for some/none/every relation filters#2455
perf(orm): use EXISTS instead of COUNT subquery for some/none/every relation filters#2455
Conversation
…elation filters Fixes correlated COUNT subquery that caused O(n) performance (one subquery execution per parent row) for relation filters. Replaces with EXISTS which short-circuits on first match. Fixes #2440 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Claude finished @ymc9's task —— View job PR Review: EXISTS optimization for relation filtersProgress
Analysis of Main ImplementationThe changes in Before (lines 447-478): const countSelect = (negate: boolean) => {
return this.eb
.selectFrom(/* nested count subquery with outer wrapper for MySQL scope */)
.select('$count');
};
// Then: countSelect(false) > 0, countSelect(true) = 0, etc.After (lines 447-455): const existsSelect = (negate: boolean) => {
return this.eb.exists(/* direct EXISTS query */);
};
// Then: existsSelect(false), NOT existsSelect(true), etc.Code Quality Assessment ✅Strengths:
Architecture: The change maintains all existing interfaces and abstractions. The Test Coverage Analysis ✅The regression test provides excellent coverage:
Performance Analysis ✅Significant improvement:
Potential Issues & Edge Cases ✅Reviewed for:
Security Considerations ✅No security concerns identified:
Final AssessmentVerdict: Excellent optimization with no issues identified ✅ This is a textbook performance optimization that:
Recommendation: Approve and merge. This change will significantly benefit users with large datasets using relation filters. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaces COUNT-based correlated subqueries with EXISTS predicates for relation filters ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ABLE_USED MySQL raises "You can't specify target table for update in FROM clause" when a DELETE/UPDATE references the same table in an EXISTS subquery. Fix by overriding buildExistsExpression in MySqlCrudDialect to wrap the inner SELECT in a derived table, which MySQL treats as a separate entity. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Claude finished @ymc9's task —— View job PR Review: EXISTS optimization for relation filtersProgress
Analysis of Main ImplementationThe changes in Before (lines 447-478): const countSelect = (negate: boolean) => {
return this.eb
.selectFrom(/* nested count subquery with outer wrapper for MySQL scope */)
.select('$count');
};
// Then: countSelect(false) > 0, countSelect(true) = 0, etc.After (lines 447-455): const existsSelect = (negate: boolean) => {
const filter = this.buildFilter(relationModel, relationFilterSelectAlias, subPayload);
const innerQuery = this.buildSelectModel(relationModel, relationFilterSelectAlias)
.select(this.eb.lit(1).as('_'))
.where(buildPkFkWhereRefs(this.eb))
.where(() => (negate ? this.eb.not(filter) : filter));
return this.buildExistsExpression(innerQuery);
};
// Then: existsSelect(false), NOT existsSelect(true), etc.Code Quality Assessment ✅Strengths:
Architecture: The change maintains all existing interfaces and abstractions. The MySQL Dialect Enhancement ✅The MySQL dialect adds a crucial optimization in protected override buildExistsExpression(innerQuery: SelectQueryBuilder<any, any, any>): Expression<SqlBool> {
// MySQL doesn't allow referencing the target table of a DELETE/UPDATE in a subquery
// directly within the same statement. Wrapping in a derived table materializes the
// subquery, making it a separate virtual table that MySQL accepts.
return this.eb.exists(this.eb.selectFrom(innerQuery.as('$exists_sub')).select(this.eb.lit(1).as('_')));
}This addresses MySQL's "can't specify target table for update in FROM clause" limitation by wrapping the EXISTS subquery in a derived table. Test Coverage Analysis ✅The regression test provides excellent coverage:
Test Logic Validation:
Performance Analysis ✅Significant improvement:
Theoretical performance:
Potential Issues & Edge Cases ✅Reviewed for:
Security Considerations ✅No security concerns identified:
Final AssessmentVerdict: Excellent optimization with no issues identified ✅ This is a textbook performance optimization that:
Recommendation: Approve and merge. This change will significantly benefit users with large datasets using relation filters. |
Summary
COUNT(*) > 0subqueries withEXISTSfor{ some },{ none }, and{ every }relation filterssome→EXISTS (SELECT 1 FROM ... WHERE <filter>)none→NOT EXISTS (SELECT 1 FROM ... WHERE <filter>)every→NOT EXISTS (SELECT 1 FROM ... WHERE NOT <filter>)The old COUNT pattern executed once per parent row (O(n)), causing ~20s response times on tables with 928k rows. EXISTS short-circuits on the first match, reducing execution to <1ms with proper indexes.
Fixes #2440
Test plan
tests/regression/test/issue-2440.test.tscovering all three filter operators (some,none,every)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
some,none, andeveryconditions by switching to direct existence checks, yielding correct and more efficient results across dialects.Tests
some,none, andeveryfiltering scenarios to ensure correct behavior with various related-record setups.