From 86f082c47023ccb393ed6d8a03bc0ffea161e373 Mon Sep 17 00:00:00 2001 From: deusaquilus Date: Fri, 6 Sep 2019 16:44:27 -0400 Subject: [PATCH] Fixing Embedded Coproduct Column Duplication Issue --- .gitignore | 1 + .../getquill/context/cassandra/CqlIdiom.scala | 5 +- .../main/scala/io/getquill/AstPrinter.scala | 11 +++- .../main/scala/io/getquill/MirrorIdiom.scala | 4 +- .../src/main/scala/io/getquill/ast/Ast.scala | 22 ++++++-- .../io/getquill/ast/StatefulTransformer.scala | 4 +- .../getquill/ast/StatelessTransformer.scala | 2 +- .../norm/NormalizeAggregationIdent.scala | 4 +- .../io/getquill/norm/RenameProperties.scala | 3 +- .../capture/DemarcateExternalAliases.scala | 4 +- .../io/getquill/quotation/Liftables.scala | 7 ++- .../scala/io/getquill/quotation/Parsing.scala | 13 ++++- .../io/getquill/quotation/Unliftables.scala | 7 ++- .../scala/io/getquill/ast/OpinionSpec.scala | 13 ++--- .../ast/StatefulTransformerSpec.scala | 5 +- .../ast/StatelessTransformerSpec.scala | 9 ++-- .../io/getquill/norm/BetaReductionSpec.scala | 5 +- .../getquill/norm/ExpandReturningSpec.scala | 9 ++-- .../io/getquill/quotation/IsDynamicSpec.scala | 3 +- .../context/orientdb/OrientDBIdiom.scala | 10 ++-- .../getquill/context/spark/SparkDialect.scala | 2 +- .../main/scala/io/getquill/MySQLDialect.scala | 5 +- .../getquill/context/sql/idiom/SqlIdiom.scala | 20 ++++---- .../sql/norm/ExpandNestedQueries.scala | 50 ++++++++++++------- .../sql/norm/nested/ExpandSelect.scala | 12 +++-- .../getquill/context/sql/EmbeddedSpec.scala | 40 ++++++++++++++- 26 files changed, 191 insertions(+), 79 deletions(-) diff --git a/.gitignore b/.gitignore index be23d59da5..c03aff2793 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,7 @@ MyTestJdbc.scala MyJdbcTest.scala MySqlTest.scala MyTest.scala +MyCqlTest.scala quill-core/src/main/resources/logback.xml quill-jdbc/src/main/resources/logback.xml log.txt* diff --git a/quill-cassandra/src/main/scala/io/getquill/context/cassandra/CqlIdiom.scala b/quill-cassandra/src/main/scala/io/getquill/context/cassandra/CqlIdiom.scala index d7f567bb74..d22bd14657 100644 --- a/quill-cassandra/src/main/scala/io/getquill/context/cassandra/CqlIdiom.scala +++ b/quill-cassandra/src/main/scala/io/getquill/context/cassandra/CqlIdiom.scala @@ -112,9 +112,12 @@ trait CqlIdiom extends Idiom { case other => fail(s"Cql doesn't support the '$other' operator.") } + // Note: The CqlIdiom does not support joins so there is no need for any complex un-nesting logic like in SqlIdiom + // if there are Embedded classes, they will result in Property(Property(embedded, embeddedProp), actualProp) + // and we only need to take the top-level property i.e. `actualProp`. implicit def propertyTokenizer(implicit valueTokenizer: Tokenizer[Value], identTokenizer: Tokenizer[Ident], strategy: NamingStrategy): Tokenizer[Property] = Tokenizer[Property] { - case Property.Opinionated(_, name, renameable) => renameable.fixedOr(name.token)(strategy.column(name).token) + case Property.Opinionated(_, name, renameable, _) => renameable.fixedOr(name.token)(strategy.column(name).token) } implicit def valueTokenizer(implicit strategy: NamingStrategy): Tokenizer[Value] = Tokenizer[Value] { diff --git a/quill-core/src/main/scala/io/getquill/AstPrinter.scala b/quill-core/src/main/scala/io/getquill/AstPrinter.scala index 739cddca86..51694b8048 100644 --- a/quill-core/src/main/scala/io/getquill/AstPrinter.scala +++ b/quill-core/src/main/scala/io/getquill/AstPrinter.scala @@ -2,7 +2,8 @@ package io.getquill import fansi.Str import io.getquill.ast.Renameable.{ ByStrategy, Fixed } -import io.getquill.ast.{ Ast, Entity, Property, Renameable } +import io.getquill.ast.Visibility.{ Hidden, Visible } +import io.getquill.ast._ import pprint.{ Renderer, Tree, Truncated } object AstPrinter { @@ -28,6 +29,12 @@ class AstPrinter(traceOpinions: Boolean, traceAstSimple: Boolean) extends pprint case Fixed => Tree.Literal("F") } + private def printVisibility(v: Visibility) = + v match { + case Visible => Tree.Literal("Vis") + case Hidden => Tree.Literal("Hide") + } + override def additionalHandlers: PartialFunction[Any, Tree] = { case ast: Ast if (traceAstSimple) => Tree.Literal(ast + "") // Do not blow up if it is null @@ -36,7 +43,7 @@ class AstPrinter(traceOpinions: Boolean, traceAstSimple: Boolean) extends pprint Tree.Literal(past + "") // Do not blow up if it is null case p: Property if (traceOpinions) => - Tree.Apply("Property", List[Tree](treeify(p.ast), treeify(p.name), printRenameable(p.renameable)).iterator) + Tree.Apply("Property", List[Tree](treeify(p.ast), treeify(p.name), printRenameable(p.renameable), printVisibility(p.visibility)).iterator) case e: Entity if (traceOpinions) => Tree.Apply("Entity", List[Tree](treeify(e.name), treeify(e.properties), printRenameable(e.renameable)).iterator) diff --git a/quill-core/src/main/scala/io/getquill/MirrorIdiom.scala b/quill-core/src/main/scala/io/getquill/MirrorIdiom.scala index a8e0e4af01..4de624e011 100644 --- a/quill-core/src/main/scala/io/getquill/MirrorIdiom.scala +++ b/quill-core/src/main/scala/io/getquill/MirrorIdiom.scala @@ -184,8 +184,8 @@ trait MirrorIdiomBase extends Idiom { } implicit def propertyTokenizer(implicit liftTokenizer: Tokenizer[Lift]): Tokenizer[Property] = Tokenizer[Property] { - case Property.Opinionated(ExternalIdent(_), name, renameable) => stmt"${tokenizeName(name, renameable).token}" - case Property.Opinionated(ref, name, renameable) => stmt"${scopedTokenizer(ref)}.${tokenizeName(name, renameable).token}" + case Property.Opinionated(ExternalIdent(_), name, renameable, _) => stmt"${tokenizeName(name, renameable).token}" + case Property.Opinionated(ref, name, renameable, _) => stmt"${scopedTokenizer(ref)}.${tokenizeName(name, renameable).token}" } implicit val valueTokenizer: Tokenizer[Value] = Tokenizer[Value] { diff --git a/quill-core/src/main/scala/io/getquill/ast/Ast.scala b/quill-core/src/main/scala/io/getquill/ast/Ast.scala index d06de62034..c8384ae220 100644 --- a/quill-core/src/main/scala/io/getquill/ast/Ast.scala +++ b/quill-core/src/main/scala/io/getquill/ast/Ast.scala @@ -148,6 +148,14 @@ sealed trait OpinionValues[T <: Opinion[T]] { def neutral: T } +sealed trait Visibility extends Opinion[Visibility] +object Visibility extends OpinionValues[Visibility] { + case object Visible extends Visibility with Opinion[Visibility] + case object Hidden extends Visibility with Opinion[Visibility] + + override def neutral: Visibility = Visible +} + sealed trait Renameable extends Opinion[Renameable] { def fixedOr[T](plain: T)(otherwise: T) = this match { @@ -179,18 +187,23 @@ case class Property(ast: Ast, name: String) extends Ast { // scala creates companion objects, the apply/unapply wouldn't be able to work correctly. def renameable: Renameable = Renameable.neutral + // Properties that are 'Hidden' are used for embedded objects whose path should not be expressed + // during SQL Tokenization. + def visibility: Visibility = Visibility.Visible + override def neutral: Property = new Property(ast, name) { override def renameable = Renameable.neutral + override def visibility: Visibility = Visibility.neutral } override def equals(that: Any) = that match { - case p: Property => (p.ast, p.name, p.renameable) == ((ast, name, renameable)) + case p: Property => (p.ast, p.name, p.renameable, p.visibility) == ((ast, name, renameable, visibility)) case _ => false } - override def hashCode = (ast, name, renameable).hashCode() + override def hashCode = (ast, name, renameable, visibility).hashCode() } object Property { @@ -198,12 +211,13 @@ object Property { def unapply(p: Property) = Some((p.ast, p.name)) object Opinionated { - def apply(ast: Ast, name: String, renameableNew: Renameable) = + def apply(ast: Ast, name: String, renameableNew: Renameable, visibilityNew: Visibility) = new Property(ast, name) { override def renameable: Renameable = renameableNew + override def visibility: Visibility = visibilityNew } def unapply(p: Property) = - Some((p.ast, p.name, p.renameable)) + Some((p.ast, p.name, p.renameable, p.visibility)) } } diff --git a/quill-core/src/main/scala/io/getquill/ast/StatefulTransformer.scala b/quill-core/src/main/scala/io/getquill/ast/StatefulTransformer.scala index 2ec64f9212..fec8a8f9c6 100644 --- a/quill-core/src/main/scala/io/getquill/ast/StatefulTransformer.scala +++ b/quill-core/src/main/scala/io/getquill/ast/StatefulTransformer.scala @@ -210,9 +210,9 @@ trait StatefulTransformer[T] { def apply(e: Property): (Property, StatefulTransformer[T]) = e match { - case Property.Opinionated(a, b, renameable) => + case Property.Opinionated(a, b, renameable, visibility) => val (at, att) = apply(a) - (Property.Opinionated(at, b, renameable), att) + (Property.Opinionated(at, b, renameable, visibility), att) } def apply(e: Operation): (Operation, StatefulTransformer[T]) = diff --git a/quill-core/src/main/scala/io/getquill/ast/StatelessTransformer.scala b/quill-core/src/main/scala/io/getquill/ast/StatelessTransformer.scala index e3b11328d9..4a229f3b86 100644 --- a/quill-core/src/main/scala/io/getquill/ast/StatelessTransformer.scala +++ b/quill-core/src/main/scala/io/getquill/ast/StatelessTransformer.scala @@ -86,7 +86,7 @@ trait StatelessTransformer { def apply(e: Property): Property = e match { - case Property.Opinionated(a, name, renameable) => Property.Opinionated(apply(a), name, renameable) + case Property.Opinionated(a, name, renameable, visibility) => Property.Opinionated(apply(a), name, renameable, visibility) } def apply(e: Operation): Operation = diff --git a/quill-core/src/main/scala/io/getquill/norm/NormalizeAggregationIdent.scala b/quill-core/src/main/scala/io/getquill/norm/NormalizeAggregationIdent.scala index 03d2204ac7..ede4c16af9 100644 --- a/quill-core/src/main/scala/io/getquill/norm/NormalizeAggregationIdent.scala +++ b/quill-core/src/main/scala/io/getquill/norm/NormalizeAggregationIdent.scala @@ -9,8 +9,8 @@ object NormalizeAggregationIdent { // a => a.b.map(x => x.c).agg => // a => a.b.map(a => a.c).agg - case Aggregation(op, Map(p @ Property(i: Ident, _), mi, Property.Opinionated(_: Ident, n, renameable))) if i != mi => - Some(Aggregation(op, Map(p, i, Property.Opinionated(i, n, renameable)))) // in example aove, if c in x.c is fixed c in a.c should also be + case Aggregation(op, Map(p @ Property(i: Ident, _), mi, Property.Opinionated(_: Ident, n, renameable, visibility))) if i != mi => + Some(Aggregation(op, Map(p, i, Property.Opinionated(i, n, renameable, visibility)))) // in example aove, if c in x.c is fixed c in a.c should also be case _ => None } diff --git a/quill-core/src/main/scala/io/getquill/norm/RenameProperties.scala b/quill-core/src/main/scala/io/getquill/norm/RenameProperties.scala index 0c2e19e0b8..e7d50caa29 100644 --- a/quill-core/src/main/scala/io/getquill/norm/RenameProperties.scala +++ b/quill-core/src/main/scala/io/getquill/norm/RenameProperties.scala @@ -1,6 +1,7 @@ package io.getquill.norm import io.getquill.ast.Renameable.Fixed +import io.getquill.ast.Visibility.Visible import io.getquill.ast._ object RenameProperties extends StatelessTransformer { @@ -202,7 +203,7 @@ object RenameProperties extends StatelessTransformer { case Nil => base case head :: tail => apply(Property(base, head), tail) } - apply(base, path) -> Property.Opinionated(base, alias, Fixed) + apply(base, path) -> Property.Opinionated(base, alias, Fixed, Visible) // Hidden properties cannot be renamed } case Tuple(values) => values.zipWithIndex.flatMap { diff --git a/quill-core/src/main/scala/io/getquill/norm/capture/DemarcateExternalAliases.scala b/quill-core/src/main/scala/io/getquill/norm/capture/DemarcateExternalAliases.scala index 2adf6cb029..6ee00af47c 100644 --- a/quill-core/src/main/scala/io/getquill/norm/capture/DemarcateExternalAliases.scala +++ b/quill-core/src/main/scala/io/getquill/norm/capture/DemarcateExternalAliases.scala @@ -56,9 +56,9 @@ private[getquill] case class DemarcateExternalAliases(externalIdent: Ident) exte case FlatJoin(t, a, iA, o) => FlatJoin(t, a, iA, applyNonOverride(iA)(o)) - case p @ Property.Opinionated(id @ Ident(_), value, renameable) => + case p @ Property.Opinionated(id @ Ident(_), value, renameable, visibility) => if (id == externalIdent) - Property.Opinionated(ExternalIdent(externalIdent.name), value, renameable) + Property.Opinionated(ExternalIdent(externalIdent.name), value, renameable, visibility) else p diff --git a/quill-core/src/main/scala/io/getquill/quotation/Liftables.scala b/quill-core/src/main/scala/io/getquill/quotation/Liftables.scala index 6e9aab2fad..56b72da608 100644 --- a/quill-core/src/main/scala/io/getquill/quotation/Liftables.scala +++ b/quill-core/src/main/scala/io/getquill/quotation/Liftables.scala @@ -110,6 +110,11 @@ trait Liftables { case Renameable.ByStrategy => q"$pack.Renameable.ByStrategy" } + implicit val visibilityLiftable: Liftable[Visibility] = Liftable[Visibility] { + case Visibility.Visible => q"$pack.Visibility.Visible" + case Visibility.Hidden => q"$pack.Visibility.Hidden" + } + implicit val queryLiftable: Liftable[Query] = Liftable[Query] { case Entity.Opinionated(a, b, renameable) => q"$pack.Entity.Opinionated($a, $b, $renameable)" case Filter(a, b, c) => q"$pack.Filter($a, $b, $c)" @@ -134,7 +139,7 @@ trait Liftables { } implicit val propertyLiftable: Liftable[Property] = Liftable[Property] { - case Property.Opinionated(a, b, renameable) => q"$pack.Property.Opinionated($a, $b, $renameable)" + case Property.Opinionated(a, b, renameable, visibility) => q"$pack.Property.Opinionated($a, $b, $renameable, $visibility)" } implicit val orderingLiftable: Liftable[Ordering] = Liftable[Ordering] { diff --git a/quill-core/src/main/scala/io/getquill/quotation/Parsing.scala b/quill-core/src/main/scala/io/getquill/quotation/Parsing.scala index a5aa7e65a8..85a6025083 100644 --- a/quill-core/src/main/scala/io/getquill/quotation/Parsing.scala +++ b/quill-core/src/main/scala/io/getquill/quotation/Parsing.scala @@ -15,6 +15,7 @@ import scala.collection.immutable.StringOps import scala.reflect.macros.TypecheckException import io.getquill.ast.Implicits._ import io.getquill.ast.Renameable.Fixed +import io.getquill.ast.Visibility.{ Hidden, Visible } import io.getquill.util.Interleave trait Parsing extends ValueComputation { @@ -483,7 +484,17 @@ trait Parsing extends ValueComputation { if (caseAccessors.nonEmpty && !caseAccessors.contains(property)) c.fail(s"Can't find case class property: ${property.decodedName.toString}") - Property(astParser(e), property.decodedName.toString) + val visibility = { + val tpe = c.typecheck(q"$e.$property") + val innerParam = innerOptionParam(q"$tpe".tpe, None) + if (is[Embedded](q"$innerParam")) Hidden + else Visible + } + + Property.Opinionated(astParser(e), property.decodedName.toString, + Renameable.neutral, //Renameability of the property is determined later in the RenameProperties normalization phase + visibility // Visibility is decided here. + ) } val operationParser: Parser[Operation] = Parser[Operation] { diff --git a/quill-core/src/main/scala/io/getquill/quotation/Unliftables.scala b/quill-core/src/main/scala/io/getquill/quotation/Unliftables.scala index 825cef8b08..02d1ffbf68 100644 --- a/quill-core/src/main/scala/io/getquill/quotation/Unliftables.scala +++ b/quill-core/src/main/scala/io/getquill/quotation/Unliftables.scala @@ -143,9 +143,14 @@ trait Unliftables { case q"$pack.Renameable.Fixed" => Renameable.Fixed } + implicit val visibilityUnliftable: Unliftable[Visibility] = Unliftable[Visibility] { + case q"$pack.Visibility.Visible" => Visibility.Visible + case q"$pack.Visibility.Hidden" => Visibility.Hidden + } + implicit val propertyUnliftable: Unliftable[Property] = Unliftable[Property] { case q"$pack.Property.apply(${ a: Ast }, ${ b: String })" => Property(a, b) - case q"$pack.Property.Opinionated.apply(${ a: Ast }, ${ b: String }, ${ renameable: Renameable })" => Property.Opinionated(a, b, renameable) + case q"$pack.Property.Opinionated.apply(${ a: Ast }, ${ b: String }, ${ renameable: Renameable }, ${ visibility: Visibility })" => Property.Opinionated(a, b, renameable, visibility) } implicit def optionUnliftable[T](implicit u: Unliftable[T]): Unliftable[Option[T]] = Unliftable[Option[T]] { diff --git a/quill-core/src/test/scala/io/getquill/ast/OpinionSpec.scala b/quill-core/src/test/scala/io/getquill/ast/OpinionSpec.scala index 758452b47d..28b1dd3b08 100644 --- a/quill-core/src/test/scala/io/getquill/ast/OpinionSpec.scala +++ b/quill-core/src/test/scala/io/getquill/ast/OpinionSpec.scala @@ -3,21 +3,22 @@ package io.getquill.ast import io.getquill.Spec import io.getquill.ast.Renameable.Fixed import io.getquill.ast.Renameable.neutral +import io.getquill.ast.Visibility.Visible class OpinionSpec extends Spec { "properties should neutralize" - { "to renameable default" in { - Property.Opinionated(Ident("foo"), "bar", Fixed).neutralize mustEqual (Property.Opinionated(Ident("foo"), "bar", neutral)) + Property.Opinionated(Ident("foo"), "bar", Fixed, Visible).neutralize mustEqual (Property.Opinionated(Ident("foo"), "bar", neutral, Visible)) } "to renameable default when nested" in { - Property.Opinionated(Property.Opinionated(Ident("foo"), "bar", Fixed), "baz", Fixed).neutralize mustEqual ( - Property.Opinionated(Property.Opinionated(Ident("foo"), "bar", neutral), "baz", neutral) + Property.Opinionated(Property.Opinionated(Ident("foo"), "bar", Fixed, Visible), "baz", Fixed, Visible).neutralize mustEqual ( + Property.Opinionated(Property.Opinionated(Ident("foo"), "bar", neutral, Visible), "baz", neutral, Visible) ) } "when inside other AST elements" in { - Map(Property.Opinionated(Ident("foo"), "bar", Fixed), Ident("v"), Property.Opinionated(Ident("v"), "prop", Fixed)).neutralize mustEqual ( - Map(Property.Opinionated(Ident("foo"), "bar", neutral), Ident("v"), Property.Opinionated(Ident("v"), "prop", neutral)) + Map(Property.Opinionated(Ident("foo"), "bar", Fixed, Visible), Ident("v"), Property.Opinionated(Ident("v"), "prop", Fixed, Visible)).neutralize mustEqual ( + Map(Property.Opinionated(Ident("foo"), "bar", neutral, Visible), Ident("v"), Property.Opinionated(Ident("v"), "prop", neutral, Visible)) ) } } @@ -27,7 +28,7 @@ class OpinionSpec extends Spec { Entity.Opinionated("foo", Nil, Fixed).neutralize mustEqual (Entity("foo", Nil)) } "when inside other AST elements" in { - Map(Entity.Opinionated("foo", Nil, Fixed), Ident("v"), Property.Opinionated(Ident("v"), "prop", Fixed)).neutralize mustEqual ( + Map(Entity.Opinionated("foo", Nil, Fixed), Ident("v"), Property.Opinionated(Ident("v"), "prop", Fixed, Visible)).neutralize mustEqual ( Map(Entity("foo", Nil), Ident("v"), Property(Ident("v"), "prop")) ) } diff --git a/quill-core/src/test/scala/io/getquill/ast/StatefulTransformerSpec.scala b/quill-core/src/test/scala/io/getquill/ast/StatefulTransformerSpec.scala index 9a81edb757..f6ef48d45d 100644 --- a/quill-core/src/test/scala/io/getquill/ast/StatefulTransformerSpec.scala +++ b/quill-core/src/test/scala/io/getquill/ast/StatefulTransformerSpec.scala @@ -2,6 +2,7 @@ package io.getquill.ast import io.getquill.Spec import io.getquill.ast.Renameable.Fixed +import io.getquill.ast.Visibility.Visible class StatefulTransformerSpec extends Spec { @@ -311,10 +312,10 @@ class StatefulTransformerSpec extends Spec { } "property - fixed" in { - val ast: Ast = Property.Opinionated(Ident("a"), "b", Fixed) + val ast: Ast = Property.Opinionated(Ident("a"), "b", Fixed, Visible) Subject(Nil, Ident("a") -> Ident("a'"))(ast) match { case (at, att) => - at mustEqual Property.Opinionated(Ident("a'"), "b", Fixed) + at mustEqual Property.Opinionated(Ident("a'"), "b", Fixed, Visible) att.state mustEqual List(Ident("a")) } } diff --git a/quill-core/src/test/scala/io/getquill/ast/StatelessTransformerSpec.scala b/quill-core/src/test/scala/io/getquill/ast/StatelessTransformerSpec.scala index e846311fa3..0b5030f3df 100644 --- a/quill-core/src/test/scala/io/getquill/ast/StatelessTransformerSpec.scala +++ b/quill-core/src/test/scala/io/getquill/ast/StatelessTransformerSpec.scala @@ -2,6 +2,7 @@ package io.getquill.ast import io.getquill.Spec import io.getquill.ast.Renameable.Fixed +import io.getquill.ast.Visibility.Visible class StatelessTransformerSpec extends Spec { @@ -167,9 +168,9 @@ class StatelessTransformerSpec extends Spec { OnConflict.Properties(List(Property(Ident("a'"), "b"))) } "properties - fixed" in { - val target: OnConflict.Target = OnConflict.Properties(List(Property.Opinionated(Ident("a"), "b", Fixed))) + val target: OnConflict.Target = OnConflict.Properties(List(Property.Opinionated(Ident("a"), "b", Fixed, Visible))) Subject(Ident("a") -> Ident("a'"))(target) mustEqual - OnConflict.Properties(List(Property.Opinionated(Ident("a'"), "b", Fixed))) + OnConflict.Properties(List(Property.Opinionated(Ident("a'"), "b", Fixed, Visible))) } } @@ -214,9 +215,9 @@ class StatelessTransformerSpec extends Spec { } "property - fixed" in { - val ast: Ast = Property.Opinionated(Ident("a"), "b", Fixed) + val ast: Ast = Property.Opinionated(Ident("a"), "b", Fixed, Visible) Subject(Ident("a") -> Ident("a'"))(ast) mustEqual - Property.Opinionated(Ident("a'"), "b", Fixed) + Property.Opinionated(Ident("a'"), "b", Fixed, Visible) } "infix" in { diff --git a/quill-core/src/test/scala/io/getquill/norm/BetaReductionSpec.scala b/quill-core/src/test/scala/io/getquill/norm/BetaReductionSpec.scala index c8e4a33a11..abef40ac3a 100644 --- a/quill-core/src/test/scala/io/getquill/norm/BetaReductionSpec.scala +++ b/quill-core/src/test/scala/io/getquill/norm/BetaReductionSpec.scala @@ -2,6 +2,7 @@ package io.getquill.norm import io.getquill.Spec import io.getquill.ast.Renameable.Fixed +import io.getquill.ast.Visibility.Visible import io.getquill.ast._ class BetaReductionSpec extends Spec { @@ -12,7 +13,7 @@ class BetaReductionSpec extends Spec { BetaReduction(ast) mustEqual Ident("a") } "tuple field - fixed property" in { - val ast: Ast = Property.Opinionated(Tuple(List(Ident("a"))), "_1", Fixed) + val ast: Ast = Property.Opinionated(Tuple(List(Ident("a"))), "_1", Fixed, Visible) BetaReduction(ast) mustEqual Ident("a") } "caseclass field" in { @@ -20,7 +21,7 @@ class BetaReductionSpec extends Spec { BetaReduction(ast) mustEqual Ident("a") } "caseclass field - fixed property" in { - val ast: Ast = Property.Opinionated(CaseClass(List(("foo", Ident("a")))), "foo", Fixed) + val ast: Ast = Property.Opinionated(CaseClass(List(("foo", Ident("a")))), "foo", Fixed, Visible) BetaReduction(ast) mustEqual Ident("a") } "function apply" in { diff --git a/quill-core/src/test/scala/io/getquill/norm/ExpandReturningSpec.scala b/quill-core/src/test/scala/io/getquill/norm/ExpandReturningSpec.scala index 7d6e8ed0a3..0dc0c5ca57 100644 --- a/quill-core/src/test/scala/io/getquill/norm/ExpandReturningSpec.scala +++ b/quill-core/src/test/scala/io/getquill/norm/ExpandReturningSpec.scala @@ -3,6 +3,7 @@ package io.getquill.norm import io.getquill.ReturnAction.{ ReturnColumns, ReturnRecord } import io.getquill._ import io.getquill.ast.Renameable.ByStrategy +import io.getquill.ast.Visibility.Visible import io.getquill.ast._ import io.getquill.context.Expand @@ -114,14 +115,14 @@ class ExpandReturningSpec extends Spec { Map( Entity.Opinionated("Person", List(), renameable), Ident("p"), - Tuple(List(Property.Opinionated(Ident("p"), "name", renameable), Property.Opinionated(Ident("p"), "age", renameable))) + Tuple(List(Property.Opinionated(Ident("p"), "name", renameable, Visible), Property.Opinionated(Ident("p"), "age", renameable, Visible))) ), - List(Assignment(Ident("pp"), Property.Opinionated(Ident("pp"), "name", renameable), Constant("Joe"))) + List(Assignment(Ident("pp"), Property.Opinionated(Ident("pp"), "name", renameable, Visible), Constant("Joe"))) ) def retMulti = - Returning(insert, Ident("r"), Tuple(List(Property.Opinionated(Ident("r"), "name", renameable), Property.Opinionated(Ident("r"), "age", renameable)))) + Returning(insert, Ident("r"), Tuple(List(Property.Opinionated(Ident("r"), "name", renameable, Visible), Property.Opinionated(Ident("r"), "age", renameable, Visible)))) def retSingle = - Returning(insert, Ident("r"), Tuple(List(Property.Opinionated(Ident("r"), "name", renameable)))) + Returning(insert, Ident("r"), Tuple(List(Property.Opinionated(Ident("r"), "name", renameable, Visible)))) "returning single" - { val mi = MirrorIdiomReturningSingle diff --git a/quill-core/src/test/scala/io/getquill/quotation/IsDynamicSpec.scala b/quill-core/src/test/scala/io/getquill/quotation/IsDynamicSpec.scala index d859770649..a808f77bad 100644 --- a/quill-core/src/test/scala/io/getquill/quotation/IsDynamicSpec.scala +++ b/quill-core/src/test/scala/io/getquill/quotation/IsDynamicSpec.scala @@ -4,6 +4,7 @@ import io.getquill.Spec import io.getquill.ast.Dynamic import io.getquill.ast.Property import io.getquill.ast.Renameable.Fixed +import io.getquill.ast.Visibility.Visible import io.getquill.testContext.qr1 import io.getquill.testContext.qrRegular @@ -18,7 +19,7 @@ class IsDynamicSpec extends Spec { IsDynamic(Property(Dynamic(1), "a")) mustEqual true } "partially dynamic - fixed" in { - IsDynamic(Property.Opinionated(Dynamic(1), "a", Fixed)) mustEqual true + IsDynamic(Property.Opinionated(Dynamic(1), "a", Fixed, Visible)) mustEqual true } } "false" in { diff --git a/quill-orientdb/src/main/scala/io/getquill/context/orientdb/OrientDBIdiom.scala b/quill-orientdb/src/main/scala/io/getquill/context/orientdb/OrientDBIdiom.scala index ff61b32759..78fdbf7a4d 100644 --- a/quill-orientdb/src/main/scala/io/getquill/context/orientdb/OrientDBIdiom.scala +++ b/quill-orientdb/src/main/scala/io/getquill/context/orientdb/OrientDBIdiom.scala @@ -234,7 +234,7 @@ trait OrientDBIdiom extends Idiom { case Property(ast, "isEmpty") => stmt"${ast.token} IS NULL" case Property(ast, "nonEmpty") => stmt"${ast.token} IS NOT NULL" case Property(ast, "isDefined") => stmt"${ast.token} IS NOT NULL" - case Property.Opinionated(ast, name, renameable) => + case Property.Opinionated(ast, name, renameable, _) => renameable.fixedOr(name.token)(strategy.column(name).token) } } @@ -269,10 +269,10 @@ trait OrientDBIdiom extends Idiom { implicit def actionTokenizer(implicit strategy: NamingStrategy): Tokenizer[Action] = { implicit def propertyTokenizer: Tokenizer[Property] = Tokenizer[Property] { - case Property(Property.Opinionated(_, name, renameable), "isEmpty") => stmt"${renameable.fixedOr(name.token)(strategy.column(name).token)} IS NULL" - case Property(Property.Opinionated(_, name, renameable), "isDefined") => stmt"${renameable.fixedOr(name.token)(strategy.column(name).token)} IS NOT NULL" - case Property(Property.Opinionated(_, name, renameable), "nonEmpty") => stmt"${renameable.fixedOr(name.token)(strategy.column(name).token)} IS NOT NULL" - case Property.Opinionated(_, name, renameable) => renameable.fixedOr(name.token)(strategy.column(name).token) + case Property(Property.Opinionated(_, name, renameable, _), "isEmpty") => stmt"${renameable.fixedOr(name.token)(strategy.column(name).token)} IS NULL" + case Property(Property.Opinionated(_, name, renameable, _), "isDefined") => stmt"${renameable.fixedOr(name.token)(strategy.column(name).token)} IS NOT NULL" + case Property(Property.Opinionated(_, name, renameable, _), "nonEmpty") => stmt"${renameable.fixedOr(name.token)(strategy.column(name).token)} IS NOT NULL" + case Property.Opinionated(_, name, renameable, _) => renameable.fixedOr(name.token)(strategy.column(name).token) } Tokenizer[Action] { diff --git a/quill-spark/src/main/scala/io/getquill/context/spark/SparkDialect.scala b/quill-spark/src/main/scala/io/getquill/context/spark/SparkDialect.scala index 564fa4497f..1b908b6628 100644 --- a/quill-spark/src/main/scala/io/getquill/context/spark/SparkDialect.scala +++ b/quill-spark/src/main/scala/io/getquill/context/spark/SparkDialect.scala @@ -139,7 +139,7 @@ trait SparkIdiom extends SqlIdiom with CannotReturn { self => def path(ast: Ast): Token = ast match { case Ident(name) => name.token - case Property.Opinionated(a, b, renameable) => + case Property.Opinionated(a, b, renameable, _) => stmt"${path(a)}.${renameable.fixedOr(b.token)(strategy.column(b).token)}" case other => other.token diff --git a/quill-sql/src/main/scala/io/getquill/MySQLDialect.scala b/quill-sql/src/main/scala/io/getquill/MySQLDialect.scala index 5e98f91e5e..9feeed94be 100644 --- a/quill-sql/src/main/scala/io/getquill/MySQLDialect.scala +++ b/quill-sql/src/main/scala/io/getquill/MySQLDialect.scala @@ -54,12 +54,13 @@ trait MySQLDialect fail("This upsert construct is not supported in MySQL. Please refer documentation for details.") } + // TODO Are there situations where you could have invisible properties here? val customAstTokenizer = Tokenizer.withFallback[Ast](MySQLDialect.this.astTokenizer(_, strategy)) { - case Property.Opinionated(Excluded(_), name, renameable) => + case Property.Opinionated(Excluded(_), name, renameable, _) => renameable.fixedOr(name.token)(stmt"VALUES(${strategy.column(name).token})") - case Property.Opinionated(_, name, renameable) => + case Property.Opinionated(_, name, renameable, _) => renameable.fixedOr(name.token)(strategy.column(name).token) } diff --git a/quill-sql/src/main/scala/io/getquill/context/sql/idiom/SqlIdiom.scala b/quill-sql/src/main/scala/io/getquill/context/sql/idiom/SqlIdiom.scala index 0a557cce44..9c801e62a7 100644 --- a/quill-sql/src/main/scala/io/getquill/context/sql/idiom/SqlIdiom.scala +++ b/quill-sql/src/main/scala/io/getquill/context/sql/idiom/SqlIdiom.scala @@ -9,6 +9,7 @@ import io.getquill.idiom._ import io.getquill.idiom.StatementInterpolator._ import io.getquill.NamingStrategy import io.getquill.ast.Renameable.Fixed +import io.getquill.ast.Visibility.Hidden import io.getquill.context.{ ReturningCapability, ReturningClauseSupported } import io.getquill.util.Interleave import io.getquill.util.Messages.{ fail, trace } @@ -316,14 +317,15 @@ trait SqlIdiom extends Idiom { def unnest(ast: Ast): (Ast, List[String]) = ast match { - case Property(a, name) if (name.matches("_[0-9]*")) => + case Property.Opinionated(a, _, _, Hidden) => unnest(a) match { - case (ast, nestedName) => - (ast, nestedName :+ name) + case (a, nestedName) => (a, nestedName) } + // Append the property name. This includes tuple indexes. case Property(a, name) => unnest(a) match { - case (a, nestedName) => (a, nestedName) + case (ast, nestedName) => + (ast, nestedName :+ name) } case a => (a, Nil) } @@ -334,7 +336,7 @@ trait SqlIdiom extends Idiom { )(tokenizeColumn(strategy, prefix.mkString + name, renameable).token) Tokenizer[Property] { - case Property.Opinionated(ast, name, renameable) => + case Property.Opinionated(ast, name, renameable, _ /* Top level property cannot be invisible */ ) => // When we have things like Embedded tables, properties inside of one another needs to be un-nested. // E.g. in `Property(Property(Ident("realTable"), embeddedTableAlias), realPropertyAlias)` the inner // property needs to be unwrapped and the result of this should only be `realTable.realPropertyAlias` @@ -397,10 +399,10 @@ trait SqlIdiom extends Idiom { protected def actionAstTokenizer(implicit astTokenizer: Tokenizer[Ast], strategy: NamingStrategy) = Tokenizer.withFallback[Ast](SqlIdiom.this.astTokenizer(_, strategy)) { case q: Query => astTokenizer.token(q) - case Property(Property.Opinionated(_, name, renameable), "isEmpty") => stmt"${renameable.fixedOr(name)(tokenizeColumn(strategy, name, renameable)).token} IS NULL" - case Property(Property.Opinionated(_, name, renameable), "isDefined") => stmt"${renameable.fixedOr(name)(tokenizeColumn(strategy, name, renameable)).token} IS NOT NULL" - case Property(Property.Opinionated(_, name, renameable), "nonEmpty") => stmt"${renameable.fixedOr(name)(tokenizeColumn(strategy, name, renameable)).token} IS NOT NULL" - case Property.Opinionated(_, name, renameable) => renameable.fixedOr(name.token)(tokenizeColumn(strategy, name, renameable).token) + case Property(Property.Opinionated(_, name, renameable, _), "isEmpty") => stmt"${renameable.fixedOr(name)(tokenizeColumn(strategy, name, renameable)).token} IS NULL" + case Property(Property.Opinionated(_, name, renameable, _), "isDefined") => stmt"${renameable.fixedOr(name)(tokenizeColumn(strategy, name, renameable)).token} IS NOT NULL" + case Property(Property.Opinionated(_, name, renameable, _), "nonEmpty") => stmt"${renameable.fixedOr(name)(tokenizeColumn(strategy, name, renameable)).token} IS NOT NULL" + case Property.Opinionated(_, name, renameable, _) => renameable.fixedOr(name.token)(tokenizeColumn(strategy, name, renameable).token) } def returnListTokenizer(implicit tokenizer: Tokenizer[Ast], strategy: NamingStrategy): Tokenizer[List[Ast]] = { diff --git a/quill-sql/src/main/scala/io/getquill/context/sql/norm/ExpandNestedQueries.scala b/quill-sql/src/main/scala/io/getquill/context/sql/norm/ExpandNestedQueries.scala index 33a4eae166..958cb42392 100644 --- a/quill-sql/src/main/scala/io/getquill/context/sql/norm/ExpandNestedQueries.scala +++ b/quill-sql/src/main/scala/io/getquill/context/sql/norm/ExpandNestedQueries.scala @@ -5,22 +5,16 @@ import io.getquill.ast.Ast import io.getquill.ast.Ident import io.getquill.ast._ import io.getquill.ast.StatefulTransformer -import io.getquill.context.sql.FlattenSqlQuery -import io.getquill.context.sql.FromContext -import io.getquill.context.sql.InfixContext -import io.getquill.context.sql.JoinContext -import io.getquill.context.sql.QueryContext -import io.getquill.context.sql.SetOperationSqlQuery -import io.getquill.context.sql.SqlQuery -import io.getquill.context.sql.TableContext -import io.getquill.context.sql.UnaryOperationSqlQuery -import io.getquill.context.sql.FlatJoinContext +import io.getquill.ast.Visibility.Visible +import io.getquill.context.sql._ import scala.collection.mutable.LinkedHashSet import io.getquill.util.Interpolator import io.getquill.util.Messages.TraceType.NestedQueryExpansion import io.getquill.context.sql.norm.nested.ExpandSelect +import scala.collection.mutable + class ExpandNestedQueries(strategy: NamingStrategy) { val interp = new Interpolator(NestedQueryExpansion, 3) @@ -47,19 +41,41 @@ class ExpandNestedQueries(strategy: NamingStrategy) { q match { case FlattenSqlQuery(from, where, groupBy, orderBy, limit, offset, select, distinct) => val asts = Nil ++ select.map(_.ast) ++ where ++ groupBy ++ orderBy.map(_.ast) ++ limit ++ offset - val from = q.from.map(expandContext(_, asts)) - q.copy(from = from) + val expansions = q.from.map(expandContext(_, asts)) + val from = expansions.map(_._1) + val references = expansions.flatMap(_._2) + val modifiedSelects = + select + .map(s => + if (references.contains(s.ast)) + trace"Un-hide Select $s:" andReturn unhideProperties(s) + else s) + + q.copy(select = modifiedSelects, from = from) } - private def expandContext(s: FromContext, asts: List[Ast]): FromContext = + private def unhideProperties(sv: SelectValue) = { + def unhideRecurse(ast: Ast): Ast = + Transform(ast) { + case Property.Opinionated(a, n, r, v) => + Property.Opinionated(unhideRecurse(a), n, r, Visible) + } + sv.copy(ast = unhideRecurse(sv.ast)) + } + + private def expandContext(s: FromContext, asts: List[Ast]): (FromContext, LinkedHashSet[Property]) = s match { case QueryContext(q, alias) => - QueryContext(apply(q, references(alias, asts)), alias) + val refs = references(alias, asts) + (QueryContext(apply(q, refs), alias), refs) case JoinContext(t, a, b, on) => - JoinContext(t, expandContext(a, asts :+ on), expandContext(b, asts :+ on), on) + val (left, leftRefs) = expandContext(a, asts :+ on) + val (right, rightRefs) = expandContext(b, asts :+ on) + (JoinContext(t, left, right, on), leftRefs ++ rightRefs) case FlatJoinContext(t, a, on) => - FlatJoinContext(t, expandContext(a, asts :+ on), on) - case _: TableContext | _: InfixContext => s + val (next, refs) = expandContext(a, asts :+ on) + (FlatJoinContext(t, next, on), refs) + case _: TableContext | _: InfixContext => (s, new mutable.LinkedHashSet[Property]()) } private def references(alias: String, asts: List[Ast]) = diff --git a/quill-sql/src/main/scala/io/getquill/context/sql/norm/nested/ExpandSelect.scala b/quill-sql/src/main/scala/io/getquill/context/sql/norm/nested/ExpandSelect.scala index dbe1baf13b..12d4f6981f 100644 --- a/quill-sql/src/main/scala/io/getquill/context/sql/norm/nested/ExpandSelect.scala +++ b/quill-sql/src/main/scala/io/getquill/context/sql/norm/nested/ExpandSelect.scala @@ -88,7 +88,7 @@ private class ExpandSelect(selectValues: List[SelectValue], references: LinkedHa trace"Appending $idx to $alias " andReturn OrderedSelect(o, SelectValue(ast, concat(alias, idx), c)) } - case pp @ Property.Opinionated(ast: Property, name, renameable) => + case pp @ Property.Opinionated(ast: Property, name, renameable, visible) => trace"Reference is a sub-property. Walking inside." andContinue expandReference(ast) match { case OrderedSelect(o, SelectValue(ast, nested, c)) => @@ -108,8 +108,10 @@ private class ExpandSelect(selectValues: List[SelectValue], references: LinkedHa // into the property path. OrderedSelect(o, SelectValue( - Property.Opinionated(ast, name, renameable), - Some(s"${nested.flatMap(expressIfTupleIndex(_)).getOrElse("")}${expandColumn(name, renameable)}"), c + // Note: Pass invisible properties to be tokenized by the idiom, they should be excluded there + Property.Opinionated(ast, name, renameable, visible), + // Skip concatonation of invisible properties into the alias e.g. so it will be + Some(s"${nested.getOrElse("")}${expandColumn(name, renameable)}") )) } case pp @ Property(_, TupleIndex(idx)) => @@ -118,7 +120,7 @@ private class ExpandSelect(selectValues: List[SelectValue], references: LinkedHa case OrderedSelect(o, SelectValue(ast, alias, c)) => OrderedSelect(o, SelectValue(ast, concat(alias, idx), c)) } - case pp @ Property.Opinionated(_, name, renameable) => + case pp @ Property.Opinionated(_, name, renameable, visible) => select match { case List(OrderedSelect(o, SelectValue(cc: CaseClass, alias, c))) => // Currently case class element name is not being appended. Need to change that in order to ensure @@ -131,7 +133,7 @@ private class ExpandSelect(selectValues: List[SelectValue], references: LinkedHa OrderedSelect(o :+ index, SelectValue(ast, Some(expandColumn(name, renameable)), c)) case List(OrderedSelect(o, SelectValue(i: Ident, _, c))) => trace"Reference is an identifier: " andReturn - OrderedSelect(o, SelectValue(Property.Opinionated(i, name, renameable), None, c)) + OrderedSelect(o, SelectValue(Property.Opinionated(i, name, renameable, visible), None, c)) case other => trace"Reference is unidentified: " andReturn OrderedSelect(Integer.MAX_VALUE, SelectValue(Ident(name), Some(expandColumn(name, renameable)), false)) diff --git a/quill-sql/src/test/scala/io/getquill/context/sql/EmbeddedSpec.scala b/quill-sql/src/test/scala/io/getquill/context/sql/EmbeddedSpec.scala index 6b7f1f4905..05bfe4f9cf 100644 --- a/quill-sql/src/test/scala/io/getquill/context/sql/EmbeddedSpec.scala +++ b/quill-sql/src/test/scala/io/getquill/context/sql/EmbeddedSpec.scala @@ -14,7 +14,45 @@ class EmbeddedSpec extends Spec { val q = quote { query[Emb].map(e => Parent(1, e)).distinct } - ctx.run(q).string mustEqual "SELECT e.id, e.a, e.b FROM (SELECT DISTINCT 1 AS id, e.a AS a, e.b AS b FROM Emb e) AS e" + ctx.run(q).string mustEqual "SELECT e.id, e.emb1a, e.emb1b FROM (SELECT DISTINCT 1 AS id, e.a AS emb1a, e.b AS emb1b FROM Emb e) AS e" + } + + "function property inside of nested distinct queries - tuple" in { + case class Parent(id: Int, emb1: Emb) + case class Emb(a: Int, b: Int) extends Embedded + val q = quote { + query[Emb].map(e => Parent(1, e)).distinct.map(p => (2, p)).distinct + } + ctx.run(q).string mustEqual "SELECT p._1, p._2id, p._2emb1a, p._2emb1b FROM (SELECT DISTINCT 2 AS _1, p.id AS _2id, p.emb1a AS _2emb1a, p.emb1b AS _2emb1b FROM (SELECT DISTINCT 1 AS id, e.a AS emb1a, e.b AS emb1b FROM Emb e) AS p) AS p" + } + + "function property inside of nested distinct queries through tuples" in { + case class Parent(id: Int, emb1: Emb) + case class Emb(a: Int, b: Int) extends Embedded + val q = quote { + query[Emb].map(e => (1, e)).distinct.map(t => Parent(t._1, t._2)).distinct + } + ctx.run(q).string mustEqual "SELECT t.id, t.emb1a, t.emb1b FROM (SELECT DISTINCT t._1 AS id, t._2a AS emb1a, t._2b AS emb1b FROM (SELECT DISTINCT 1 AS _1, e.a AS _2a, e.b AS _2b FROM Emb e) AS t) AS t" + } + + "function property inside of nested distinct queries - twice" in { + case class Grandparent(idG: Int, par: Parent) + case class Parent(idP: Int, emb1: Emb) extends Embedded + case class Emb(a: Int, b: Int) extends Embedded + val q = quote { + query[Emb].map(e => Parent(1, e)).distinct.map(p => Grandparent(2, p)).distinct + } + ctx.run(q).string mustEqual "SELECT p.idG, p.paridP, p.paremb1a, p.paremb1b FROM (SELECT DISTINCT 2 AS idG, p.idP AS paridP, p.emb1a AS paremb1a, p.emb1b AS paremb1b FROM (SELECT DISTINCT 1 AS idP, e.a AS emb1a, e.b AS emb1b FROM Emb e) AS p) AS p" + } + + "function property inside of nested distinct queries - twice - into tuple" in { + case class Grandparent(idG: Int, par: Parent) + case class Parent(idP: Int, emb1: Emb) extends Embedded + case class Emb(a: Int, b: Int) extends Embedded + val q = quote { + query[Emb].map(e => Parent(1, e)).distinct.map(p => Grandparent(2, p)).distinct.map(g => (3, g)).distinct + } + ctx.run(q).string mustEqual "SELECT g._1, g._2idG, g._2paridP, g._2paremb1a, g._2paremb1b FROM (SELECT DISTINCT 3 AS _1, g.idG AS _2idG, g.paridP AS _2paridP, g.paremb1a AS _2paremb1a, g.paremb1b AS _2paremb1b FROM (SELECT DISTINCT 2 AS idG, p.idP AS paridP, p.emb1a AS paremb1a, p.emb1b AS paremb1b FROM (SELECT DISTINCT 1 AS idP, e.a AS emb1a, e.b AS emb1b FROM Emb e) AS p) AS g) AS g" } }