Merge pull request #1259 from slick/tmp/issue-1129

More intelligent handling of String types on MySQL

slick-testkit/src/main/scala/com/typesafe/slick/testkit/tests/JdbcMiscTest.scala

@@ -9,7 +9,7 @@ class JdbcMiscTest extends AsyncTest[JdbcTestDB] {
 
   def testNullability = {
     class T1(tag: Tag) extends Table[String](tag, "t1") {
-      def a = column[String]("a")
+      def a = column[String]("a", O.PrimaryKey)
       def * = a
     }
     val t1 = TableQuery[T1]

slick/src/main/resources/reference.conf

@@ -20,6 +20,7 @@ slick {
 }
 
 slick.driver.MySQL {
-  # The default SQL type for strings without an explicit size limit
-  defaultStringType = "TEXT"
+  # The default SQL type for strings without an explicit size limit.
+  # When set to null / undefined, pick "TEXT" where possible, otherwise fall back to "VARCHAR(254)"
+  defaultStringType = null
 }

slick/src/main/scala/slick/ast/Symbol.scala

@@ -3,6 +3,7 @@ package slick.ast
 import Util._
 import slick.util.ConstArray
 import scala.collection.mutable.HashMap
+import scala.reflect.ClassTag
 import scala.util.DynamicVariable
 
 /** A symbol which can be used in the AST. It can be either a TypeSymbol or a TermSymbol. */
@@ -18,7 +19,10 @@ trait TypeSymbol extends Symbol
 trait TermSymbol extends Symbol
 
 /** A named symbol which refers to an (aliased or unaliased) field. */
-case class FieldSymbol(name: String)(val options: Seq[ColumnOption[_]], val tpe: Type) extends TermSymbol
+case class FieldSymbol(name: String)(val options: Seq[ColumnOption[_]], val tpe: Type) extends TermSymbol {
+  def findColumnOption[T <: ColumnOption[_]](implicit ct: ClassTag[T]): Option[T] =
+    options.find(ct.runtimeClass.isInstance _).asInstanceOf[Option[T]]
+}
 
 /** An element of a ProductNode (using a 1-based index) */
 case class ElementSymbol(idx: Int) extends TermSymbol {

slick/src/main/scala/slick/driver/DerbyDriver.scala

@@ -108,11 +108,11 @@ trait DerbyDriver extends JdbcDriver { driver =>
   override def createColumnDDLBuilder(column: FieldSymbol, table: Table[_]): ColumnDDLBuilder = new ColumnDDLBuilder(column)
   override def createSequenceDDLBuilder(seq: Sequence[_]): SequenceDDLBuilder[_] = new SequenceDDLBuilder(seq)
 
-  override def defaultSqlTypeName(tmd: JdbcType[_], size: Option[RelationalProfile.ColumnOption.Length]): String = tmd.sqlType match {
+  override def defaultSqlTypeName(tmd: JdbcType[_], sym: Option[FieldSymbol]): String = tmd.sqlType match {
     case java.sql.Types.BOOLEAN => "SMALLINT"
     /* Derby does not have a TINYINT type, so we use SMALLINT instead. */
     case java.sql.Types.TINYINT => "SMALLINT"
-    case _ => super.defaultSqlTypeName(tmd, size)
+    case _ => super.defaultSqlTypeName(tmd, sym)
   }
 
   override val scalarFrom = Some("sysibm.sysdummy1")
@@ -221,7 +221,7 @@ trait DerbyDriver extends JdbcDriver { driver =>
 
     class UUIDJdbcType extends super.UUIDJdbcType {
       override def sqlType = java.sql.Types.BINARY
-      override def sqlTypeName(size: Option[RelationalProfile.ColumnOption.Length]) = "CHAR(16) FOR BIT DATA"
+      override def sqlTypeName(sym: Option[FieldSymbol]) = "CHAR(16) FOR BIT DATA"
     }
   }
 }

slick/src/main/scala/slick/driver/H2Driver.scala

@@ -73,10 +73,11 @@ trait H2Driver extends JdbcDriver { driver =>
   override def createInsertActionExtensionMethods[T](compiled: CompiledInsert): InsertActionExtensionMethods[T] =
     new CountingInsertActionComposerImpl[T](compiled)
 
-  override def defaultSqlTypeName(tmd: JdbcType[_], size: Option[RelationalProfile.ColumnOption.Length]): String = tmd.sqlType match {
+  override def defaultSqlTypeName(tmd: JdbcType[_], sym: Option[FieldSymbol]): String = tmd.sqlType match {
     case java.sql.Types.VARCHAR =>
+      val size = sym.flatMap(_.findColumnOption[RelationalProfile.ColumnOption.Length])
       size.fold("VARCHAR")(l => if(l.varying) s"VARCHAR(${l.length})" else s"CHAR(${l.length})")
-    case _ => super.defaultSqlTypeName(tmd, size)
+    case _ => super.defaultSqlTypeName(tmd, sym)
   }
 
   class QueryBuilder(tree: Node, state: CompilerState) extends super.QueryBuilder(tree, state) {
@@ -102,7 +103,7 @@ trait H2Driver extends JdbcDriver { driver =>
 
   class JdbcTypes extends super.JdbcTypes {
     override val uuidJdbcType = new UUIDJdbcType {
-      override def sqlTypeName(size: Option[RelationalProfile.ColumnOption.Length]) = "UUID"
+      override def sqlTypeName(sym: Option[FieldSymbol]) = "UUID"
       override def valueToSQLLiteral(value: UUID) = "'" + value + "'"
       override def hasLiteralForm = true
     }

slick/src/main/scala/slick/driver/HsqldbDriver.scala

@@ -113,11 +113,11 @@ trait HsqldbDriver extends JdbcDriver { driver =>
 
   class JdbcTypes extends super.JdbcTypes {
     override val byteArrayJdbcType = new ByteArrayJdbcType {
-      override def sqlTypeName(size: Option[RelationalProfile.ColumnOption.Length]) = "LONGVARBINARY"
+      override def sqlTypeName(sym: Option[FieldSymbol]) = "LONGVARBINARY"
     }
     override val uuidJdbcType = new UUIDJdbcType {
       override def sqlType = java.sql.Types.BINARY
-      override def sqlTypeName(size: Option[RelationalProfile.ColumnOption.Length]) = "BINARY(16)"
+      override def sqlTypeName(sym: Option[FieldSymbol]) = "BINARY(16)"
     }
   }
 

slick/src/main/scala/slick/driver/JdbcStatementBuilderComponent.scala

@@ -675,7 +675,7 @@ trait JdbcStatementBuilderComponent { driver: JdbcDriver =>
       if(sqlType ne null) {
         size.foreach(l => sqlType += s"($l)")
         customSqlType = true
-      } else sqlType = jdbcType.sqlTypeName(size.map(l => RelationalProfile.ColumnOption.Length(l, varying)))
+      } else sqlType = jdbcType.sqlTypeName(Some(column))
     }
 
     protected def handleColumnOption(o: ColumnOption[_]): Unit = o match {

slick/src/main/scala/slick/driver/JdbcTypesComponent.scala

@@ -15,12 +15,12 @@ trait JdbcTypesComponent extends RelationalTypesComponent { driver: JdbcDriver =
     def comap(u: U): T
 
     def newSqlType: Option[Int] = None
-    def newSqlTypeName(size: Option[RelationalProfile.ColumnOption.Length]): Option[String] = None
+    def newSqlTypeName(size: Option[FieldSymbol]): Option[String] = None
     def newValueToSQLLiteral(value: T): Option[String] = None
     def newHasLiteralForm: Option[Boolean] = None
 
     def sqlType = newSqlType.getOrElse(tmd.sqlType)
-    def sqlTypeName(size: Option[RelationalProfile.ColumnOption.Length]) = newSqlTypeName(size).getOrElse(tmd.sqlTypeName(size))
+    def sqlTypeName(sym: Option[FieldSymbol]) = newSqlTypeName(sym).getOrElse(tmd.sqlTypeName(sym))
     def setValue(v: T, p: PreparedStatement, idx: Int) = tmd.setValue(map(v), p, idx)
     def setNull(p: PreparedStatement, idx: Int): Unit = tmd.setNull(p, idx)
     def getValue(r: ResultSet, idx: Int) = {
@@ -73,8 +73,9 @@ trait JdbcTypesComponent extends RelationalTypesComponent { driver: JdbcDriver =
     case t => throw new SlickException("JdbcProfile has no JdbcType for type "+t)
   }): JdbcType[_]).asInstanceOf[JdbcType[Any]]
 
-  def defaultSqlTypeName(tmd: JdbcType[_], size: Option[RelationalProfile.ColumnOption.Length]): String = tmd.sqlType match {
+  def defaultSqlTypeName(tmd: JdbcType[_], sym: Option[FieldSymbol]): String = tmd.sqlType match {
     case java.sql.Types.VARCHAR =>
+      val size = sym.flatMap(_.findColumnOption[RelationalProfile.ColumnOption.Length])
       size.fold("VARCHAR(254)")(l => if(l.varying) s"VARCHAR(${l.length})" else s"CHAR(${l.length})")
     case java.sql.Types.DECIMAL => "DECIMAL(21,2)"
     case t => JdbcTypesComponent.typeNames.getOrElse(t,
@@ -83,7 +84,7 @@ trait JdbcTypesComponent extends RelationalTypesComponent { driver: JdbcDriver =
 
   abstract class DriverJdbcType[@specialized T](implicit val classTag: ClassTag[T]) extends JdbcType[T] {
     def scalaType = ScalaBaseType[T]
-    def sqlTypeName(size: Option[RelationalProfile.ColumnOption.Length]): String = driver.defaultSqlTypeName(this, size)
+    def sqlTypeName(sym: Option[FieldSymbol]): String = driver.defaultSqlTypeName(this, sym)
     def valueToSQLLiteral(value: T) =
       if(hasLiteralForm) value.toString
       else throw new SlickException(sqlTypeName(None) + " does not have a literal representation")
@@ -152,7 +153,7 @@ trait JdbcTypesComponent extends RelationalTypesComponent { driver: JdbcDriver =
 
     class CharJdbcType extends DriverJdbcType[Char] {
       def sqlType = java.sql.Types.CHAR
-      override def sqlTypeName(size: Option[RelationalProfile.ColumnOption.Length]) = "CHAR(1)"
+      override def sqlTypeName(sym: Option[FieldSymbol]) = "CHAR(1)"
       def setValue(v: Char, p: PreparedStatement, idx: Int) = stringJdbcType.setValue(String.valueOf(v), p, idx)
       def getValue(r: ResultSet, idx: Int) = {
         val s = stringJdbcType.getValue(r, idx)

slick/src/main/scala/slick/driver/MySQLDriver.scala

@@ -10,6 +10,7 @@ import slick.ast._
 import slick.ast.Util._
 import slick.ast.TypeUtil._
 import slick.util.MacroSupport.macroSupportInterpolation
+import slick.util.ConfigExtensionMethods.configExtensionMethods
 import slick.profile.{RelationalProfile, SqlProfile, Capability}
 import slick.compiler.{Phase, ResolveZipJoins, CompilerState}
 import slick.model.Model
@@ -92,13 +93,22 @@ trait MySQLDriver extends JdbcDriver { driver =>
 
   override def quoteIdentifier(id: String) = '`' + id + '`'
 
-  override def defaultSqlTypeName(tmd: JdbcType[_], size: Option[RelationalProfile.ColumnOption.Length]): String = tmd.sqlType match {
+  override def defaultSqlTypeName(tmd: JdbcType[_], sym: Option[FieldSymbol]): String = tmd.sqlType match {
     case java.sql.Types.VARCHAR =>
-      size.fold(defaultStringType)(l => if(l.varying) s"VARCHAR(${l.length})" else s"CHAR(${l.length})")
-    case _ => super.defaultSqlTypeName(tmd, size)
+      sym.flatMap(_.findColumnOption[RelationalProfile.ColumnOption.Length]) match {
+        case Some(l) => if(l.varying) s"VARCHAR(${l.length})" else s"CHAR(${l.length})"
+        case None => defaultStringType match {
+          case Some(s) => s
+          case None =>
+            if(sym.flatMap(_.findColumnOption[RelationalProfile.ColumnOption.Default[_]]).isDefined ||
+               sym.flatMap(_.findColumnOption[ColumnOption.PrimaryKey.type]).isDefined)
+              "VARCHAR(254)" else "TEXT"
+        }
+      }
+    case _ => super.defaultSqlTypeName(tmd, sym)
   }
 
-  protected lazy val defaultStringType = driverConfig.getString("defaultStringType")
+  protected lazy val defaultStringType = driverConfig.getStringOpt("defaultStringType")
 
   class MySQLResolveZipJoins extends ResolveZipJoins {
     // MySQL does not support ROW_NUMBER() but you can manually increment a variable in the SELECT
@@ -258,7 +268,7 @@ trait MySQLDriver extends JdbcDriver { driver =>
 
     override val uuidJdbcType = new UUIDJdbcType {
       override def sqlType = java.sql.Types.BINARY
-      override def sqlTypeName(size: Option[RelationalProfile.ColumnOption.Length]) = "BINARY(16)"
+      override def sqlTypeName(sym: Option[FieldSymbol]) = "BINARY(16)"
 
       override def valueToSQLLiteral(value: UUID): String =
         "x'"+value.toString.replace("-", "")+"'"

slick/src/main/scala/slick/driver/PostgresDriver.scala

@@ -115,14 +115,15 @@ trait PostgresDriver extends JdbcDriver { driver =>
   override protected lazy val useTransactionForUpsert = true
   override protected lazy val useServerSideUpsertReturning = false
 
-  override def defaultSqlTypeName(tmd: JdbcType[_], size: Option[RelationalProfile.ColumnOption.Length]): String = tmd.sqlType match {
+  override def defaultSqlTypeName(tmd: JdbcType[_], sym: Option[FieldSymbol]): String = tmd.sqlType match {
     case java.sql.Types.VARCHAR =>
+      val size = sym.flatMap(_.findColumnOption[RelationalProfile.ColumnOption.Length])
       size.fold("VARCHAR")(l => if(l.varying) s"VARCHAR(${l.length})" else s"CHAR(${l.length})")
     case java.sql.Types.BLOB => "lo"
     case java.sql.Types.DOUBLE => "DOUBLE PRECISION"
     /* PostgreSQL does not have a TINYINT type, so we use SMALLINT instead. */
     case java.sql.Types.TINYINT => "SMALLINT"
-    case _ => super.defaultSqlTypeName(tmd, size)
+    case _ => super.defaultSqlTypeName(tmd, sym)
   }
 
   class QueryBuilder(tree: Node, state: CompilerState) extends super.QueryBuilder(tree, state) {
@@ -205,11 +206,11 @@ trait PostgresDriver extends JdbcDriver { driver =>
 
     class ByteArrayJdbcType extends super.ByteArrayJdbcType {
       override val sqlType = java.sql.Types.BINARY
-      override def sqlTypeName(size: Option[RelationalProfile.ColumnOption.Length]) = "BYTEA"
+      override def sqlTypeName(sym: Option[FieldSymbol]) = "BYTEA"
     }
 
     class UUIDJdbcType extends super.UUIDJdbcType {
-      override def sqlTypeName(size: Option[RelationalProfile.ColumnOption.Length]) = "UUID"
+      override def sqlTypeName(sym: Option[FieldSymbol]) = "UUID"
       override def setValue(v: UUID, p: PreparedStatement, idx: Int) = p.setObject(idx, v, sqlType)
       override def getValue(r: ResultSet, idx: Int) = r.getObject(idx).asInstanceOf[UUID]
       override def updateValue(v: UUID, r: ResultSet, idx: Int) = r.updateObject(idx, v)

slick/src/main/scala/slick/driver/SQLiteDriver.scala

@@ -212,9 +212,9 @@ trait SQLiteDriver extends JdbcDriver { driver =>
     override protected def useTransactionForUpsert = !useServerSideUpsert
   }
 
-  override def defaultSqlTypeName(tmd: JdbcType[_], size: Option[RelationalProfile.ColumnOption.Length]): String = tmd.sqlType match {
+  override def defaultSqlTypeName(tmd: JdbcType[_], sym: Option[FieldSymbol]): String = tmd.sqlType match {
     case java.sql.Types.TINYINT | java.sql.Types.SMALLINT | java.sql.Types.BIGINT => "INTEGER"
-    case _ => super.defaultSqlTypeName(tmd, size)
+    case _ => super.defaultSqlTypeName(tmd, sym)
   }
 
   class JdbcTypes extends super.JdbcTypes {
@@ -227,7 +227,7 @@ trait SQLiteDriver extends JdbcDriver { driver =>
     /* SQLite does not have a proper BOOLEAN type. The suggested workaround is
      * INTEGER with constants 1 and 0 for TRUE and FALSE. */
     class BooleanJdbcType extends super.BooleanJdbcType {
-      override def sqlTypeName(size: Option[RelationalProfile.ColumnOption.Length]) = "INTEGER"
+      override def sqlTypeName(sym: Option[FieldSymbol]) = "INTEGER"
       override def valueToSQLLiteral(value: Boolean) = if(value) "1" else "0"
     }
     /* The SQLite JDBC driver does not support the JDBC escape syntax for

slick/src/main/scala/slick/jdbc/JdbcType.scala

@@ -1,8 +1,7 @@
 package slick.jdbc
 
 import java.sql.{PreparedStatement, ResultSet}
-import slick.ast.BaseTypedType
-import slick.profile.RelationalProfile
+import slick.ast.{FieldSymbol, BaseTypedType}
 
 /** A JdbcType object represents a Scala type that can be
   * used as a column type in the database. Implicit JdbcTypes
@@ -12,7 +11,7 @@ trait JdbcType[@specialized(Byte, Short, Int, Long, Char, Float, Double, Boolean
     * of the type to NULL. */
   def sqlType: Int
   /** The default name for the SQL type that is used for column declarations. */
-  def sqlTypeName(size: Option[RelationalProfile.ColumnOption.Length]): String
+  def sqlTypeName(size: Option[FieldSymbol]): String
   /** Set a parameter of the type. */
   def setValue(v: T, p: PreparedStatement, idx: Int): Unit
   /** Set a parameter of the type to NULL. */

slick/src/sphinx/upgrade.rst

@@ -65,3 +65,11 @@ to a design problem) was not to include non-matching rows in the total (equivale
 discriminator column only). This does not make sense anymore for the new outer join operators (introduced
 in 3.0) with correct `Option` types. The new semantics are identical to those of Scala collections.
 Semantics for counts of single columns remain unchanged.
+
+Default String type on MySQL
+----------------------------
+
+Slick 3.0 changed the default string type for MySQL to `TEXT`, which is not allowed for primary keys
+and columns with default values. In these cases we now fall back to the old `VARCHAR(254)` type which
+was used up to Slick 2.1. Like in 3.0 you can change this default by setting the application.conf key
+`slick.driver.MySQL.defaultStringType`.