Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing Embedded Coproduct Column Duplication Issue #1604

Merged
merged 1 commit into from
Sep 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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