Skip to content
Permalink
Browse files

Remove misguided performance optimization from NominalType.

Putting the structural view into a second parameter list was done to
speed up equality and hash code computation for NominalType, on the
grounds that structural views have to be consistent anyway throughout
the tree. However, this invariant only holds between phases. Inside a
phase, the structural expansions may be temporarily inconsistent,
causing Node.nodeTypedOrCopy to overwrite a type that has already been
seen when replacing a NominalType with a new NominalType that has the
same TypeSymbol but a different expansion.

Fixes #912. Test in UnionTest.testBasic.
  • Loading branch information
szeiger committed Jul 16, 2014
1 parent 2b5e2e1 commit 1aa13839095172eeba5b44f67a941251b90436fe
@@ -58,6 +58,12 @@ class UnionTest extends TestkitTest[RelationalTestDB] {
q3.run.foreach(o => println(" "+o))
assertEquals(List((2,"Amy"), (7,"Ben"), (8,"Greg"), (6,"Leonard"), (3,"Steve")), q3.run)

val q4 = managers.map(_.id)
val q4b = q4 union q4
assertEquals(Set(1, 2, 3), q4b.run.to[Set])
val q4c = q4 union q4 union q4
assertEquals(Set(1, 2, 3), q4c.run.to[Set])

(managers.ddl ++ employees.ddl).drop
}

@@ -274,7 +274,7 @@ final case class Pure(value: Node, identity: TypeSymbol = new AnonTypeSymbol) ex
def withComputedTypeNoRec: Self = nodeBuildTypedNode(this, buildType)
protected def buildType =
CollectionType(TypedCollectionTypeConstructor.seq,
NominalType(identity)(value.nodeType))
NominalType(identity, value.nodeType))
}

final case class CollectionCast(child: Node, cons: CollectionTypeConstructor) extends UnaryNode with SimplyTypedNode {
@@ -370,7 +370,7 @@ final case class GroupBy(fromGen: Symbol, from: Node, by: Node, identity: TypeSy
val by2 = by.nodeWithComputedType(scope + (fromGen -> from2Type.elementType), typeChildren, retype)
nodeRebuildOrThis(Vector(from2, by2)).nodeTypedOrCopy(
if(!nodeHasType || retype)
CollectionType(from2Type.cons, ProductType(IndexedSeq(NominalType(identity)(by2.nodeType), CollectionType(TypedCollectionTypeConstructor.seq, from2Type.elementType))))
CollectionType(from2Type.cons, ProductType(IndexedSeq(NominalType(identity, by2.nodeType), CollectionType(TypedCollectionTypeConstructor.seq, from2Type.elementType))))
else nodeType)
}
}
@@ -531,7 +531,7 @@ object FwdPath {
/** A Node representing a database table. */
final case class TableNode(schemaName: Option[String], tableName: String, identity: TableIdentitySymbol, driverTable: Any, baseIdentity: TableIdentitySymbol) extends NullaryNode with TypedNode {
type Self = TableNode
def tpe = CollectionType(TypedCollectionTypeConstructor.seq, NominalType(identity)(UnassignedStructuralType(identity)))
def tpe = CollectionType(TypedCollectionTypeConstructor.seq, NominalType(identity, UnassignedStructuralType(identity)))
def nodeRebuild = copy()
override def getDumpInfo = super.getDumpInfo.copy(name = "Table", mainInfo = schemaName.map(_ + ".").getOrElse("") + tableName)
}
@@ -178,17 +178,17 @@ final case class UnassignedStructuralType(sym: TypeSymbol) extends AtomicType {
* of the structural view but must update the AST at the end of the phase
* so that all NominalTypes with the same symbol have the same structural
* view. */
final case class NominalType(sym: TypeSymbol)(val structuralView: Type) extends Type {
final case class NominalType(sym: TypeSymbol, structuralView: Type) extends Type {
def toShortString = s"NominalType($sym)"
override def toString = s"$toShortString($structuralView)"
def withStructuralView(t: Type): NominalType =
if(t == structuralView) this else copy()(t)
if(t == structuralView) this else copy(structuralView = t)
override def structural: Type = structuralView.structural
override def select(sym: Symbol): Type = structuralView.select(sym)
def mapChildren(f: Type => Type): NominalType = {
val struct2 = f(structuralView)
if(struct2 eq structuralView) this
else new NominalType(sym)(struct2)
else new NominalType(sym, struct2)
}
def children: Seq[Type] = Seq(structuralView)
def sourceNominalType: NominalType = structuralView match {
@@ -13,7 +13,7 @@ class ExpandTables extends Phase {
def apply(state: CompilerState) = state.map { n => ClientSideOp.mapServerSide(n) { tree =>
// Check for table types
val tsyms: Set[TableIdentitySymbol] =
tree.nodeType.collect { case NominalType(sym: TableIdentitySymbol) => sym }.toSet
tree.nodeType.collect { case NominalType(sym: TableIdentitySymbol, _) => sym }.toSet
logger.debug("Tables for expansion in result type: " + tsyms.mkString(", "))
if(tsyms.isEmpty) tree else {
// Find the corresponding TableExpansions
@@ -33,7 +33,7 @@ class ExpandTables extends Phase {
def createResult(expansions: Map[TableIdentitySymbol, (Symbol, Node)], path: List[Symbol], tpe: Type): Node = tpe match {
case p: ProductType =>
ProductNode(p.numberedElements.map { case (s, t) => createResult(expansions, s :: path, t) }.toVector)
case NominalType(tsym: TableIdentitySymbol) if expansions contains tsym =>
case NominalType(tsym: TableIdentitySymbol, _) if expansions contains tsym =>
val (sym, exp) = expansions(tsym)
val p = Path(path)
exp.replace { case Ref(s) if s == sym => p }
@@ -91,7 +91,7 @@ class FlattenProjections extends Phase {
logger.debug("Found "+Path.toString(path)+" with local part "+rest.map(Path.toString _)+" over "+tsym)
val (paths, tpe) = translations(tsym)
def retype(n: Node): Node = n.nodeMapChildren(retype, keepType = true).nodeTypedOrCopy(n.nodeType.replace {
case t @ NominalType(tsym) if translations.contains(tsym) =>
case t @ NominalType(tsym, _) if translations.contains(tsym) =>
t.withStructuralView(tpe)
})
rest match {
@@ -116,7 +116,7 @@ class FlattenProjections extends Phase {
* (possibly empty) path of symbols on top of `base`. */
def splitPath(n: Node, candidates: scala.collection.Set[TypeSymbol]): Option[(Node, Option[List[Symbol]], TypeSymbol)] = {
def checkType(tpe: Type): Option[(Node, Option[List[Symbol]], TypeSymbol)] = tpe match {
case NominalType(tsym) if candidates contains tsym => Some((n, Some(Nil), tsym))
case NominalType(tsym, _) if candidates contains tsym => Some((n, Some(Nil), tsym))
case CollectionType(cons, el) => checkType(el).map { case (n, _, tsym) => (n, None, tsym) }
case _ => None
}
@@ -193,12 +193,12 @@ class PruneFields extends Phase {

def apply(state: CompilerState) = state.map { n => ClientSideOp.mapServerSide(n){ n =>
val top = n.nodeType.asCollectionType.elementType.asInstanceOf[NominalType].sym
val referenced = n.collect[(TypeSymbol, Symbol)] { case Select(_ :@ NominalType(s), f) => (s, f) }.toSet
val allTSyms = n.collect[TypeSymbol] { case Pure(_, _) :@ CollectionType(_, NominalType(ts)) => ts }.toSet
val referenced = n.collect[(TypeSymbol, Symbol)] { case Select(_ :@ NominalType(s, _), f) => (s, f) }.toSet
val allTSyms = n.collect[TypeSymbol] { case Pure(_, _) :@ CollectionType(_, NominalType(ts, _)) => ts }.toSet
val unrefTSyms = allTSyms -- referenced.map(_._1)
logger.debug(s"Result tsym: $top; Unreferenced: ${unrefTSyms.mkString(", ")}; Field refs: ${referenced.mkString(", ")}")
def tr(n: Node): Node = n.replace {
case (p @ Pure(s @ StructNode(ch), pts)) :@ CollectionType(_, NominalType(ts)) =>
case (p @ Pure(s @ StructNode(ch), pts)) :@ CollectionType(_, NominalType(ts, _)) =>
if(unrefTSyms contains ts) {
val ch2 = s.nodeChildren.map(tr)
val res = Pure(if(ch2.length == 1 && ts != top) ch2(0) else ProductNode(ch2), pts)
@@ -17,7 +17,7 @@ class CreateResultSetMapping extends Phase {
val n3 = toCollection(n2)
logger.debug("Converted to collection:", n3)
val tables: Map[TypeSymbol, Node] = n.collect {
case TableExpansion(_, _, columns) :@ CollectionType(_, NominalType(ts)) =>
case TableExpansion(_, _, columns) :@ CollectionType(_, NominalType(ts, _)) =>
ts -> columns
}.toMap
logger.debug("Found tables: "+tables)
@@ -75,7 +75,7 @@ class CreateResultSetMapping extends Phase {
ProductNode(ch.map { case (_, t) => f(t) })
case t: MappedScalaType =>
TypeMapping(f(t.baseType), t.mapper, t.classTag)
case n @ NominalType(ts) => tables.get(ts) match {
case n @ NominalType(ts, _) => tables.get(ts) match {
case Some(n) => f(n.nodeType)
case None => f(n.structuralView)
}

0 comments on commit 1aa1383

Please sign in to comment.
You can’t perform that action at this time.