From 9d74355e1bb01ce76f7bf7790c511a319fdfcba5 Mon Sep 17 00:00:00 2001 From: phdoerfler Date: Fri, 7 Aug 2026 11:08:35 +0200 Subject: [PATCH 1/8] Bump tlBaseVersion to 0.30 Laterality.joinPredicates is replaced by distributeJoinConditions, which is not binary compatible with 0.29. --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index c891a096..456ccd42 100644 --- a/build.sbt +++ b/build.sbt @@ -36,7 +36,7 @@ ThisBuild / scalaVersion := Scala2 ThisBuild / crossScalaVersions := Seq(Scala2, Scala3) ThisBuild / tlJdkRelease := Some(11) -ThisBuild / tlBaseVersion := "0.29" +ThisBuild / tlBaseVersion := "0.30" ThisBuild / startYear := Some(2019) ThisBuild / licenses := Seq(License.Apache2) ThisBuild / developers := List( From e2b64e035d8a17ac9183a387b2045fc03b189f36 Mon Sep 17 00:00:00 2001 From: phdoerfler Date: Fri, 7 Aug 2026 18:04:37 +0200 Subject: [PATCH 2/8] Give each condition in isMergeable its own line scalafmt packs the conjunction and, forbidden from breaking on a chain's first dot, breaks it mid-chain instead. Giving each condition its own line leaves nothing long enough to need breaking. --- modules/sql-core/src/main/scala/SqlMapping.scala | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/modules/sql-core/src/main/scala/SqlMapping.scala b/modules/sql-core/src/main/scala/SqlMapping.scala index 272866fc..38e1efcd 100644 --- a/modules/sql-core/src/main/scala/SqlMapping.scala +++ b/modules/sql-core/src/main/scala/SqlMapping.scala @@ -2384,11 +2384,13 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self joinCols: List[SqlColumn], suffix: String): SqlSelect = { def isMergeable: Boolean = - !multiTable && !nested.joins.exists(_.isPredicate) && nested - .wheres - .isEmpty && nested.orders.isEmpty && nested.offset.isEmpty && nested - .limit - .isEmpty && !nested.isDistinct + !multiTable && + !nested.joins.exists(_.isPredicate) && + nested.wheres.isEmpty && + nested.orders.isEmpty && + nested.offset.isEmpty && + nested.limit.isEmpty && + !nested.isDistinct if (isMergeable) nested else { From f17645d21eb87f81a7e13e81e8431eaadacf28c5 Mon Sep 17 00:00:00 2001 From: phdoerfler Date: Thu, 6 Aug 2026 20:25:40 +0200 Subject: [PATCH 3/8] Don't flatten a nested select with INNER joins below a LEFT join A non-null field nested beneath a nullable parent generated an INNER JOIN in the same flat join chain as the parent's LEFT JOIN, so rows whose nullable parent was absent were eliminated. They should be returned with the parent as null: the child's non-null-ness only applies when the parent exists. SqlSelect.nest now declines to merge a nested select into the parent's join chain when this join is LEFT and the nested select carries INNER joins, which once flattened would sit below it and filter the null-padded rows away. The existing mkSubquery path then scopes those joins inside a subquery, where the INNER remains valid and cannot eliminate outer rows, so the optimisation is preserved rather than discarded. Fixes #888. --- .../sql-core/src/main/scala/SqlMapping.scala | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/modules/sql-core/src/main/scala/SqlMapping.scala b/modules/sql-core/src/main/scala/SqlMapping.scala index 38e1efcd..3fc8b73c 100644 --- a/modules/sql-core/src/main/scala/SqlMapping.scala +++ b/modules/sql-core/src/main/scala/SqlMapping.scala @@ -2377,6 +2377,20 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self parentTableForType(parentContext).flatMap { parentTable => val inner = !context.tpe.isNullable && !context.tpe.isList + // A nested select may be flattened into this select's join chain only if doing so + // can't cost this select rows. + // + // Flattening moves the nested select's joins into this chain, below the join which + // attaches it. Where that join is LEFT and finds no match it still yields a row + // padded with NULLs, and an INNER join below discards it — losing a row which + // should have been returned with the nested field simply null or empty. Every join + // in `nested.joins` is downstream of that select's own table, so all of them land + // below the attaching join and see its padded columns, directly or transitively. + // Flattening is safe, then, if this join is INNER or the nested select has no + // INNER joins. + def mergePreservesRows(nested: SqlSelect): Boolean = + inner || !nested.joins.exists(_.inner) + def mkJoins(joins: List[Join]): SqlSelect = { def mkSubquery( multiTable: Boolean, @@ -2384,7 +2398,8 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self joinCols: List[SqlColumn], suffix: String): SqlSelect = { def isMergeable: Boolean = - !multiTable && + mergePreservesRows(nested) && + !multiTable && !nested.joins.exists(_.isPredicate) && nested.wheres.isEmpty && nested.orders.isEmpty && From f9a1b2863b45327234de202b21ac871e67516a21 Mon Sep 17 00:00:00 2001 From: phdoerfler Date: Fri, 7 Aug 2026 11:08:26 +0200 Subject: [PATCH 4/8] Correlate OUTER APPLY subqueries on MSSQL MSSQL renders a lateral join as OUTER/CROSS APPLY, which takes no ON clause, so the join's conditions were lifted into the enclosing select's WHERE clause instead. For CROSS APPLY that is equivalent to an ON clause, but for OUTER APPLY it is not: the WHERE clause is applied after the join and discards precisely the null padded rows the outer join produced, so an OUTER APPLY behaved as an inner join. Wrap the subquery of an OUTER APPLY in a correlating select which applies the conditions to its result, referring to the enclosing select's tables. That is legal because APPLY is lateral, and makes OUTER APPLY equivalent to the LEFT JOIN LATERAL other backends emit. Applying the conditions to the subquery's result rather than pushing them into it preserves any LIMIT, OFFSET or DISTINCT, and keeps a same named table within the subquery from shadowing one of the enclosing select. Laterality.joinPredicates is replaced by distributeJoinConditions, which can rewrite the join as well as yield predicates for the enclosing select. --- .../sql-core/src/main/scala/SqlMapping.scala | 130 +++++++++++++++--- 1 file changed, 112 insertions(+), 18 deletions(-) diff --git a/modules/sql-core/src/main/scala/SqlMapping.scala b/modules/sql-core/src/main/scala/SqlMapping.scala index 3fc8b73c..91124380 100644 --- a/modules/sql-core/src/main/scala/SqlMapping.scala +++ b/modules/sql-core/src/main/scala/SqlMapping.scala @@ -1385,7 +1385,19 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self trait Laterality { def toFragment: Fragment def joinToFragment(join: SqlJoin, subquery: SubqueryRef): Aliased[Fragment] - def joinPredicates(join: SqlJoin): List[Predicate] + + /** + * Distribute the conditions of `join` between the join itself and the enclosing select. + * + * Yields a possibly rewritten join, together with the predicates which must be added to + * the enclosing select's WHERE clause for the join to have its intended semantics. + * + * Lateralities which render the join with an `ON` clause need neither, and so yield the + * join unchanged and no predicates. + */ + def distributeJoinConditions( + join: SqlJoin, + subquery: SubqueryRef): (SqlJoin, List[Predicate]) } object Laterality { def apply(lateral: Boolean, inner: Boolean): Laterality = @@ -1395,21 +1407,94 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self def toFragment: Fragment = Fragments.empty def joinToFragment(join: SqlJoin, subquery: SubqueryRef): Aliased[Fragment] = join.toFragmentWithoutLaterality - def joinPredicates(join: SqlJoin): List[Predicate] = Nil + def distributeJoinConditions( + join: SqlJoin, + subquery: SubqueryRef): (SqlJoin, List[Predicate]) = (join, Nil) } case object Lateral extends Laterality { def toFragment: Fragment = Fragments.const("LATERAL ") def joinToFragment(join: SqlJoin, subquery: SubqueryRef): Aliased[Fragment] = join.toFragmentWithoutLaterality - def joinPredicates(join: SqlJoin): List[Predicate] = Nil + def distributeJoinConditions( + join: SqlJoin, + subquery: SubqueryRef): (SqlJoin, List[Predicate]) = (join, Nil) } case class Apply(inner: Boolean) extends Laterality { def toFragment: Fragment = Fragments.const(if (inner) "CROSS " else "OUTER ") |+| Fragments.const("APPLY ") def joinToFragment(join: SqlJoin, subquery: SubqueryRef): Aliased[Fragment] = subquery.toDefFragment - def joinPredicates(join: SqlJoin): List[Predicate] = + + /** + * An `APPLY` join is rendered without an `ON` clause, so its conditions have to be + * expressed elsewhere. + * + * For `CROSS APPLY` they are lifted into the enclosing select's WHERE clause, which for + * an inner join is equivalent to an `ON` clause. + * + * For `OUTER APPLY` that would be wrong: the WHERE clause is applied after the join and + * would discard precisely the null padded rows the outer join produced. Instead the + * subquery is wrapped in a correlating select which applies the conditions to its + * result, referring to the enclosing select's tables. That is legal because `APPLY` is + * lateral, and is what makes `OUTER APPLY` equivalent to `LEFT JOIN LATERAL`. + * + * Only a subquery of the right shape can be correlated, and the conditions are lifted + * as a last resort otherwise. For an `OUTER APPLY` that reinstates the semantics + * described above, so it has to stay unreachable: the shapes `correlate` declines are + * those `addFilterOrderByOffsetLimit` builds, and those join on a predicate, which is + * short circuited before it. + */ + def distributeJoinConditions( + join: SqlJoin, + subquery: SubqueryRef): (SqlJoin, List[Predicate]) = + if (inner || join.isPredicate) (join, liftedPredicates(join)) + else + correlate(join, subquery).fold((join, liftedPredicates(join)))(join0 => + (join0, Nil)) + + /** + * The join's conditions expressed as predicates of the enclosing select. + */ + private def liftedPredicates(join: SqlJoin): List[Predicate] = if (!join.isPredicate) join.on.map { case (p, c) => Eql(p.toTerm, c.toTerm) } else Nil + + /** + * Yields a copy of `join` with its conditions moved inside its subquery. + * + * The subquery is wrapped in a select which applies the conditions to the subquery's + * result, so that any LIMIT, OFFSET, DISTINCT or window function in the subquery is + * evaluated first, exactly as it would be were the conditions in the ON clause of a + * `LEFT JOIN LATERAL`. Wrapping also ensures that only the subquery's exposed columns + * are in scope where the conditions are applied, so that a table of the enclosing + * select can't be shadowed by a same named table within the subquery. + * + * The subquery side of each condition names a column exposed by the subquery, which has + * to be mapped to the corresponding column of the wrapper. Yields `None` if that isn't + * possible, in which case the caller falls back to lifting the conditions into the + * enclosing select. + */ + private def correlate(join: SqlJoin, subquery: SubqueryRef): Option[SqlJoin] = + subquery.subquery match { + case sel: SqlSelect if sel.withs.isEmpty => + sel.table match { + case table: TableRef => + val exposed = + join.on.traverse { + case (p, c) => sel.cols.find(_ == c.subst(subquery, table)).map((p, _)) + } + exposed.map { exposed0 => + val wrapper = + sel.toSubquery(subquery.name + "_corr", NotLateral, correlated = true) + val wheres = + exposed0.map { + case (p, c) => Eql(p.toTerm, c.derive(wrapper.table).toTerm) + } + join.copy(child = subquery.copy(subquery = wrapper.copy(wheres = wheres))) + } + case _ => None + } + case _ => None + } } } @@ -1420,7 +1505,8 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self context: Context, name: String, subquery: SqlQuery, - laterality: Laterality) + laterality: Laterality, + correlated: Boolean = false) extends TableExpr { def owns(col: SqlColumn): Boolean = col.owner.isSameOwner(this) || subquery.owns(col) def contains(other: ColumnOwner): Boolean = isSameOwner(other) || subquery.contains(other) @@ -2427,22 +2513,23 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self cols: List[SqlColumn], wheres: List[Predicate], joins: List[SqlJoin]): SqlSelect = { - val extraWheres = - if (!outer) Nil - else - joins.flatMap { join => - join.child match { - case sq: SubqueryRef => sq.laterality.joinPredicates(join) - case _ => Nil - } + val distributed = + joins.map { join => + join.child match { + case sq: SubqueryRef if outer => + sq.laterality.distributeJoinConditions(join, sq) + case _ => (join, Nil) } + } + val joins0 = distributed.map(_._1) + val extraWheres = distributed.flatMap(_._2) SqlSelect( context = parentContext, withs = withs, table = table, cols = cols, - joins = joins, + joins = joins0, wheres = wheres ++ extraWheres, orders = Nil, offset = None, @@ -2475,6 +2562,10 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self true) val assocJoin = lastJoin.toSqlJoin(lastJoinParentTable, assocTable, inner) + // This join is on the key between a table and itself, so it always matches and + // an outer join is as good as an inner one. `assocJoin` carries the join's real + // innerness, and it's that which decides how a subquery beneath it distributes + // its conditions. val finalJoin = SqlJoin( assocTable, @@ -3160,8 +3251,11 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self def toSubquery(name: String): Result[SqlSelect] = toSubquery(name, Laterality.NotLateral).success - def toSubquery(name: String, lateral: Laterality): SqlSelect = { - val ref = SubqueryRef(context, name, this, lateral) + def toSubquery( + name: String, + lateral: Laterality, + correlated: Boolean = false): SqlSelect = { + val ref = SubqueryRef(context, name, this, lateral, correlated) SqlSelect( context, Nil, @@ -3182,7 +3276,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self */ def subqueryToWithQuery: SqlSelect = { table match { - case SubqueryRef(_, name, sq, _) => + case SubqueryRef(_, name, sq, _, _) => val with0 = WithRef(context, name + "_base", sq) val ref = TableExpr.DerivedTableRef(context, Some(name), with0, true) copy(withs = with0 :: withs, table = ref) @@ -3482,7 +3576,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self */ def isPredicate: Boolean = child match { - case SubqueryRef(_, _, sq: SqlSelect, _) => sq.predicate + case SubqueryRef(_, _, sq: SqlSelect, _, _) => sq.predicate case _ => false } From 3566967d6fe8bb0fb905d7a05838be34a7cbd2f4 Mon Sep 17 00:00:00 2001 From: phdoerfler Date: Fri, 7 Aug 2026 12:56:50 +0200 Subject: [PATCH 5/8] Add a cross-backend suite for a non-null field under a nullable parent Exercises a nullable field with a non-null field beneath it, on every SQL backend, via a shared mapping and suite. The fixture data includes a row whose non-null reference dangles, which is the case that distinguishes a row being absent from a row being dropped. --- .../src/test/scala/DoobieMSSqlSuites.scala | 4 + .../src/test/scala/DoobieOracleSuites.scala | 4 + .../src/test/scala/DoobiePgSuites.scala | 4 + .../js-jvm/src/test/scala/SkunkSuites.scala | 4 + .../test/scala/SqlNullableParentMapping.scala | 84 ++++++++++++ .../test/scala/SqlNullableParentSuite.scala | 126 ++++++++++++++++++ testdata/mssql/nullable-parent.sql | 30 +++++ testdata/oracle/nullable-parent.sql | 28 ++++ testdata/pg/nullable-parent.sql | 31 +++++ 9 files changed, 315 insertions(+) create mode 100644 modules/sql-core/src/test/scala/SqlNullableParentMapping.scala create mode 100644 modules/sql-core/src/test/scala/SqlNullableParentSuite.scala create mode 100644 testdata/mssql/nullable-parent.sql create mode 100644 testdata/oracle/nullable-parent.sql create mode 100644 testdata/pg/nullable-parent.sql diff --git a/modules/doobie-mssql/src/test/scala/DoobieMSSqlSuites.scala b/modules/doobie-mssql/src/test/scala/DoobieMSSqlSuites.scala index d2b11c4b..51ce66c1 100644 --- a/modules/doobie-mssql/src/test/scala/DoobieMSSqlSuites.scala +++ b/modules/doobie-mssql/src/test/scala/DoobieMSSqlSuites.scala @@ -191,6 +191,10 @@ final class NestedEffectsSuite extends DoobieMSSqlDatabaseSuite with SqlNestedEf } } +final class NullableParentSuite extends DoobieMSSqlDatabaseSuite with SqlNullableParentSuite { + lazy val mapping = new DoobieMSSqlTestMapping(transactor) with SqlNullableParentMapping[IO] +} + final class NullOrderingSuite extends DoobieMSSqlDatabaseSuite with SqlNullOrderingSuite { lazy val mapping = new DoobieMSSqlTestMapping(transactor) with SqlNullOrderingMapping[IO] } diff --git a/modules/doobie-oracle/src/test/scala/DoobieOracleSuites.scala b/modules/doobie-oracle/src/test/scala/DoobieOracleSuites.scala index e53bf36f..56418c46 100644 --- a/modules/doobie-oracle/src/test/scala/DoobieOracleSuites.scala +++ b/modules/doobie-oracle/src/test/scala/DoobieOracleSuites.scala @@ -197,6 +197,10 @@ final class NestedEffectsSuite extends DoobieOracleDatabaseSuite with SqlNestedE } } +final class NullableParentSuite extends DoobieOracleDatabaseSuite with SqlNullableParentSuite { + lazy val mapping = new DoobieOracleTestMapping(transactor) with SqlNullableParentMapping[IO] +} + final class NullOrderingSuite extends DoobieOracleDatabaseSuite with SqlNullOrderingSuite { lazy val mapping = new DoobieOracleTestMapping(transactor) with SqlNullOrderingMapping[IO] } diff --git a/modules/doobie-pg/src/test/scala/DoobiePgSuites.scala b/modules/doobie-pg/src/test/scala/DoobiePgSuites.scala index 470ac7c7..83b9c444 100644 --- a/modules/doobie-pg/src/test/scala/DoobiePgSuites.scala +++ b/modules/doobie-pg/src/test/scala/DoobiePgSuites.scala @@ -193,6 +193,10 @@ final class NestedEffectsSuite extends DoobiePgDatabaseSuite with SqlNestedEffec } } +final class NullableParentSuite extends DoobiePgDatabaseSuite with SqlNullableParentSuite { + lazy val mapping = new DoobiePgTestMapping(transactor) with SqlNullableParentMapping[IO] +} + final class NullOrderingSuite extends DoobiePgDatabaseSuite with SqlNullOrderingSuite { lazy val mapping = new DoobiePgTestMapping(transactor) with SqlNullOrderingMapping[IO] } diff --git a/modules/skunk/js-jvm/src/test/scala/SkunkSuites.scala b/modules/skunk/js-jvm/src/test/scala/SkunkSuites.scala index 250f3abe..1834751c 100644 --- a/modules/skunk/js-jvm/src/test/scala/SkunkSuites.scala +++ b/modules/skunk/js-jvm/src/test/scala/SkunkSuites.scala @@ -198,6 +198,10 @@ final class NestedEffectsSuite extends SkunkDatabaseSuite with SqlNestedEffectsS } } +final class NullableParentSuite extends SkunkDatabaseSuite with SqlNullableParentSuite { + lazy val mapping = new SkunkTestMapping(pool) with SqlNullableParentMapping[IO] +} + final class NullOrderingSuite extends SkunkDatabaseSuite with SqlNullOrderingSuite { lazy val mapping = new SkunkTestMapping(pool) with SqlNullOrderingMapping[IO] } diff --git a/modules/sql-core/src/test/scala/SqlNullableParentMapping.scala b/modules/sql-core/src/test/scala/SqlNullableParentMapping.scala new file mode 100644 index 00000000..7550a299 --- /dev/null +++ b/modules/sql-core/src/test/scala/SqlNullableParentMapping.scala @@ -0,0 +1,84 @@ +// Copyright (c) 2016-2025 Association of Universities for Research in Astronomy, Inc. (AURA) +// Copyright (c) 2016-2025 Grackle Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package grackle.sql.test + +import grackle.syntax._ + +trait SqlNullableParentMapping[F[_]] extends SqlTestMapping[F] { + + object aTable extends TableDef("nullable_parent_a") { + val id = col("id", int4) + val bId = col("b_id", nullable(int4)) + val name = col("name", text) + } + + object bTable extends TableDef("nullable_parent_b") { + val id = col("id", int4) + val cId = col("c_id", int4) + val name = col("name", text) + } + + object cTable extends TableDef("nullable_parent_c") { + val id = col("id", int4) + val name = col("name", text) + } + + val schema = + schema""" + type Query { + as: [A!]! + } + type A { + name: String! + b: B + } + type B { + name: String! + c: C! + } + type C { + name: String! + } + """ + + val QueryType = schema.ref("Query") + val AType = schema.ref("A") + val BType = schema.ref("B") + val CType = schema.ref("C") + + val typeMappings = + TypeMappings( + ObjectMapping(QueryType)( + SqlObject("as") + ), + ObjectMapping(AType)( + SqlField("id", aTable.id, key = true, hidden = true), + SqlField("bId", aTable.bId, hidden = true), + SqlField("name", aTable.name), + SqlObject("b", Join(aTable.bId, bTable.id)) + ), + ObjectMapping(BType)( + SqlField("id", bTable.id, key = true, hidden = true), + SqlField("cId", bTable.cId, hidden = true), + SqlField("name", bTable.name), + SqlObject("c", Join(bTable.cId, cTable.id)) + ), + ObjectMapping(CType)( + SqlField("id", cTable.id, key = true, hidden = true), + SqlField("name", cTable.name) + ) + ) +} diff --git a/modules/sql-core/src/test/scala/SqlNullableParentSuite.scala b/modules/sql-core/src/test/scala/SqlNullableParentSuite.scala new file mode 100644 index 00000000..e027c833 --- /dev/null +++ b/modules/sql-core/src/test/scala/SqlNullableParentSuite.scala @@ -0,0 +1,126 @@ +// Copyright (c) 2016-2025 Association of Universities for Research in Astronomy, Inc. (AURA) +// Copyright (c) 2016-2025 Grackle Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package grackle.sql.test + +import cats.effect.IO +import io.circe.literal._ +import munit.CatsEffectSuite + +import grackle._ +import grackle.test.GraphQLResponseTests.assertWeaklyEqualIO + +/** + * A non-null field beneath a nullable one must not remove rows whose nullable field is absent. + * + * Both joins are individually correct — the nullable field's is a LEFT JOIN, the non-null + * field's an INNER JOIN — but flattening them into one chain lets the INNER JOIN eliminate the + * rows the LEFT JOIN null-padded. The non-null-ness of `c` only constrains anything when a `B` + * exists at all: per the GraphQL spec, completing a nullable field with a null result returns + * null without executing its sub-selections. + */ +trait SqlNullableParentSuite extends CatsEffectSuite { + def mapping: Mapping[IO] + + test("a nullable field that is absent does not remove its row") { + val query = """ + query { + as { + name + b { + name + c { + name + } + } + } + } + """ + + // `a-with-dangling-b` names a `B` which doesn't exist, so its row is reported with a null + // `b` rather than as an error. That isn't what the spec asks for — a non-null field with no + // row should raise an execution error propagated to the nearest nullable ancestor — but it + // is a separate defect from the one under test here, and returning the row is already an + // improvement on dropping it. Note that expecting `data` alone also expects no `errors` + // entry, so this has to be revisited when that defect is addressed. + val expected = json""" + { + "data" : { + "as" : [ + { + "name" : "a-with-good-b", + "b" : { + "name" : "b-with-c", + "c" : { + "name" : "cat-1" + } + } + }, + { + "name" : "a-with-dangling-b", + "b" : null + }, + { + "name" : "a-without-b", + "b" : null + } + ] + } + } + """ + + assertWeaklyEqualIO(mapping.compileAndRun(query), expected) + } + + test("the same query stopping above the non-null field is unaffected") { + val query = """ + query { + as { + name + b { + name + } + } + } + """ + + val expected = json""" + { + "data" : { + "as" : [ + { + "name" : "a-with-good-b", + "b" : { + "name" : "b-with-c" + } + }, + { + "name" : "a-with-dangling-b", + "b" : { + "name" : "b-with-dangling-c" + } + }, + { + "name" : "a-without-b", + "b" : null + } + ] + } + } + """ + + assertWeaklyEqualIO(mapping.compileAndRun(query), expected) + } +} diff --git a/testdata/mssql/nullable-parent.sql b/testdata/mssql/nullable-parent.sql new file mode 100644 index 00000000..9d438e84 --- /dev/null +++ b/testdata/mssql/nullable-parent.sql @@ -0,0 +1,30 @@ +CREATE TABLE nullable_parent_c ( + id INTEGER PRIMARY KEY, + name VARCHAR(64) NOT NULL +); + +CREATE TABLE nullable_parent_b ( + id INTEGER PRIMARY KEY, + c_id INTEGER NOT NULL, + name VARCHAR(64) NOT NULL +); + +CREATE TABLE nullable_parent_a ( + id INTEGER PRIMARY KEY, + b_id INTEGER, + name VARCHAR(64) NOT NULL +); + +INSERT INTO nullable_parent_c (id, name) VALUES +(1, 'cat-1'); + +INSERT INTO nullable_parent_b (id, c_id, name) VALUES +(10, 1, 'b-with-c'), +(20, 999, 'b-with-dangling-c'); + +INSERT INTO nullable_parent_a (id, b_id, name) VALUES +(100, 10, 'a-with-good-b'), +(200, 20, 'a-with-dangling-b'), +(300, NULL, 'a-without-b'); + +GO diff --git a/testdata/oracle/nullable-parent.sql b/testdata/oracle/nullable-parent.sql new file mode 100644 index 00000000..02511cbc --- /dev/null +++ b/testdata/oracle/nullable-parent.sql @@ -0,0 +1,28 @@ +CREATE TABLE nullable_parent_c ( + id INTEGER PRIMARY KEY, + name VARCHAR(64) NOT NULL +); + +CREATE TABLE nullable_parent_b ( + id INTEGER PRIMARY KEY, + c_id INTEGER NOT NULL, + name VARCHAR(64) NOT NULL +); + +CREATE TABLE nullable_parent_a ( + id INTEGER PRIMARY KEY, + b_id INTEGER, + name VARCHAR(64) NOT NULL +); + +INSERT INTO nullable_parent_c (id, name) VALUES +(1, 'cat-1'); + +INSERT INTO nullable_parent_b (id, c_id, name) VALUES +(10, 1, 'b-with-c'), +(20, 999, 'b-with-dangling-c'); + +INSERT INTO nullable_parent_a (id, b_id, name) VALUES +(100, 10, 'a-with-good-b'), +(200, 20, 'a-with-dangling-b'), +(300, NULL, 'a-without-b'); diff --git a/testdata/pg/nullable-parent.sql b/testdata/pg/nullable-parent.sql new file mode 100644 index 00000000..8b3c3fba --- /dev/null +++ b/testdata/pg/nullable-parent.sql @@ -0,0 +1,31 @@ +CREATE TABLE nullable_parent_c ( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL +); + +CREATE TABLE nullable_parent_b ( + id INTEGER PRIMARY KEY, + c_id INTEGER NOT NULL, + name TEXT NOT NULL +); + +CREATE TABLE nullable_parent_a ( + id INTEGER PRIMARY KEY, + b_id INTEGER, + name TEXT NOT NULL +); + +COPY nullable_parent_c (id, name) FROM STDIN WITH DELIMITER '|'; +1|cat-1 +\. + +COPY nullable_parent_b (id, c_id, name) FROM STDIN WITH DELIMITER '|'; +10|1|b-with-c +20|999|b-with-dangling-c +\. + +COPY nullable_parent_a (id, b_id, name) FROM STDIN WITH DELIMITER '|'; +100|10|a-with-good-b +200|20|a-with-dangling-b +300|\N|a-without-b +\. From 775b3d283506fd71f0b80b0c06218ae114594ac9 Mon Sep 17 00:00:00 2001 From: phdoerfler Date: Fri, 7 Aug 2026 14:21:09 +0200 Subject: [PATCH 6/8] Extend the nullable parent suite to a non-null field under a list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flattening bug turns on the attaching join being LEFT rather than INNER, and SqlSelect.nest makes that choice for lists exactly as it does for nullable fields: `inner` is false for both. Only the nullable half was covered, yet the list half is the shape more schemas will meet, since a list beneath which something is non-null needs no nullable field anywhere for its rows to be eliminated. Extends the shared fixture with a second, one-to-many group of tables — a `D` with `es: [E!]!`, an `E` with a non-null `f` — including a `D` with no `E`s at all. Before the fix that `D` disappeared from the result instead of being returned with `es` as `[]`; neutralising the fix makes the new test fail, so it does pin down the behaviour it claims to. --- .../test/scala/SqlNullableParentMapping.scala | 51 ++++++++++++++- .../test/scala/SqlNullableParentSuite.scala | 62 +++++++++++++++++-- testdata/mssql/nullable-parent.sql | 29 +++++++++ testdata/oracle/nullable-parent.sql | 29 +++++++++ testdata/pg/nullable-parent.sql | 32 ++++++++++ 5 files changed, 196 insertions(+), 7 deletions(-) diff --git a/modules/sql-core/src/test/scala/SqlNullableParentMapping.scala b/modules/sql-core/src/test/scala/SqlNullableParentMapping.scala index 7550a299..81db2251 100644 --- a/modules/sql-core/src/test/scala/SqlNullableParentMapping.scala +++ b/modules/sql-core/src/test/scala/SqlNullableParentMapping.scala @@ -36,10 +36,28 @@ trait SqlNullableParentMapping[F[_]] extends SqlTestMapping[F] { val name = col("name", text) } + object dTable extends TableDef("nullable_parent_d") { + val id = col("id", int4) + val name = col("name", text) + } + + object eTable extends TableDef("nullable_parent_e") { + val id = col("id", int4) + val dId = col("d_id", int4) + val fId = col("f_id", int4) + val name = col("name", text) + } + + object fTable extends TableDef("nullable_parent_f") { + val id = col("id", int4) + val name = col("name", text) + } + val schema = schema""" type Query { as: [A!]! + ds: [D!]! } type A { name: String! @@ -52,17 +70,32 @@ trait SqlNullableParentMapping[F[_]] extends SqlTestMapping[F] { type C { name: String! } + type D { + name: String! + es: [E!]! + } + type E { + name: String! + f: F! + } + type F { + name: String! + } """ val QueryType = schema.ref("Query") val AType = schema.ref("A") val BType = schema.ref("B") val CType = schema.ref("C") + val DType = schema.ref("D") + val EType = schema.ref("E") + val FType = schema.ref("F") val typeMappings = TypeMappings( ObjectMapping(QueryType)( - SqlObject("as") + SqlObject("as"), + SqlObject("ds") ), ObjectMapping(AType)( SqlField("id", aTable.id, key = true, hidden = true), @@ -79,6 +112,22 @@ trait SqlNullableParentMapping[F[_]] extends SqlTestMapping[F] { ObjectMapping(CType)( SqlField("id", cTable.id, key = true, hidden = true), SqlField("name", cTable.name) + ), + ObjectMapping(DType)( + SqlField("id", dTable.id, key = true, hidden = true), + SqlField("name", dTable.name), + SqlObject("es", Join(dTable.id, eTable.dId)) + ), + ObjectMapping(EType)( + SqlField("id", eTable.id, key = true, hidden = true), + SqlField("dId", eTable.dId, hidden = true), + SqlField("fId", eTable.fId, hidden = true), + SqlField("name", eTable.name), + SqlObject("f", Join(eTable.fId, fTable.id)) + ), + ObjectMapping(FType)( + SqlField("id", fTable.id, key = true, hidden = true), + SqlField("name", fTable.name) ) ) } diff --git a/modules/sql-core/src/test/scala/SqlNullableParentSuite.scala b/modules/sql-core/src/test/scala/SqlNullableParentSuite.scala index e027c833..61f0e6c1 100644 --- a/modules/sql-core/src/test/scala/SqlNullableParentSuite.scala +++ b/modules/sql-core/src/test/scala/SqlNullableParentSuite.scala @@ -23,13 +23,13 @@ import grackle._ import grackle.test.GraphQLResponseTests.assertWeaklyEqualIO /** - * A non-null field beneath a nullable one must not remove rows whose nullable field is absent. + * A non-null field beneath a nullable one, or beneath a list, must not remove rows whose parent + * is absent or empty. * - * Both joins are individually correct — the nullable field's is a LEFT JOIN, the non-null - * field's an INNER JOIN — but flattening them into one chain lets the INNER JOIN eliminate the - * rows the LEFT JOIN null-padded. The non-null-ness of `c` only constrains anything when a `B` - * exists at all: per the GraphQL spec, completing a nullable field with a null result returns - * null without executing its sub-selections. + * The non-null-ness of such a field only constrains anything where its parent exists at all: + * per the GraphQL spec, completing a nullable field with a null result returns null without + * executing its sub-selections, and a `D` with no `E`s must still be returned with `es` as + * `[]`. */ trait SqlNullableParentSuite extends CatsEffectSuite { def mapping: Mapping[IO] @@ -123,4 +123,54 @@ trait SqlNullableParentSuite extends CatsEffectSuite { assertWeaklyEqualIO(mapping.compileAndRun(query), expected) } + + test("a list field that is empty does not remove its row") { + val query = """ + query { + ds { + name + es { + name + f { + name + } + } + } + } + """ + + // `d-without-es` is the row at issue: it has no `E`s, so nothing beneath `es` is selected at + // all, and the non-null-ness of `f` cannot bear on whether the `D` itself is returned. + val expected = json""" + { + "data" : { + "ds" : [ + { + "name" : "d-with-es", + "es" : [ + { + "name" : "e-with-f", + "f" : { + "name" : "fish-1" + } + }, + { + "name" : "e-with-another-f", + "f" : { + "name" : "fish-2" + } + } + ] + }, + { + "name" : "d-without-es", + "es" : [] + } + ] + } + } + """ + + assertWeaklyEqualIO(mapping.compileAndRun(query), expected) + } } diff --git a/testdata/mssql/nullable-parent.sql b/testdata/mssql/nullable-parent.sql index 9d438e84..27e2d64b 100644 --- a/testdata/mssql/nullable-parent.sql +++ b/testdata/mssql/nullable-parent.sql @@ -27,4 +27,33 @@ INSERT INTO nullable_parent_a (id, b_id, name) VALUES (200, 20, 'a-with-dangling-b'), (300, NULL, 'a-without-b'); +CREATE TABLE nullable_parent_f ( + id INTEGER PRIMARY KEY, + name VARCHAR(64) NOT NULL +); + +CREATE TABLE nullable_parent_e ( + id INTEGER PRIMARY KEY, + d_id INTEGER NOT NULL, + f_id INTEGER NOT NULL, + name VARCHAR(64) NOT NULL +); + +CREATE TABLE nullable_parent_d ( + id INTEGER PRIMARY KEY, + name VARCHAR(64) NOT NULL +); + +INSERT INTO nullable_parent_f (id, name) VALUES +(1, 'fish-1'), +(2, 'fish-2'); + +INSERT INTO nullable_parent_e (id, d_id, f_id, name) VALUES +(10, 100, 1, 'e-with-f'), +(11, 100, 2, 'e-with-another-f'); + +INSERT INTO nullable_parent_d (id, name) VALUES +(100, 'd-with-es'), +(200, 'd-without-es'); + GO diff --git a/testdata/oracle/nullable-parent.sql b/testdata/oracle/nullable-parent.sql index 02511cbc..4124bcd4 100644 --- a/testdata/oracle/nullable-parent.sql +++ b/testdata/oracle/nullable-parent.sql @@ -26,3 +26,32 @@ INSERT INTO nullable_parent_a (id, b_id, name) VALUES (100, 10, 'a-with-good-b'), (200, 20, 'a-with-dangling-b'), (300, NULL, 'a-without-b'); + +CREATE TABLE nullable_parent_f ( + id INTEGER PRIMARY KEY, + name VARCHAR(64) NOT NULL +); + +CREATE TABLE nullable_parent_e ( + id INTEGER PRIMARY KEY, + d_id INTEGER NOT NULL, + f_id INTEGER NOT NULL, + name VARCHAR(64) NOT NULL +); + +CREATE TABLE nullable_parent_d ( + id INTEGER PRIMARY KEY, + name VARCHAR(64) NOT NULL +); + +INSERT INTO nullable_parent_f (id, name) VALUES +(1, 'fish-1'), +(2, 'fish-2'); + +INSERT INTO nullable_parent_e (id, d_id, f_id, name) VALUES +(10, 100, 1, 'e-with-f'), +(11, 100, 2, 'e-with-another-f'); + +INSERT INTO nullable_parent_d (id, name) VALUES +(100, 'd-with-es'), +(200, 'd-without-es'); diff --git a/testdata/pg/nullable-parent.sql b/testdata/pg/nullable-parent.sql index 8b3c3fba..37b3a009 100644 --- a/testdata/pg/nullable-parent.sql +++ b/testdata/pg/nullable-parent.sql @@ -29,3 +29,35 @@ COPY nullable_parent_a (id, b_id, name) FROM STDIN WITH DELIMITER '|'; 200|20|a-with-dangling-b 300|\N|a-without-b \. + +CREATE TABLE nullable_parent_f ( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL +); + +CREATE TABLE nullable_parent_e ( + id INTEGER PRIMARY KEY, + d_id INTEGER NOT NULL, + f_id INTEGER NOT NULL, + name TEXT NOT NULL +); + +CREATE TABLE nullable_parent_d ( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL +); + +COPY nullable_parent_f (id, name) FROM STDIN WITH DELIMITER '|'; +1|fish-1 +2|fish-2 +\. + +COPY nullable_parent_e (id, d_id, f_id, name) FROM STDIN WITH DELIMITER '|'; +10|100|1|e-with-f +11|100|2|e-with-another-f +\. + +COPY nullable_parent_d (id, name) FROM STDIN WITH DELIMITER '|'; +100|d-with-es +200|d-without-es +\. From 72fbeb08b827bee1f9c55fbff32bc24aff460616 Mon Sep 17 00:00:00 2001 From: phdoerfler Date: Sat, 8 Aug 2026 01:07:10 +0200 Subject: [PATCH 7/8] Don't re-lift the conditions of an already correlated OUTER APPLY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nesting a select re-examines joins which an earlier nesting already correlated. `correlate` didn't recognise the wrapper it had built for such a join, declined, and the caller fell back to lifting the conditions into the enclosing select — which for an OUTER APPLY discards the null padded rows the join produced, undoing the correlation and losing exactly the rows it was there to keep. Recognise that wrapper by the `correlated` flag it carries and yield the join unchanged. It is recognised only while it still holds its conditions, so one which had somehow lost them falls back to lifting rather than being taken for correlated with nothing to apply. With the wrapper recognised, no OUTER APPLY reachable by `nest` declines, so `distributeJoinConditions` now treats a failure to correlate as a bug rather than silently lifting into a wrong result. --- .../sql-core/src/main/scala/SqlMapping.scala | 57 ++++++++++--------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/modules/sql-core/src/main/scala/SqlMapping.scala b/modules/sql-core/src/main/scala/SqlMapping.scala index 91124380..a06a39c1 100644 --- a/modules/sql-core/src/main/scala/SqlMapping.scala +++ b/modules/sql-core/src/main/scala/SqlMapping.scala @@ -1427,30 +1427,25 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self /** * An `APPLY` join is rendered without an `ON` clause, so its conditions have to be - * expressed elsewhere. + * expressed elsewhere. For `CROSS APPLY` they are lifted into the enclosing select's + * WHERE clause, which for an inner join is equivalent to an `ON` clause. * - * For `CROSS APPLY` they are lifted into the enclosing select's WHERE clause, which for - * an inner join is equivalent to an `ON` clause. - * - * For `OUTER APPLY` that would be wrong: the WHERE clause is applied after the join and - * would discard precisely the null padded rows the outer join produced. Instead the - * subquery is wrapped in a correlating select which applies the conditions to its - * result, referring to the enclosing select's tables. That is legal because `APPLY` is - * lateral, and is what makes `OUTER APPLY` equivalent to `LEFT JOIN LATERAL`. - * - * Only a subquery of the right shape can be correlated, and the conditions are lifted - * as a last resort otherwise. For an `OUTER APPLY` that reinstates the semantics - * described above, so it has to stay unreachable: the shapes `correlate` declines are - * those `addFilterOrderByOffsetLimit` builds, and those join on a predicate, which is - * short circuited before it. + * For `OUTER APPLY` lifting them would discard precisely the null padded rows the outer + * join produced, the WHERE clause being applied after the join. Its subquery is instead + * wrapped in a correlating select, which is legal because `APPLY` is lateral and is + * what makes `OUTER APPLY` equivalent to `LEFT JOIN LATERAL`. A subquery that can't be + * correlated has no correct rendering, so `correlate` failing is treated as a bug. */ def distributeJoinConditions( join: SqlJoin, subquery: SubqueryRef): (SqlJoin, List[Predicate]) = if (inner || join.isPredicate) (join, liftedPredicates(join)) else - correlate(join, subquery).fold((join, liftedPredicates(join)))(join0 => - (join0, Nil)) + // Every OUTER APPLY reachable here can be correlated; a None is a bug, not a query. + correlate(join, subquery) + .map((_, Nil)) + .getOrElse(throw new SqlMappingException( + s"OUTER APPLY subquery '${subquery.name}' could not be correlated")) /** * The join's conditions expressed as predicates of the enclosing select. @@ -1461,22 +1456,22 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self /** * Yields a copy of `join` with its conditions moved inside its subquery. * - * The subquery is wrapped in a select which applies the conditions to the subquery's - * result, so that any LIMIT, OFFSET, DISTINCT or window function in the subquery is - * evaluated first, exactly as it would be were the conditions in the ON clause of a - * `LEFT JOIN LATERAL`. Wrapping also ensures that only the subquery's exposed columns - * are in scope where the conditions are applied, so that a table of the enclosing - * select can't be shadowed by a same named table within the subquery. + * The subquery is wrapped in a select which applies the conditions to its result, so + * that any LIMIT, OFFSET or DISTINCT within is evaluated first, as it would be were + * they in the ON clause of a `LEFT JOIN LATERAL`, and so that only the subquery's + * exposed columns are in scope where they are applied — a table of the enclosing select + * can otherwise be shadowed by a same named table within. * - * The subquery side of each condition names a column exposed by the subquery, which has - * to be mapped to the corresponding column of the wrapper. Yields `None` if that isn't - * possible, in which case the caller falls back to lifting the conditions into the - * enclosing select. + * Yields `None` only for a subquery shape it doesn't handle; no `OUTER APPLY` produces + * one, so the caller treats a `None` as a bug rather than lifting. A subquery already + * such a wrapper is yielded unchanged, so re-nesting doesn't correlate it twice. */ private def correlate(join: SqlJoin, subquery: SubqueryRef): Option[SqlJoin] = subquery.subquery match { case sel: SqlSelect if sel.withs.isEmpty => sel.table match { + case wrapped: SubqueryRef if wrapped.correlated && sel.wheres.nonEmpty => + Some(join) case table: TableRef => val exposed = join.on.traverse { @@ -1484,7 +1479,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self } exposed.map { exposed0 => val wrapper = - sel.toSubquery(subquery.name + "_corr", NotLateral, correlated = true) + sel.toSubquery(correlationName(subquery), NotLateral, correlated = true) val wheres = exposed0.map { case (p, c) => Eql(p.toTerm, c.derive(wrapper.table).toTerm) @@ -1495,6 +1490,12 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self } case _ => None } + + /** + * The alias given to the correlating select `correlate` wraps around `subquery`. Only + * cosmetic: the wrapper is recognised by its `correlated` flag, not by this name. + */ + private def correlationName(subquery: SubqueryRef): String = subquery.name + "_corr" } } From 8e617460b5e7ed30dfbcd24c34fd5009084306a0 Mon Sep 17 00:00:00 2001 From: phdoerfler Date: Sat, 8 Aug 2026 01:07:14 +0200 Subject: [PATCH 8/8] Test an empty list reached through a back reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Descends two lists through a non-null field, which nests the same shape twice and so presents the inner join for correlation a second time. `E` gains a second reference to `D` so that the second list can be empty where the first is not — a shape the World fixture can't express, since a country reached through a city always has that city. --- .../test/scala/SqlNullableParentMapping.scala | 6 +- .../test/scala/SqlNullableParentSuite.scala | 69 +++++++++++++++++++ testdata/mssql/nullable-parent.sql | 7 +- testdata/oracle/nullable-parent.sql | 7 +- testdata/pg/nullable-parent.sql | 7 +- 5 files changed, 86 insertions(+), 10 deletions(-) diff --git a/modules/sql-core/src/test/scala/SqlNullableParentMapping.scala b/modules/sql-core/src/test/scala/SqlNullableParentMapping.scala index 81db2251..983f8a41 100644 --- a/modules/sql-core/src/test/scala/SqlNullableParentMapping.scala +++ b/modules/sql-core/src/test/scala/SqlNullableParentMapping.scala @@ -45,6 +45,7 @@ trait SqlNullableParentMapping[F[_]] extends SqlTestMapping[F] { val id = col("id", int4) val dId = col("d_id", int4) val fId = col("f_id", int4) + val otherDId = col("other_d_id", int4) val name = col("name", text) } @@ -77,6 +78,7 @@ trait SqlNullableParentMapping[F[_]] extends SqlTestMapping[F] { type E { name: String! f: F! + otherD: D! } type F { name: String! @@ -122,8 +124,10 @@ trait SqlNullableParentMapping[F[_]] extends SqlTestMapping[F] { SqlField("id", eTable.id, key = true, hidden = true), SqlField("dId", eTable.dId, hidden = true), SqlField("fId", eTable.fId, hidden = true), + SqlField("otherDId", eTable.otherDId, hidden = true), SqlField("name", eTable.name), - SqlObject("f", Join(eTable.fId, fTable.id)) + SqlObject("f", Join(eTable.fId, fTable.id)), + SqlObject("otherD", Join(eTable.otherDId, dTable.id)) ), ObjectMapping(FType)( SqlField("id", fTable.id, key = true, hidden = true), diff --git a/modules/sql-core/src/test/scala/SqlNullableParentSuite.scala b/modules/sql-core/src/test/scala/SqlNullableParentSuite.scala index 61f0e6c1..29348575 100644 --- a/modules/sql-core/src/test/scala/SqlNullableParentSuite.scala +++ b/modules/sql-core/src/test/scala/SqlNullableParentSuite.scala @@ -173,4 +173,73 @@ trait SqlNullableParentSuite extends CatsEffectSuite { assertWeaklyEqualIO(mapping.compileAndRun(query), expected) } + + // Reaching a second list through a non-null field nests the same shape twice, which on a + // backend joining by `APPLY` presents the inner join to correlation a second time, once the + // first has already rewritten it. `e-with-f` is the row at issue: its `otherD` is the `D` + // with no `E`s, so only a query which descends that far has a row to lose. + test("an empty list reached through a back reference does not remove its row") { + val query = """ + query { + ds { + name + es { + name + otherD { + name + es { + name + f { + name + } + } + } + } + } + } + """ + + val expected = json""" + { + "data" : { + "ds" : [ + { + "name" : "d-with-es", + "es" : [ + { + "name" : "e-with-f", + "otherD" : { + "name" : "d-without-es", + "es" : [] + } + }, + { + "name" : "e-with-another-f", + "otherD" : { + "name" : "d-with-es", + "es" : [ + { + "name" : "e-with-f", + "f" : { "name" : "fish-1" } + }, + { + "name" : "e-with-another-f", + "f" : { "name" : "fish-2" } + } + ] + } + } + ] + }, + { + "name" : "d-without-es", + "es" : [] + } + ] + } + } + """ + + assertWeaklyEqualIO(mapping.compileAndRun(query), expected) + } } diff --git a/testdata/mssql/nullable-parent.sql b/testdata/mssql/nullable-parent.sql index 27e2d64b..38de5a79 100644 --- a/testdata/mssql/nullable-parent.sql +++ b/testdata/mssql/nullable-parent.sql @@ -36,6 +36,7 @@ CREATE TABLE nullable_parent_e ( id INTEGER PRIMARY KEY, d_id INTEGER NOT NULL, f_id INTEGER NOT NULL, + other_d_id INTEGER NOT NULL, name VARCHAR(64) NOT NULL ); @@ -48,9 +49,9 @@ INSERT INTO nullable_parent_f (id, name) VALUES (1, 'fish-1'), (2, 'fish-2'); -INSERT INTO nullable_parent_e (id, d_id, f_id, name) VALUES -(10, 100, 1, 'e-with-f'), -(11, 100, 2, 'e-with-another-f'); +INSERT INTO nullable_parent_e (id, d_id, f_id, other_d_id, name) VALUES +(10, 100, 1, 200, 'e-with-f'), +(11, 100, 2, 100, 'e-with-another-f'); INSERT INTO nullable_parent_d (id, name) VALUES (100, 'd-with-es'), diff --git a/testdata/oracle/nullable-parent.sql b/testdata/oracle/nullable-parent.sql index 4124bcd4..fbc89990 100644 --- a/testdata/oracle/nullable-parent.sql +++ b/testdata/oracle/nullable-parent.sql @@ -36,6 +36,7 @@ CREATE TABLE nullable_parent_e ( id INTEGER PRIMARY KEY, d_id INTEGER NOT NULL, f_id INTEGER NOT NULL, + other_d_id INTEGER NOT NULL, name VARCHAR(64) NOT NULL ); @@ -48,9 +49,9 @@ INSERT INTO nullable_parent_f (id, name) VALUES (1, 'fish-1'), (2, 'fish-2'); -INSERT INTO nullable_parent_e (id, d_id, f_id, name) VALUES -(10, 100, 1, 'e-with-f'), -(11, 100, 2, 'e-with-another-f'); +INSERT INTO nullable_parent_e (id, d_id, f_id, other_d_id, name) VALUES +(10, 100, 1, 200, 'e-with-f'), +(11, 100, 2, 100, 'e-with-another-f'); INSERT INTO nullable_parent_d (id, name) VALUES (100, 'd-with-es'), diff --git a/testdata/pg/nullable-parent.sql b/testdata/pg/nullable-parent.sql index 37b3a009..5222355a 100644 --- a/testdata/pg/nullable-parent.sql +++ b/testdata/pg/nullable-parent.sql @@ -39,6 +39,7 @@ CREATE TABLE nullable_parent_e ( id INTEGER PRIMARY KEY, d_id INTEGER NOT NULL, f_id INTEGER NOT NULL, + other_d_id INTEGER NOT NULL, name TEXT NOT NULL ); @@ -52,9 +53,9 @@ COPY nullable_parent_f (id, name) FROM STDIN WITH DELIMITER '|'; 2|fish-2 \. -COPY nullable_parent_e (id, d_id, f_id, name) FROM STDIN WITH DELIMITER '|'; -10|100|1|e-with-f -11|100|2|e-with-another-f +COPY nullable_parent_e (id, d_id, f_id, other_d_id, name) FROM STDIN WITH DELIMITER '|'; +10|100|1|200|e-with-f +11|100|2|100|e-with-another-f \. COPY nullable_parent_d (id, name) FROM STDIN WITH DELIMITER '|';