Fix unneeded nesting of FlatMap(FlatMap(Map(FlatJoin),_),_) cases - #87
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR removes the overly restrictive condition
&& this.head.body !is XR.Mapfrom theFlatMap(FlatMap)transformation in SymbolicReduction. This condition was blocking legitimate query flattening for composite-type fragments extended withcomposeFrom, causing unnecessary subqueries to be generated.The condition was originally added to prevent "FROM INNER JOIN" SQL generation errors, but investigation revealed that it was actually not solving the underlying problem. Instead, ApplyMap was already handling the problematic
Map(FlatJoin, projection)structures that the condition was meant to prevent.The Original Problem
When using composite-type fragments with
composeFromjoins, queries were generating nested subqueries instead of flat JOINs:Actual SQL (with condition):
Expected SQL (without condition):
Investigation & Discovery
The Condition's Original Purpose
The condition was added to prevent this problematic structure from reaching SqlQueryModel:
If SqlQueryModel processed
Map(FlatJoin(...))directly, it would generate invalid SQL:What Was Actually Happening
Through extensive testing (see
QueryFilterFlatteningReq.kt), we discovered:SymbolicReduction's
FlatMap(FlatMap)transformation DOES createMap(FlatJoin, projection)structures:ApplyMap was ALREADY fixing the problem via two transformations:
Case 1: DetachableMap (pure projections)
Transforms:
FlatMap(Map(FlatJoin(table), id, projection), bindId, body)To:
FlatMap(FlatJoin(table), id, BetaReduction(body, bindId -> projection))Case 2: Map(FlatJoin) special case (impure projections)
Same transformation, but handles cases where
projectioncontains impurities (likefree("rand()")).The condition was NOT preventing the problem - it was just blocking legitimate flattening. ApplyMap was doing all the actual work of fixing
Map(FlatJoin)structures!Why Case 2 Was Necessary
DetachableMapchecksbody.hasImpurities()and returnsnullif impurities are found. This is correct for normal cases, but forMap(FlatJoin, impureProjection):So we added an explicit case that handles
FlatMap(Map(FlatJoin(...), ...), ...)without checking for impurities.The Fix
Changes Made
Removed the restrictive condition from
SymbolicReduction.kt(line 217):Updated SymbolicReduction comments (lines 160-241) to:
Map(FlatJoin, projection)structuresUpdated ApplyMap comments (lines 117-189) to:
Testing
All test cases are in
testing/src/commonTest/kotlin/io/exoquery/QueryFilterFlatteningReq.kt:Test: "should flatten fragment returning composite type when extended with composeFrom"
Tests the original bug report scenario - composite fragment with WHERE clause extended with
composeFrom.joinLeft.Expected behavior: Generates flat JOINs without subqueries.
Test: "should flatten fragment with where clause when extended with composeFrom that also has where"
Tests a variant where BOTH the inner fragment and outer query have WHERE clauses.
Expected behavior: Both WHERE clauses are merged into a single statement with proper AND logic.
Why This Matters
Performance: Eliminates unnecessary subqueries. While most query optimizers can flatten these, some don't, and it's unnecessary work for the optimizer.
SQL Clarity: Generated SQL is cleaner and matches what developers would write by hand.
Consistency: The
composeFrompattern without composite types produces flat joins. The composite type pattern now produces equivalent SQL.Documentation Promise: Our abstraction patterns documentation shows composite types as the way to "DRY up queries that share common joins." This fix ensures that promise is kept.