Skip to content

Fix unneeded nesting of FlatMap(FlatMap(Map(FlatJoin),_),_) cases - #87

Merged
deusaquilus merged 2 commits into
mainfrom
fix-overly-strict-nesting
Dec 24, 2025
Merged

Fix unneeded nesting of FlatMap(FlatMap(Map(FlatJoin),_),_) cases#87
deusaquilus merged 2 commits into
mainfrom
fix-overly-strict-nesting

Conversation

@deusaquilus

@deusaquilus deusaquilus commented Dec 24, 2025

Copy link
Copy Markdown
Member

This PR removes the overly restrictive condition && this.head.body !is XR.Map from the FlatMap(FlatMap) transformation in SymbolicReduction. This condition was blocking legitimate query flattening for composite-type fragments extended with composeFrom, 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 composeFrom joins, queries were generating nested subqueries instead of flat JOINs:

@Serializable
data class CustomerOrder(val c: Customer, val o: Order)

@SqlFragment
fun activeCustomerOrders(): SqlQuery<CustomerOrder> = sql.select {
    val c = from(Table<Customer>())
    val o = join(Table<Order>()) { ord -> ord.customerId == c.id }
    where { o.status == "pending" || o.status == "processing" || o.status == "shipped" }
    CustomerOrder(c, o)
}

@SqlFragment
fun Order.items() = sql {
    composeFrom.joinLeft(Table<OrderItem>()) { oi -> oi.orderId == this@items.id }
}

val query = sql.select {
    val row = from(activeCustomerOrders())
    val oi = from(row.o.items())
    groupBy(row.c.id, row.c.name, row.o.id)
    Result(row.c.name, row.o.id, free("COALESCE(SUM(${oi?.quantity}), 0)").asPure<Int>())
}

Actual SQL (with condition):

SELECT
  "row".c_name AS customerName,
  "row".o_id AS orderId,
  COALESCE(SUM(oi.quantity), 0) AS totalItems
FROM
  (
    SELECT
      c.id AS c_id,
      c.name AS c_name,
      ...
    FROM
      Customer c
      INNER JOIN "Order" o ON o."customer_id" = c.id
    WHERE
      o.status = 'pending' OR ...
  ) AS "row"
  LEFT JOIN OrderItem oi ON oi."order_id" = "row".o_id
GROUP BY
  "row".c_id, "row".c_name, "row".o_id

Expected SQL (without condition):

SELECT
  c.name AS customerName,
  o.id AS orderId,
  COALESCE(SUM(oi.quantity), 0) AS totalItems
FROM
  Customer c
  INNER JOIN "Order" o ON o."customer_id" = c.id
  LEFT JOIN OrderItem oi ON oi."order_id" = o.id
WHERE
  o.status = 'pending' OR ...
GROUP BY
  c.id, c.name, o.id

Investigation & Discovery

The Condition's Original Purpose

The condition was added to prevent this problematic structure from reaching SqlQueryModel:

FlatMap(something, id, Map(FlatJoin(table), joinId, projection))

If SqlQueryModel processed Map(FlatJoin(...)) directly, it would generate invalid SQL:

SELECT projection FROM INNER JOIN table ...  -- Missing left-hand table!

What Was Actually Happening

Through extensive testing (see QueryFilterFlatteningReq.kt), we discovered:

  1. SymbolicReduction's FlatMap(FlatMap) transformation DOES create Map(FlatJoin, projection) structures:

    Input:  FlatMap(FlatMap(people, p, Map(FlatJoin(addr), a, (p,a))), kv, body)
    Output: FlatMap(people, p, FlatMap(Map(FlatJoin(addr), a, (p,a)), kv, body))
    
  2. ApplyMap was ALREADY fixing the problem via two transformations:

    Case 1: DetachableMap (pure projections)

    case(XR.FlatMap[DetachableMap[Is(), Is()], Is()])

    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)

    case(XR.FlatMap[XR.Map[Is<XR.FlatJoin>(), Is()], Is()])

    Same transformation, but handles cases where projection contains impurities (like free("rand()")).

  3. 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

DetachableMap checks body.hasImpurities() and returns null if impurities are found. This is correct for normal cases, but for Map(FlatJoin, impureProjection):

  • The structure is INVALID regardless of purity
  • It MUST be transformed to avoid "FROM INNER JOIN" errors
  • We can't rely on DetachableMap because it will skip impure projections

So we added an explicit case that handles FlatMap(Map(FlatJoin(...), ...), ...) without checking for impurities.

The Fix

Changes Made

  1. Removed the restrictive condition from SymbolicReduction.kt (line 217):

    // OLD (with condition):
    this is XR.FlatMap && head is XR.FlatMap && 
      !(this.hasTailPositionFlatJoin() && this.head.hasTailPositionFlatJoin() && this.head.body !is XR.Map) -> {
    
    // NEW (condition removed):
    this is XR.FlatMap && head is XR.FlatMap -> {
  2. Updated SymbolicReduction comments (lines 160-241) to:

    • Explain that the transformation creates Map(FlatJoin, projection) structures
    • Document that we rely on ApplyMap to fix these structures
    • Provide step-by-step example showing the transformation pipeline
  3. Updated ApplyMap comments (lines 117-189) to:

    • Explain both the DetachableMap case (pure projections)
    • Explain the Map(FlatJoin) special case (impure projections)
    • Document why we don't check for purity in the special case
    • Provide concrete examples with impure projections

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

  1. Performance: Eliminates unnecessary subqueries. While most query optimizers can flatten these, some don't, and it's unnecessary work for the optimizer.

  2. SQL Clarity: Generated SQL is cleaner and matches what developers would write by hand.

  3. Consistency: The composeFrom pattern without composite types produces flat joins. The composite type pattern now produces equivalent SQL.

  4. 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.

@deusaquilus deusaquilus changed the title Fix overly strict nesting Fix unneeded nesting of FlatMap(FlatMap(Map(FlatJoin),_),_) cases Dec 24, 2025
@deusaquilus
deusaquilus merged commit 5d1e0c7 into main Dec 24, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant