diff --git a/build.sbt b/build.sbt index 107d4b10..610bc591 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.30" +ThisBuild / tlBaseVersion := "0.31" ThisBuild / startYear := Some(2019) ThisBuild / licenses := Seq(License.Apache2) ThisBuild / developers := List( diff --git a/modules/doobie-core/src/main/scala/DoobieMapping.scala b/modules/doobie-core/src/main/scala/DoobieMapping.scala index 597d0e38..704ba092 100644 --- a/modules/doobie-core/src/main/scala/DoobieMapping.scala +++ b/modules/doobie-core/src/main/scala/DoobieMapping.scala @@ -58,7 +58,7 @@ trait DoobieMappingLike[F[_]] extends Mapping[F] with SqlMappingLike[F] { implicit tableName: TableName, typeName: TypeName[T], pos: SourcePos): ColumnRef = - ColumnRef(tableName.name, colName, (codec, nullable), typeName.value, pos) + ColumnRef(tableName, colName, (codec, nullable), typeName.value, pos) implicit def Fragments: SqlFragment[Fragment] = new SqlFragment[Fragment] { diff --git a/modules/doobie-mssql/src/main/scala/DoobieMSSqlMapping.scala b/modules/doobie-mssql/src/main/scala/DoobieMSSqlMapping.scala index 3b839361..726da148 100644 --- a/modules/doobie-mssql/src/main/scala/DoobieMSSqlMapping.scala +++ b/modules/doobie-mssql/src/main/scala/DoobieMSSqlMapping.scala @@ -88,7 +88,10 @@ trait DoobieMSSqlMappingLike[F[_]] extends DoobieMappingLike[F] with SqlMappingL def encapsulateUnionBranch(s: SqlSelect): SqlSelect = if (s.orders.isEmpty) s - else s.toSubquery(s.table.name + "_encaps", Laterality.NotLateral) + else + // The subquery name lands in alias position, so a schema-qualified table name must be + // folded to a bare identifier first (issue #342). + s.toSubquery(s.table.identifier + "_encaps", Laterality.NotLateral) def mkLateral(inner: Boolean): Laterality = Laterality.Apply(inner) diff --git a/modules/doobie-pg/src/test/scala/DoobiePgSuites.scala b/modules/doobie-pg/src/test/scala/DoobiePgSuites.scala index 470ac7c7..f07368d4 100644 --- a/modules/doobie-pg/src/test/scala/DoobiePgSuites.scala +++ b/modules/doobie-pg/src/test/scala/DoobiePgSuites.scala @@ -213,6 +213,14 @@ final class ProjectionSuite extends DoobiePgDatabaseSuite with SqlProjectionSuit lazy val mapping = new DoobiePgTestMapping(transactor) with SqlProjectionMapping[IO] } +final class QualifiedNamesSuite extends DoobiePgDatabaseSuite with SqlQualifiedNamesSuite { + lazy val mapping = new DoobiePgTestMapping(transactor) with SqlQualifiedNamesMapping[IO] +} + +final class TableNameSuite extends DoobiePgDatabaseSuite with SqlTableNameSuite { + lazy val mapping = new DoobiePgTestMapping(transactor) with SqlQualifiedNamesMapping[IO] +} + final class RecursiveInterfacesSuite extends DoobiePgDatabaseSuite with SqlRecursiveInterfacesSuite { diff --git a/modules/skunk/js-jvm/src/test/scala/SkunkSuites.scala b/modules/skunk/js-jvm/src/test/scala/SkunkSuites.scala index 250f3abe..d6a1a63a 100644 --- a/modules/skunk/js-jvm/src/test/scala/SkunkSuites.scala +++ b/modules/skunk/js-jvm/src/test/scala/SkunkSuites.scala @@ -218,6 +218,14 @@ final class ProjectionSuite extends SkunkDatabaseSuite with SqlProjectionSuite { lazy val mapping = new SkunkTestMapping(pool) with SqlProjectionMapping[IO] } +final class QualifiedNamesSuite extends SkunkDatabaseSuite with SqlQualifiedNamesSuite { + lazy val mapping = new SkunkTestMapping(pool) with SqlQualifiedNamesMapping[IO] +} + +final class TableNameSuite extends SkunkDatabaseSuite with SqlTableNameSuite { + lazy val mapping = new SkunkTestMapping(pool) with SqlQualifiedNamesMapping[IO] +} + final class RecursiveInterfacesSuite extends SkunkDatabaseSuite with SqlRecursiveInterfacesSuite { diff --git a/modules/skunk/shared/src/main/scala/SkunkMapping.scala b/modules/skunk/shared/src/main/scala/SkunkMapping.scala index 7463f8fc..c37db74b 100644 --- a/modules/skunk/shared/src/main/scala/SkunkMapping.scala +++ b/modules/skunk/shared/src/main/scala/SkunkMapping.scala @@ -90,7 +90,7 @@ trait SkunkMappingLike[F[_]] extends Mapping[F] with SqlPgMappingLike[F] { outer typeName: NullableTypeName[T], isNullable: IsNullable[T], pos: SourcePos): ColumnRef = - ColumnRef(tableName.name, colName, (codec, isNullable.isNullable), typeName.value, pos) + ColumnRef(tableName, colName, (codec, isNullable.isNullable), typeName.value, pos) // We need to demonstrate that our `Fragment` type has certain compositional properties. implicit def Fragments: SqlFragment[AppliedFragment] = diff --git a/modules/sql-core/src/main/scala/SqlMapping.scala b/modules/sql-core/src/main/scala/SqlMapping.scala index de7cc72e..0fc2fe17 100644 --- a/modules/sql-core/src/main/scala/SqlMapping.scala +++ b/modules/sql-core/src/main/scala/SqlMapping.scala @@ -65,11 +65,43 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self def orderToFragment(col: Fragment, ascending: Boolean, nullsLast: Boolean): Fragment def nullsHigh: Boolean - case class TableName(name: String) + /** + * The name of a SQL table, split into an optional schema qualifier and a local name. + * + * A table's raw SQL name plays two distinct roles depending on where it's used: a reference + * (`sqlRef`, schema-qualified, e.g. "public.country" — legal in a FROM/JOIN clause) and a + * bare identifier (`identifier`, e.g. "public_country" — required wherever an alias or + * synthesized name is minted, since a dot is not a legal identifier character). Keeping both + * derived from one structured value means a call site that needs the identifier form reaches + * for `.identifier` and can't accidentally reach for the raw, possibly-dotted `.sqlRef` + * instead (issue #342). + */ + case class TableName(schema: Option[String], name: String) { + def sqlRef: String = schema.fold(name)(s => s"$s.$name") + def identifier: String = TableName.foldToIdentifier(sqlRef) + override def toString: String = sqlRef + } object TableName { + def apply(raw: String): TableName = + raw.lastIndexOf('.') match { + case -1 => TableName(None, raw) + case i => + val (schema, dotName) = raw.splitAt(i) + TableName(Some(schema), dotName.tail) + } + + /** + * Fold a raw SQL name into a bare identifier. + * + * `TableName.identifier` covers names that are still structured. Subquery and common table + * expression names reach an alias slot as plain strings, composed from whatever they were + * synthesized over, so they need the same fold applied to the composed result. + */ + def foldToIdentifier(name: String): String = name.replace('.', '_') + val rootName = "" - val rootTableName = TableName(rootName) - def isRoot(table: String): Boolean = table == rootName + val rootTableName = TableName(None, rootName) + def isRoot(table: TableName): Boolean = table == rootTableName } class TableDef(name: String) { implicit val tableName: TableName = TableName(name) @@ -88,7 +120,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self * used to construct `SqlColumns`. */ case class ColumnRef( - table: String, + table: TableName, column: String, codec: Codec, scalaTypeName: String, @@ -144,8 +176,17 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self tableAliases.get((table.context.resultPath, table.name)) match { case Some(alias) => (this, alias) case None => - if (seenTables(table.name)) { - val alias = s"${table.name}_alias_$next" + // An alias must be a bare identifier, so a qualified name like "public.country" + // can never stand as its own alias (issue #342). Fold first and collision-check the + // folded form, since folding can land on a real table of that name; uniqueness is + // then preserved by the counter, which is shared across table and column aliases. + // + // Note that the map stays keyed by the unfolded name. That is what lets a table and + // a subquery synthesized over it — which share a name in order to share an identity + // — also share an alias, so columns of either render the same prefix. + val identifier = table.identifier + if (seenTables(identifier)) { + val alias = s"${identifier}_alias_$next" val newState = copy( next = next + 1, @@ -155,11 +196,11 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self } else { val newState = copy( - seenTables = seenTables + table.name, + seenTables = seenTables + identifier, tableAliases = - tableAliases + ((table.context.resultPath, table.name) -> table.name) + tableAliases + ((table.context.resultPath, table.name) -> identifier) ) - (newState, table.name) + (newState, identifier) } } @@ -169,7 +210,13 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self def tableRef(table: TableExpr): (AliasState, String) = tableAliases.get((table.context.resultPath, table.name)) match { case Some(alias) => (this, alias) - case None => (this, table.name) + // No alias is registered under this result path when the reference is correlated to a + // table defined by an enclosing query, which registers under its own path. Falling back + // to the bare identifier is what reuses that enclosing alias; registering a fresh one + // here instead would name a table absent from the from clause. For a schema-qualified + // table the raw name is a dotted reference, which no longer matches the folded alias + // the enclosing query defined (issue #342). + case None => (this, table.identifier) } /** @@ -1309,6 +1356,12 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self */ def name: String + /** + * A bare-identifier-safe form of this `TableExpr`'s name, for use in alias and synthesized + * subquery name positions where a dot is not legal (issue #342). + */ + def identifier: String + /** * Is the supplied column an immediate component of this `TableExpr`? */ @@ -1355,14 +1408,17 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self /** * Table expression corresponding to a possibly aliased table */ - case class TableRef(context: Context, name: String) extends TableExpr { + case class TableRef(context: Context, tableName: TableName) extends TableExpr { + def name: String = tableName.sqlRef + def identifier: String = tableName.identifier + def owns(col: SqlColumn): Boolean = isSameOwner(col.owner) def contains(other: ColumnOwner): Boolean = isSameOwner(other) def findNamedOwner(col: SqlColumn): Option[TableExpr] = if (this == col.owner) Some(this) else None - def isRoot: Boolean = TableName.isRoot(name) + def isRoot: Boolean = TableName.isRoot(tableName) def isUnion: Boolean = false @@ -1430,6 +1486,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self if (this == col.owner) Some(this) else subquery.findNamedOwner(col) def isRoot: Boolean = false + def identifier: String = TableName.foldToIdentifier(name) def isUnion: Boolean = subquery.isUnion @@ -1464,6 +1521,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self if (this == col.owner) Some(this) else withQuery.findNamedOwner(col) def isRoot: Boolean = false + def identifier: String = name def isUnion: Boolean = withQuery.isUnion @@ -1494,6 +1552,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self assert(!underlying.isInstanceOf[WithRef] || noalias) def name = alias.getOrElse(underlying.name) + def identifier: String = alias.getOrElse(underlying.identifier) def owns(col: SqlColumn): Boolean = col.owner.isSameOwner(this) || underlying.owns(col) def contains(other: ColumnOwner): Boolean = @@ -2298,7 +2357,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self def isDistinct: Boolean = distinct.nonEmpty override def isSameOwner(other: ColumnOwner): Boolean = - other.isSameOwner(TableRef(context, table.name)) + other.isSameOwner(TableRef(context, TableName(table.name))) def owns(col: SqlColumn): Boolean = cols.contains(col) || owns0(col) def contains(other: ColumnOwner): Boolean = @@ -2347,8 +2406,10 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self * joins */ def syntheticName(suffix: String): String = { - val joinNames = joins.map(_.child.name) - (table.name :: joinNames).mkString("_").take(50 - suffix.length) + suffix + // Synthesized names are used as subquery aliases, so they must be bare identifiers + // even when built from schema-qualified table names (issue #342). + val joinNames = joins.map(_.child.identifier) + (table.identifier :: joinNames).mkString("_").take(50 - suffix.length) + suffix } /** @@ -2452,9 +2513,11 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self val finalJoin = lastJoin.toSqlJoin(lastJoinParentTable, base.table, inner) finalJoin :: Nil } else { + // On the mergeable branch base.table is the raw TableRef, so its name may be + // schema-qualified and must be folded before use in alias position (#342). val assocTable = TableExpr.DerivedTableRef( context, - Some(base.table.name + "_assoc"), + Some(base.table.identifier + "_assoc"), base.table, true) val assocJoin = lastJoin.toSqlJoin(lastJoinParentTable, assocTable, inner) @@ -3167,7 +3230,10 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self def subqueryToWithQuery: SqlSelect = { table match { case SubqueryRef(_, name, sq, _) => - val with0 = WithRef(context, name + "_base", sq) + // A common table expression name is rendered verbatim rather than through the + // alias machinery, so it has to be folded here (issue #342). The derived table + // keeps the unfolded name, which carries the subquery's identity. + val with0 = WithRef(context, TableName.foldToIdentifier(name) + "_base", sq) val ref = TableExpr.DerivedTableRef(context, Some(name), with0, true) copy(withs = with0 :: withs, table = ref) case _ => @@ -3369,6 +3435,11 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self for { withFilter0 <- withFilter table <- parentTableForType(context) + // Unlike the synthesized names elsewhere, this one is load bearing: naming the + // subquery after its parent table is what keeps it `isSameOwner` with that + // table, so columns still bound to the table resolve against the subquery. + // Folding here would break that identity, so the fold happens in alias + // position instead, when the name is emitted (issue #342). sel <- withFilter0.toSubquery(table.name) res <- sel.addFilterOrderByOffsetLimit( None, @@ -4632,7 +4703,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self val tables = allTables(List(om)) val split = tables.sizeCompare(1) > 0 if (!split) Nil - else List(SplitObjectTypeMapping(om, tables)) + else List(SplitObjectTypeMapping(om, tables.map(_.sqlRef))) } def checkSuperInterfaces(om: ObjectMapping): List[ValidationFailure] = { @@ -4644,7 +4715,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self val tables = allTables(allMappings) val split = tables.sizeCompare(1) > 0 if (!split) Nil - else List(SplitInterfaceTypeMapping(om, allMappings, tables)) + else List(SplitInterfaceTypeMapping(om, allMappings, tables.map(_.sqlRef))) } def checkUnionMembers(om: ObjectMapping): List[ValidationFailure] = { @@ -4654,7 +4725,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self val tables = allTables(allMappings) val split = tables.sizeCompare(1) > 0 if (!split) Nil - else List(SplitUnionTypeMapping(om, allMappings, tables)) + else List(SplitUnionTypeMapping(om, allMappings, tables.map(_.sqlRef))) case _ => Nil } @@ -4779,7 +4850,14 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self } yield { val childTables = allTables(List(com)) if (parentTables.sameElements(childTables)) Nil - else List(SplitEmbeddedObjectTypeMapping(om, fm, com, parentTables, childTables)) + else + List( + SplitEmbeddedObjectTypeMapping( + om, + fm, + com, + parentTables.map(_.sqlRef), + childTables.map(_.sqlRef))) }).getOrElse(Nil) } @@ -4815,8 +4893,8 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self InconsistentJoinConditions( om, fm, - j.conditions.map(_._1.table).distinct, - j.conditions.map(_._2.table).distinct) + j.conditions.map(_._1.table.sqlRef).distinct, + j.conditions.map(_._2.table.sqlRef).distinct) } val serConsistent = { @@ -4836,9 +4914,12 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self if (headIsParent && lastIsChild && consistentChain) Nil else { val path = nonEmptyJoins.map(j => - (j.conditions.head._1.table, j.conditions.last._2.table)) + ( + j.conditions.head._1.table.sqlRef, + j.conditions.last._2.table.sqlRef)) - List(MisalignedJoins(om, fm, parentTable, childTable, path)) + List( + MisalignedJoins(om, fm, parentTable.sqlRef, childTable.sqlRef, path)) } } } @@ -4854,7 +4935,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self } } - private def allTables(oms: List[ObjectMapping]): List[String] = + private def allTables(oms: List[ObjectMapping]): List[TableName] = oms .flatMap(_.fieldMappings.flatMap { case SqlField(_, columnRef, _, _, _, _) => List(columnRef.table) diff --git a/modules/sql-core/src/test/scala/SqlMappingValidatorInvalidSuite.scala b/modules/sql-core/src/test/scala/SqlMappingValidatorInvalidSuite.scala index ce1c539d..4fec8bc9 100644 --- a/modules/sql-core/src/test/scala/SqlMappingValidatorInvalidSuite.scala +++ b/modules/sql-core/src/test/scala/SqlMappingValidatorInvalidSuite.scala @@ -46,7 +46,7 @@ trait SqlMappingValidatorInvalidSuite extends CatsEffectSuite { fm.fieldName, SchemaRenderer.renderType(field.tpe), field.tpe.isNullable, - columnRef.table, + columnRef.table.sqlRef, columnRef.column, colIsNullable)) case _ => None @@ -63,7 +63,7 @@ trait SqlMappingValidatorInvalidSuite extends CatsEffectSuite { om.tpe.name, fm.fieldName, SchemaRenderer.renderType(field.tpe), - columnRef.table, + columnRef.table.sqlRef, columnRef.column, columnRef.scalaTypeName)) case _ => None @@ -80,7 +80,7 @@ trait SqlMappingValidatorInvalidSuite extends CatsEffectSuite { om.tpe.name, fm.fieldName, SchemaRenderer.renderType(field.tpe), - columnRef.table, + columnRef.table.sqlRef, columnRef.column, columnRef.scalaTypeName)) case _ => None diff --git a/modules/sql-core/src/test/scala/SqlQualifiedNamesMapping.scala b/modules/sql-core/src/test/scala/SqlQualifiedNamesMapping.scala new file mode 100644 index 00000000..d3b64384 --- /dev/null +++ b/modules/sql-core/src/test/scala/SqlQualifiedNamesMapping.scala @@ -0,0 +1,326 @@ +// 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.Predicate.{Const, Eql} +import grackle.Query.{ + Binding, + Count, + Filter, + FilterOrderByOffsetLimit, + Limit, + OrderBy, + OrderSelection, + OrderSelections, + Select, + Unique +} +import grackle.QueryCompiler.{Elab, SelectElaborator} +import grackle.Term +import grackle.Value.{IntValue, StringValue} +import grackle.syntax._ + +// Mapping over tables with schema-qualified names (issue #342). The City -> Country +// relationship closes a cycle, so a query traversing Country -> City -> Country revisits +// the country table and forces the alias machinery to mint an alias for a qualified name. +trait SqlQualifiedNamesMapping[F[_]] extends SqlTestMapping[F] { + + object root extends RootDef { + val numCountries = col("num_countries", int8) + } + + object country extends TableDef("qualified.country") { + val code = col("code", bpchar(3)) + val name = col("name", text) + } + + object city extends TableDef("qualified.city") { + val id = col("id", int4) + val countrycode = col("countrycode", bpchar(3)) + val name = col("name", text) + } + + object speaks extends TableDef("qualified.speaks") { + val countrycode = col("countrycode", bpchar(3)) + val lang = col("lang", text) + } + + // Named so that folding qualified.country's qualifier with an underscore yields exactly + // this table's name, pinning that synthesized identifiers and real tables coexist. + object twin extends TableDef("qualified_country") { + val code = col("code", bpchar(3)) + val motto = col("motto", text) + } + + val schema = + schema""" + type Query { + country(code: String!): Country + countries(limit: Int!): [Country!]! + paged(offset: Int!, limit: Int!): CountryPage! + } + type Country { + code: String! + name: String! + cities(limit: Int): [City!]! + languages: [Language!]! + twin: Twin + } + type City { + name: String! + country: Country! + languages: [Language!]! + twins: [Twin!]! + } + type Language { + language: String! + } + type Twin { + motto: String! + } + type CountryPage { + total: Int! + items: [PagedCountry!]! + } + type PagedCountry { + code: String! + name: String! + twin: PagedTwin! + cities(offset: Int!, limit: Int!): CityPage! + } + type PagedTwin { + motto: String! + cities(offset: Int!, limit: Int!): TwinCityPage! + } + type CityPage { + items: [PagedCity!]! + } + type TwinCityPage { + items: [PagedCity!]! + } + type PagedCity { + name: String! + country: CountryRef! + } + type CountryRef { + code: String! + name: String! + } + """ + + val QueryType = schema.ref("Query") + val CountryType = schema.ref("Country") + val CityType = schema.ref("City") + val LanguageType = schema.ref("Language") + val TwinType = schema.ref("Twin") + val CountryPageType = schema.ref("CountryPage") + val PagedCountryType = schema.ref("PagedCountry") + val PagedTwinType = schema.ref("PagedTwin") + val CityPageType = schema.ref("CityPage") + val TwinCityPageType = schema.ref("TwinCityPage") + val PagedCityType = schema.ref("PagedCity") + val CountryRefType = schema.ref("CountryRef") + + val typeMappings = + List( + ObjectMapping( + tpe = QueryType, + fieldMappings = List( + SqlObject("country"), + SqlObject("countries"), + SqlObject("paged") + ) + ), + ObjectMapping( + tpe = CountryType, + fieldMappings = List( + SqlField("code", country.code, key = true), + SqlField("name", country.name), + SqlObject("cities", Join(country.code, city.countrycode)), + SqlObject("languages", Join(country.code, speaks.countrycode)), + SqlObject("twin", Join(country.code, twin.code)) + ) + ), + ObjectMapping( + tpe = CityType, + fieldMappings = List( + SqlField("id", city.id, key = true, hidden = true), + SqlField("countrycode", city.countrycode, hidden = true), + SqlField("name", city.name), + SqlObject("country", Join(city.countrycode, country.code)), + // Two sibling list children of City. A limit on the parent `cities` field compiles + // these to the branches of an SqlUnion, which is the shape that exercises + // SqlUnion.addFilterOrderByOffsetLimit's subquery naming. + SqlObject("languages", Join(city.countrycode, speaks.countrycode)), + SqlObject("twins", Join(city.countrycode, twin.code)) + ) + ), + ObjectMapping( + tpe = LanguageType, + fieldMappings = List( + SqlField("language", speaks.lang, key = true, associative = true), + SqlField("countrycode", speaks.countrycode, hidden = true) + ) + ), + ObjectMapping( + tpe = TwinType, + fieldMappings = List( + SqlField("code", twin.code, key = true, hidden = true), + SqlField("motto", twin.motto) + ) + ), + // The paged half of the mapping. Offset/limit paging synthesizes a numbered subquery + // per level, and the page types below deliberately map a parent table's key column at + // the child's result path, so a column reference arrives from a result path that has + // no table definition of its own. + ObjectMapping( + tpe = CountryPageType, + fieldMappings = List( + SqlField("total", root.numCountries), + SqlObject("items") + ) + ), + ObjectMapping( + tpe = PagedCountryType, + fieldMappings = List( + SqlField("code", country.code, key = true), + SqlField("name", country.name), + SqlObject("twin", Join(country.code, twin.code)), + SqlObject("cities") + ) + ), + // qualified_country as an enclosing table with a paged child of its own, so the table + // that loses the alias slot to the fold of qualified.country is itself referenced from + // a deeper result path. + ObjectMapping( + tpe = PagedTwinType, + fieldMappings = List( + SqlField("code", twin.code, key = true, hidden = true), + SqlField("motto", twin.motto), + SqlObject("cities") + ) + ), + ObjectMapping( + tpe = CityPageType, + fieldMappings = List( + SqlField("code", country.code, key = true, hidden = true), + SqlObject("items", Join(country.code, city.countrycode)) + ) + ), + ObjectMapping( + tpe = TwinCityPageType, + fieldMappings = List( + SqlField("code", twin.code, key = true, hidden = true), + SqlObject("items", Join(twin.code, city.countrycode)) + ) + ), + ObjectMapping( + tpe = PagedCityType, + fieldMappings = List( + SqlField("id", city.id, key = true, hidden = true), + SqlField("countrycode", city.countrycode, hidden = true), + SqlField("name", city.name), + SqlObject("country", Join(city.countrycode, country.code)) + ) + ), + ObjectMapping( + tpe = CountryRefType, + fieldMappings = List( + SqlField("code", country.code, key = true), + SqlField("name", country.name) + ) + ) + ) + + abstract class PagingConfig(key: String, orderTerm: Term[String]) { + def setup(offset: Int, limit: Int): Elab[Unit] = + Elab.env(key -> PagingInfo(offset, limit)) + + def elabItems: Elab[Unit] = Elab.envE[PagingInfo](key).flatMap(_.elabItems) + def elabTotal: Elab[Unit] = Elab.envE[PagingInfo](key).flatMap(_.elabTotal) + + case class PagingInfo(offset: Int, limit: Int) { + def elabItems: Elab[Unit] = + Elab.transformChild { child => + FilterOrderByOffsetLimit( + None, + Some(List(OrderSelection(orderTerm, nullsLast = nullsHigh))), + Some(offset), + Some(limit), + child) + } + + def elabTotal: Elab[Unit] = + Elab.transformChild(_ => Count(Select("items", Select("code")))) + } + } + + object CountryPaging extends PagingConfig("countryPaging", PagedCountryType / "code") + object CityPaging extends PagingConfig("cityPaging", PagedCityType / "name") + object TwinCityPaging extends PagingConfig("twinCityPaging", PagedCityType / "name") + + override val selectElaborator = SelectElaborator { + case (QueryType, "country", List(Binding("code", StringValue(code)))) => + Elab.transformChild(child => + Unique(Filter(Eql(CountryType / "code", Const(code)), child))) + + case (QueryType, "countries", List(Binding("limit", IntValue(limit)))) => + Elab.transformChild(child => + Limit( + limit, + OrderBy(OrderSelections(List(OrderSelection[String](CountryType / "code"))), child))) + + case ( + QueryType, + "paged", + List(Binding("offset", IntValue(off)), Binding("limit", IntValue(lim)))) => + CountryPaging.setup(off, lim) + + case (CountryPageType, "items", Nil) => + CountryPaging.elabItems + + case (CountryPageType, "total", Nil) => + CountryPaging.elabTotal + + case ( + PagedCountryType, + "cities", + List(Binding("offset", IntValue(off)), Binding("limit", IntValue(lim)))) => + CityPaging.setup(off, lim) + + case (CityPageType, "items", Nil) => + CityPaging.elabItems + + case ( + PagedTwinType, + "cities", + List(Binding("offset", IntValue(off)), Binding("limit", IntValue(lim)))) => + TwinCityPaging.setup(off, lim) + + case (TwinCityPageType, "items", Nil) => + TwinCityPaging.elabItems + + case (CountryType, "cities", List(Binding("limit", limit))) => + Elab.transformChild(child => + limit match { + case IntValue(lim) => + Limit( + lim, + OrderBy(OrderSelections(List(OrderSelection[String](CityType / "name"))), child)) + case _ => child + }) + } +} diff --git a/modules/sql-core/src/test/scala/SqlQualifiedNamesSuite.scala b/modules/sql-core/src/test/scala/SqlQualifiedNamesSuite.scala new file mode 100644 index 00000000..6e8a8bae --- /dev/null +++ b/modules/sql-core/src/test/scala/SqlQualifiedNamesSuite.scala @@ -0,0 +1,449 @@ +// 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 + +// Wired up for doobie-pg and skunk, which share the testdata/pg fixtures; the fix under test +// lives in sql-core so those two backends suffice to pin it. Oracle is omitted because its +// schemas are users (a qualified-name fixture needs dedicated user setup), and MSSQL because +// its fixture init would need equivalent schema plumbing - both can adopt this suite later. +trait SqlQualifiedNamesSuite extends CatsEffectSuite { + def mapping: Mapping[IO] + + test("recursive query against schema-qualified table names (#342)") { + val query = """ + query { + country(code: "CAN") { + name + cities { + name + country { + name + } + } + } + } + """ + + val expected = json""" + { + "data" : { + "country" : { + "name" : "Canada", + "cities" : [ + { + "name" : "Toronto", + "country" : { + "name" : "Canada" + } + }, + { + "name" : "Ottawa", + "country" : { + "name" : "Canada" + } + } + ] + } + } + } + """ + + assertWeaklyEqualIO(mapping.compileAndRun(query), expected) + } + + // Top-level limit over a list with a child join forces the compiler to synthesize named + // subqueries (via syntheticName), a second path on which a schema-qualified table name + // must not leak into alias position. + test("top-level limit over schema-qualified tables (#342)") { + val query = """ + query { + countries(limit: 2) { + name + cities { + name + } + } + } + """ + + val expected = json""" + { + "data" : { + "countries" : [ + { + "name" : "Canada", + "cities" : [ + { "name" : "Toronto" }, + { "name" : "Ottawa" } + ] + }, + { + "name" : "Germany", + "cities" : [ + { "name" : "Berlin" } + ] + } + ] + } + } + """ + + assertWeaklyEqualIO(mapping.compileAndRun(query), expected) + } + + // A limit nested below the root exercises the window-function machinery and its + // synthesized subquery names. + test("nested limit over schema-qualified tables (#342)") { + val query = """ + query { + country(code: "CAN") { + name + cities(limit: 1) { + name + } + } + } + """ + + val expected = json""" + { + "data" : { + "country" : { + "name" : "Canada", + "cities" : [ + { "name" : "Ottawa" } + ] + } + } + } + """ + + assertWeaklyEqualIO(mapping.compileAndRun(query), expected) + } + + // An associative child reached through a single mergeable join takes the DerivedTableRef + // "_assoc" path, whose alias is derived from the raw table name - a further place a + // schema-qualified name must not leak into alias position. + test("associative field over schema-qualified tables (#342)") { + val query = """ + query { + country(code: "CAN") { + name + languages { + language + } + } + } + """ + + val expected = json""" + { + "data" : { + "country" : { + "name" : "Canada", + "languages" : [ + { "language" : "English" }, + { "language" : "French" } + ] + } + } + } + """ + + assertWeaklyEqualIO(mapping.compileAndRun(query), expected) + } + + // A limit on a node with two sibling list children compiles to an SqlUnion whose branches are + // then wrapped in a subquery by SqlUnion.addFilterOrderByOffsetLimit. That subquery is named + // from the parent table, so for a schema-qualified table the name has to be folded - but + // folding it also changes the TableExpr's identity, since TableExprs are compared by name. + // Columns still bound to the unfolded TableRef are then rendered qualified by the raw dotted + // name while the FROM entry carries the folded one, e.g. + // + // FROM ( ... UNION ALL ... ) AS qualified_city + // WHERE (qualified.city. IS NOT NULL) + // + // For an unqualified table the folded and unfolded names coincide, so the divergence is + // invisible; it only surfaces here. Note the limit must be nested - the same union under a + // top-level limit renders correctly. + test("nested limit over a union of schema-qualified tables (#342)") { + val query = """ + query { + country(code: "CAN") { + cities(limit: 1) { + name + languages { + language + } + twins { + motto + } + } + } + } + """ + + val expected = json""" + { + "data" : { + "country" : { + "cities" : [ + { + "name" : "Ottawa", + "languages" : [ + { "language" : "English" }, + { "language" : "French" } + ], + "twins" : [ + { "motto" : "A mari usque ad mare" } + ] + } + ] + } + } + } + """ + + assertWeaklyEqualIO(mapping.compileAndRun(query), expected) + } + + // qualified_country is a real table whose name equals qualified.country with its qualifier + // folded by an underscore. Recursing through qualified.country while joining + // qualified_country in the same statement pins that folded synthesized identifiers and + // identically-named real tables coexist (the render-time alias state uniquifies any + // second occurrence of an already-seen name). + test("folded qualified name coexists with an identically named table (#342)") { + val query = """ + query { + country(code: "CAN") { + name + twin { + motto + } + cities { + name + country { + name + twin { + motto + } + } + } + } + } + """ + + val expected = json""" + { + "data" : { + "country" : { + "name" : "Canada", + "twin" : { + "motto" : "A mari usque ad mare" + }, + "cities" : [ + { + "name" : "Toronto", + "country" : { + "name" : "Canada", + "twin" : { + "motto" : "A mari usque ad mare" + } + } + }, + { + "name" : "Ottawa", + "country" : { + "name" : "Canada", + "twin" : { + "motto" : "A mari usque ad mare" + } + } + } + ] + } + } + } + """ + + assertWeaklyEqualIO(mapping.compileAndRun(query), expected) + } + + test("offset/limit paging over schema-qualified table names (#342)") { + val query = """ + query { + paged(offset: 0, limit: 2) { + items { + code + name + cities(offset: 0, limit: 2) { + items { + name + } + } + } + } + } + """ + + val expected = json""" + { + "data" : { + "paged" : { + "items" : [ + { + "code" : "CAN", + "name" : "Canada", + "cities" : { + "items" : [ + { + "name" : "Ottawa" + }, + { + "name" : "Toronto" + } + ] + } + }, + { + "code" : "DEU", + "name" : "Germany", + "cities" : { + "items" : [ + { + "name" : "Berlin" + } + ] + } + } + ] + } + } + } + """ + + assertWeaklyEqualIO(mapping.compileAndRun(query), expected) + } + + test("paging over a qualified table reached at two result paths (#342)") { + val query = """ + query { + paged(offset: 0, limit: 2) { + total + items { + code + name + twin { + motto + cities(offset: 0, limit: 1) { + items { + name + } + } + } + cities(offset: 0, limit: 2) { + items { + name + country { + code + name + } + } + } + } + } + } + """ + + val expected = json""" + { + "data" : { + "paged" : { + "total" : 2, + "items" : [ + { + "code" : "CAN", + "name" : "Canada", + "twin" : { + "motto" : "A mari usque ad mare", + "cities" : { + "items" : [ + { + "name" : "Ottawa" + } + ] + } + }, + "cities" : { + "items" : [ + { + "name" : "Ottawa", + "country" : { + "code" : "CAN", + "name" : "Canada" + } + }, + { + "name" : "Toronto", + "country" : { + "code" : "CAN", + "name" : "Canada" + } + } + ] + } + }, + { + "code" : "DEU", + "name" : "Germany", + "twin" : { + "motto" : "Einigkeit und Recht und Freiheit", + "cities" : { + "items" : [ + { + "name" : "Berlin" + } + ] + } + }, + "cities" : { + "items" : [ + { + "name" : "Berlin", + "country" : { + "code" : "DEU", + "name" : "Germany" + } + } + ] + } + } + ] + } + } + } + """ + + assertWeaklyEqualIO(mapping.compileAndRun(query), expected) + } +} diff --git a/modules/sql-core/src/test/scala/SqlTestMapping.scala b/modules/sql-core/src/test/scala/SqlTestMapping.scala index 15e47246..b0cc3f59 100644 --- a/modules/sql-core/src/test/scala/SqlTestMapping.scala +++ b/modules/sql-core/src/test/scala/SqlTestMapping.scala @@ -54,5 +54,5 @@ trait SqlTestMapping[F[_]] extends SqlMappingLike[F] { outer => implicit tableName: TableName, typeName: TypeName[T], pos: SourcePos): ColumnRef = - ColumnRef(tableName.name, colName, codec, typeName.value, pos) + ColumnRef(tableName, colName, codec, typeName.value, pos) } diff --git a/modules/sql-core/src/test/scala/TableNameSuite.scala b/modules/sql-core/src/test/scala/TableNameSuite.scala new file mode 100644 index 00000000..5b42a750 --- /dev/null +++ b/modules/sql-core/src/test/scala/TableNameSuite.scala @@ -0,0 +1,57 @@ +// 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 munit.CatsEffectSuite + +import grackle.sql._ + +// TableName is a path-dependent member of SqlMappingLike[F[_]], not a free-standing top-level +// type, so it needs a concrete mapping instance to reach - any SqlMappingLike[IO] will do, since +// these tests never touch the mapping's own fields or run a query. +trait SqlTableNameSuite extends CatsEffectSuite { + def mapping: SqlMappingLike[IO] + + lazy val M = mapping + + test("unqualified name has no schema") { + val tn = M.TableName("country") + assertEquals(tn.schema, None) + assertEquals(tn.name, "country") + } + + test("schema-qualified name splits on the last dot") { + val tn = M.TableName("public.country") + assertEquals(tn.schema, Some("public")) + assertEquals(tn.name, "country") + } + + test("sqlRef renders the qualified reference, dots intact") { + assertEquals(M.TableName("public.country").sqlRef, "public.country") + assertEquals(M.TableName("country").sqlRef, "country") + } + + test("identifier folds the qualifier to a bare-identifier-safe form") { + assertEquals(M.TableName("public.country").identifier, "public_country") + assertEquals(M.TableName("country").identifier, "country") + } + + test("toString matches sqlRef") { + val tn = M.TableName("public.country") + assertEquals(tn.toString, tn.sqlRef) + } +} diff --git a/testdata/pg/qualified-names.sql b/testdata/pg/qualified-names.sql new file mode 100644 index 00000000..e5dfb965 --- /dev/null +++ b/testdata/pg/qualified-names.sql @@ -0,0 +1,43 @@ +CREATE SCHEMA qualified; + +CREATE TABLE qualified.country ( + code character(3) NOT NULL PRIMARY KEY, + name text NOT NULL +); + +CREATE TABLE qualified.city ( + id integer NOT NULL PRIMARY KEY, + countrycode character(3) NOT NULL, + name text NOT NULL +); + +CREATE TABLE qualified.speaks ( + countrycode character(3) NOT NULL, + lang text NOT NULL, + PRIMARY KEY (countrycode, lang) +); + +-- Deliberately named so that folding the qualifier of qualified.country with an underscore +-- yields this table's name: pins that synthesized aliases and real tables can coexist. +CREATE TABLE qualified_country ( + code character(3) NOT NULL PRIMARY KEY, + motto text NOT NULL +); + +INSERT INTO qualified.country (code, name) VALUES +('CAN', 'Canada'), +('DEU', 'Germany'); + +INSERT INTO qualified.city (id, countrycode, name) VALUES +(1, 'CAN', 'Toronto'), +(2, 'CAN', 'Ottawa'), +(3, 'DEU', 'Berlin'); + +INSERT INTO qualified.speaks (countrycode, lang) VALUES +('CAN', 'English'), +('CAN', 'French'), +('DEU', 'German'); + +INSERT INTO qualified_country (code, motto) VALUES +('CAN', 'A mari usque ad mare'), +('DEU', 'Einigkeit und Recht und Freiheit');