Skip to content

Commit

Permalink
Fixing Embedded Coproduct Column Duplication Issue
Browse files Browse the repository at this point in the history
  • Loading branch information
deusaquilus committed Sep 12, 2019
1 parent 45e604e commit 86f082c
Show file tree
Hide file tree
Showing 26 changed files with 191 additions and 79 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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] {
Expand Down
11 changes: 9 additions & 2 deletions quill-core/src/main/scala/io/getquill/AstPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions quill-core/src/main/scala/io/getquill/MirrorIdiom.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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] {
Expand Down
22 changes: 18 additions & 4 deletions quill-core/src/main/scala/io/getquill/ast/Ast.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -179,31 +187,37 @@ 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 {
def apply(ast: Ast, name: String) = new Property(ast, name)
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))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand All @@ -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] {
Expand Down
13 changes: 12 additions & 1 deletion quill-core/src/main/scala/io/getquill/quotation/Parsing.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]] {
Expand Down
13 changes: 7 additions & 6 deletions quill-core/src/test/scala/io/getquill/ast/OpinionSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
)
}
}
Expand All @@ -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"))
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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"))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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)))
}
}

Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -12,15 +13,15 @@ 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 {
val ast: Ast = Property(CaseClass(List(("foo", Ident("a")))), "foo")
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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 86f082c

Please sign in to comment.