Fixed MySQLProfile drop SQL commands to use qualified schema+table names

[NicolasRouquette]

Fixes #1897

[hvesalai]

@NicolasRouquette Could you also add a test case that failes without the change and then works with it.

[djx314]

@NicolasRouquette What will happen if I jsut define the tablename and I didn‘t defined the schema name? I have to avoid to define schema in my codegen script in my code. And what will it happen?

[NicolasRouquette]

@djx314 The fix ensures that SQL statements produced for dropping foreign keys use the same qualified name mechanism as that used for producing other kinds of SQL statements (e.g., create/drop table). So, if you use unqualified table names without a schema name, then all SQL statements should be correspondingly unqualified table names as before. The fix does not change this behavior.

[NicolasRouquette]

@hvesalai I'm converting my example (https://github.com/NicolasRouquette/MySQLSlickTest) into a new unit test: com.typesafe.slick.testkit.tests.MySQLQualifiedExtraTests (in response to #1898 ) and com.typesafe.slick.testkit.tests.MySQLUnqualifiedExtraTests (to confirm that fixing #1898 will not affect @djx314 ).

[NicolasRouquette]

@hvesalai The test could be much simpler than I though.

I am refactoring ForeignKeyTest as an abstract class with two variants:

abstract class ForeignKeyTest(schema: Option[String]) extends AsyncTest[RelationalTestDB] {
  import tdb.profile.api._

  def testSimple = {
    class Categories(tag: Tag) extends Table[(Int, String)](tag, schema,"categories") {
    ...
    }
 ...
}

class QualifiedForeignKeyTest extends ForeignKeyTest(Some("slick_test"))

class UnqualifiedForeignKeyTest extends ForeignKeyTest(None)

Hopefully, this will work to demonstrate the bug.

[hvesalai]

Looks, good, but now you pushed the revert of the fix also... can you rebase and rework (see documentation of git rebase -i) the four commits so there are only two:

• commit 1: tests that demonstrate the bug
• commit 2: the fix

You can then force push the result to replace the commits now in this PR with these two.

[NicolasRouquette]

@hvesalai I'll rewrite the history after I manage to get the qualified tests to fail as intended.

I have problems creating SQL statements for create/drop schema.

  def createSchemaIfQualified(): DBIO[Unit] = schema.fold[DBIO[Unit]] {
    SuccessAction(())
  } { s =>
    // It should be possible to write instead: sql"create schema `${s}`;"
    // Unfortunately, this results in: "create schema `?`;"
    val action = SQLActionBuilder(s"create schema `${s}`;", SetParameter.SetUnit)
    DBIO.seq(action.as[Unit])
  }

  def dropSchemaIfQualified(): DBIO[Unit] = schema.fold[DBIO[Unit]] {
    SuccessAction(())
  } { s =>
    // It should be possible to write instead: sql"drop schema `${s}`;"
    // Unfortunately, this results in: "drop schema `?`;"
    val action = SQLActionBuilder(s"drop schema `${s}`;", SetParameter.SetUnit)
    DBIO.seq(action.as[Unit])
  }

How should it be written so that it produces SQL statements like this: create schema `mysql_test`; ?

[hvesalai]

Why can't use use sql interpolation or StaticQuery.updateNA

[NicolasRouquette]

All the QualifiedForeignKey tests fail. Looking at the test failures, there is no common syntax for creating a database (aka schema) that will work across all implementations. It seems that slick would have to add support for this and specify in an implementation specific profile the corresponding syntax for the SQL statement(s) to produce.

[hvesalai]

@NicolasRouquette there are some test cases failing now. Do you have time to check it out and try to fix the code.

[hvesalai]

@NicolasRouquette I'm going throught the PRs that I will include in the next 3.3. release. Do you think this could make it, i.e. can it be fixed?

[NicolasRouquette]

@hvesalai Sorry for the delay in responding.

The fix is really simple; unfortunately, making a test that demonstrates the problem is more difficult because this seems to be the first time that there is a test involving "create/drop schema/table"
and these SQL statements clearly vary across implementations.

I'm not sure how to proceed.

[hvesalai]

@NicolasRouquette do you think this can be closed in favor of #1928 by @smootoo

[hvesalai]

This might now be obsolete.

[smootoo]

Agreed. I think this can be closed now, so I'm going to close it. Please re-open, if required.