Add support for MySQL's DROP INDEX syntax (#149)

* While SQLite has no DROP INDEX statement at all and PostgreSQL treats indexes as schema-global objects, MySQL's DROP INDEX statement (which is mapped to ALTER TABLE internally) requires specifying the table the index exists on. Rather than try to further complicate the logic for ALTER TABLE, add support for `DROP INDEX ... ON ...` to SQLDropIndex and SQLDropIndexBuilder. In the future, whether or not the owner gets serialized should be guarded by a SQLDialect switch (same as IF EXISTS).

* The behavior of `SQLDataType.type(_:)` is not correct in all dialects and can not be safely fixed. It was also only ever a utility method used by a single test and was never intended to be public API at all. Explicitly deprecate it to curtail incorrect usage and update the one call site to stop using it.

* Switch to reusable workflow for unit tests, heavily update integration test, add workflow to build code coverage on main

.github/workflows/main-codecov.yml

@@ -0,0 +1,9 @@
+name: Update code coverage baselines
+on:
+  push: { branches: [ main ] }
+jobs:
+  update-main-codecov:
+    uses: vapor/ci/.github/workflows/run-unit-tests.yml@reusable-workflows
+    with:
+      with_coverage: true
+      with_tsan: true

.github/workflows/test.yml

@@ -3,108 +3,80 @@ on:
   pull_request:
 
 jobs:
-  linux-all:
-    strategy:
-      fail-fast: false
-      matrix:
-        swiftver:
-          - swift:5.2
-          - swift:5.3
-          - swift:5.4
-          - swift:5.5
-          - swiftlang/swift:nightly-main
-        swiftos:
-          - focal
-    container: ${{ format('{0}-{1}', matrix.swiftver, matrix.swiftos) }}
-    runs-on: ubuntu-latest
-    env:
-      LOG_LEVEL: debug
-    steps:
-      - name: Check out package
-        uses: actions/checkout@v2
-      - name: Run tests with Thread Sanitizer
-        run: swift test --enable-test-discovery --sanitize=thread
+  unit-tests:
+    uses: vapor/ci/.github/workflows/run-unit-tests.yml@reusable-workflows
+    with:
+      with_coverage: true
+      with_tsan: true
 
-  macos-all:
-    strategy:
-      fail-fast: false
-      matrix:
-        xcode:
-          - latest-stable
-          - latest
-    runs-on: macos-11
-    env:
-      LOG_LEVEL: debug
-    steps:
-      - name: Select latest available Xcode
-        uses: maxim-lobanov/setup-xcode@v1
-        with:
-          xcode-version: ${{ matrix.xcode }}
-      - name: Checkout code
-        uses: actions/checkout@v2
-      - name: Run tests and Thread Sanitizer
-        run: |
-          swift test --enable-test-discovery ${{ matrix.filter }} --sanitize=thread \
-            -Xlinker -rpath \
-            -Xlinker $(xcode-select -p)/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift-5.5/macosx
-
-  driver-integration:
+  integration-tests:
     services:
       mysql-a:
         image: mysql:latest
-        env: { MYSQL_ALLOW_EMPTY_PASSWORD: "true", MYSQL_USER: vapor_username, MYSQL_PASSWORD: vapor_password, MYSQL_DATABASE: vapor_database }
+        env: { MYSQL_ALLOW_EMPTY_PASSWORD: "true", MYSQL_USER: test_username, MYSQL_PASSWORD: test_password, MYSQL_DATABASE: test_database }
       mysql-b:
         image: mysql:latest
-        env: { MYSQL_ALLOW_EMPTY_PASSWORD: "true", MYSQL_USER: vapor_username, MYSQL_PASSWORD: vapor_password, MYSQL_DATABASE: vapor_database }
+        env: { MYSQL_ALLOW_EMPTY_PASSWORD: "true", MYSQL_USER: test_username, MYSQL_PASSWORD: test_password, MYSQL_DATABASE: test_database }
       postgres-a:
         image: postgres:latest
-        env: { POSTGRES_USER: vapor_username, POSTGRES_PASSWORD: vapor_password, POSTGRES_DB: vapor_database }
+        env: { POSTGRES_USER: test_username, POSTGRES_PASSWORD: test_password, POSTGRES_DB: test_database }
       postgres-b:
         image: postgres:latest
-        env: { POSTGRES_USER: vapor_username, POSTGRES_PASSWORD: vapor_password, POSTGRES_DB: vapor_database }
+        env: { POSTGRES_USER: test_username, POSTGRES_PASSWORD: test_password, POSTGRES_DB: test_database }
     strategy:
       fail-fast: false
       matrix:
         swiftver:
-          - swift:5.2
+          - swift:5.4
           - swift:5.5
+          - swift:5.6
+          - swiftlang/swift:nightly-main
         swiftos:
           - focal
     runs-on: ubuntu-latest
     container: ${{ format('{0}-{1}', matrix.swiftver, matrix.swiftos) }}
     env:
       LOG_LEVEL: debug
+      POSTGRES_HOSTNAME: postgres-a
       POSTGRES_HOSTNAME_A: postgres-a
-      POSTGRES_USER_A: vapor_username
-      POSTGRES_PASSWORD_A: vapor_password
-      POSTGRES_DB_A: vapor_database
       POSTGRES_HOSTNAME_B: postgres-b
-      POSTGRES_USER_B: vapor_username
-      POSTGRES_PASSWORD_B: vapor_password
-      POSTGRES_DB_B: vapor_database
+      POSTGRES_DB: test_database
+      POSTGRES_DB_A: test_database
+      POSTGRES_DB_B: test_database
+      POSTGRES_USER: test_username
+      POSTGRES_USER_A: test_username
+      POSTGRES_USER_B: test_username
+      POSTGRES_PASSWORD: test_password
+      POSTGRES_PASSWORD_A: test_password
+      POSTGRES_PASSWORD_B: test_password
+      MYSQL_DATABASE: test_database
+      MYSQL_DATABASE_A: test_database
+      MYSQL_DATABASE_B: test_database
+      MYSQL_USER: tet_username
+      MYSQL_USERNAME: test_username
+      MYSQL_USERNAME_A: test_username
+      MYSQL_USERNAME_B: test_username
+      MYSQL_PASSWORD: test_password
+      MYSQL_PASSWORD_A: test_password
+      MYSQL_PASSWORD_B: test_password
+      MYSQL_HOSTNAME: mysql-a
       MYSQL_HOSTNAME_A: mysql-a
-      MYSQL_USERNAME_A: vapor_username
-      MYSQL_PASSWORD_A: vapor_password
-      MYSQL_DATABASE_A: vapor_database
       MYSQL_HOSTNAME_B: mysql-b
-      MYSQL_USERNAME_B: vapor_username
-      MYSQL_PASSWORD_B: vapor_password
-      MYSQL_DATABASE_B: vapor_database
     steps:
       - name: Install SQLite dependencies
         run: apt-get -q update && apt-get -q install -y libsqlite3-dev
 
       - name: Check out sql-kit
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
         with: { path: sql-kit }
       - name: Check out fluent-sqlite-driver
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
         with: { repository: 'vapor/fluent-sqlite-driver', path: fluent-sqlite-driver }
       - name: Check out fluent-postgres-driver
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
         with: { repository: 'vapor/fluent-postgres-driver', path: fluent-postgres-driver }
       - name: Check out fluent-mysql-driver
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
         with: { repository: 'vapor/fluent-mysql-driver', path: fluent-mysql-driver }
 
       - name: Use sql-kit in fluent-sqlite-driver
@@ -115,8 +87,8 @@ jobs:
         run: swift package --package-path fluent-mysql-driver edit sql-kit --path sql-kit
 
       - name: Run fluent-sqlite-driver tests with Thread Sanitizer
-        run: swift test --package-path fluent-sqlite-driver --enable-test-discovery --sanitize=thread
+        run: swift test --package-path fluent-sqlite-driver --sanitize=thread
       - name: Run fluent-postgres-driver tests with Thread Sanitizer
-        run: swift test --package-path fluent-postgres-driver --enable-test-discovery --sanitize=thread
+        run: swift test --package-path fluent-postgres-driver --sanitize=thread
       - name: Run fluent-mysql-driver tests with Thread Sanitizer
-        run: swift test --package-path fluent-mysql-driver --enable-test-discovery --sanitize=thread
+        run: swift test --package-path fluent-mysql-driver --sanitize=thread

Sources/SQLKit/Builders/SQLDropIndexBuilder.swift

@@ -28,6 +28,25 @@ public final class SQLDropIndexBuilder: SQLQueryBuilder {
         dropIndex.ifExists = true
         return self
     }
+
+    /// Convenience method for specifying an owning object using a `String`. See
+    /// ``on(_:)-84xo2`` for details.
+    @discardableResult
+    public func on(_ owningObject: String) -> Self {
+        return self.on(SQLIdentifier(owningObject))
+    }
+
+    /// The object (usually a table) which owns the index may be explicitly specified.
+    /// Some dialects treat indexes as database-level objects in their own right and
+    /// treat specifying an owner as an error, while others require the owning object
+    /// in order to perform the drop operation at all. At the time of this writing,
+    /// there is no support for specifying this in `SQLDialect`; callers must ensure
+    /// that they either specify or omit an owning object as appropriate.
+    @discardableResult
+    public func on(_ owningObject: SQLExpression) -> Self {
+        dropIndex.owningObject = owningObject
+        return self
+    }
 
     /// The drop behavior clause specifies if objects that depend on a index
     /// should also be dropped or not when the index is dropped, for databases

Sources/SQLKit/Query/SQLDataType.swift

@@ -7,6 +7,7 @@ public enum SQLDataType: SQLExpression {
     case real
     case blob
 
+    @available(*, deprecated, message: "This is a test utility method that was incorrectly made public. Use `.custom()` directly instead.")
     public static func type(_ string: String) -> Self {
         .custom(SQLIdentifier(string))
     }

Sources/SQLKit/Query/SQLDropIndex.swift

@@ -8,6 +8,10 @@ public struct SQLDropIndex: SQLExpression {
     /// The optional `IF EXISTS` clause suppresses the error that would normally
     /// result if the index does not exist.
     public var ifExists: Bool
+
+    /// The object (usually a table) on which the index exists. Not all databases support specifying
+    /// this, while others require it.
+    public var owningObject: SQLExpression?
 
     /// The optional drop behavior clause specifies if objects that depend on the
     /// index should also be dropped or not, for databases that support this
@@ -22,21 +26,18 @@ public struct SQLDropIndex: SQLExpression {
 
     /// See `SQLExpression`.
     public func serialize(to serializer: inout SQLSerializer) {
-        serializer.write("DROP INDEX ")
-        if self.ifExists {
-            if serializer.dialect.supportsIfExists {
-                serializer.write("IF EXISTS ")
-            } else {
-                serializer.database.logger.warning("\(serializer.dialect.name) does not support IF EXISTS")
+        serializer.statement {
+            $0.append("DROP INDEX")
+            if self.ifExists, $0.dialect.supportsIfExists {
+                $0.append("IF EXISTS")
             }
-        }
-        self.name.serialize(to: &serializer)
-        if serializer.dialect.supportsDropBehavior {
-            serializer.write(" ")
-            if let dropBehavior = behavior {
-                dropBehavior.serialize(to: &serializer)
-            } else {
-                SQLDropBehavior.restrict.serialize(to: &serializer)
+            $0.append(self.name)
+            if let owningObject = self.owningObject {
+                $0.append("ON")
+                $0.append(owningObject)
+            }
+            if $0.dialect.supportsDropBehavior {
+                $0.append(self.behavior ?? SQLDropBehavior.restrict)
             }
         }
     }

Sources/SQLKitBenchmark/SQLBenchmark+Enum.swift

@@ -14,7 +14,7 @@ extension SQLBenchmarker {
             let planetType: SQLDataType
             switch $0.dialect.enumSyntax {
             case .typeName:
-                planetType = .type("planet_type")
+                planetType = .custom(SQLIdentifier("planet_type"))
                 try $0.create(enum: "planet_type")
                     .value("smallRocky")
                     .value("gasGiant")

Tests/SQLKitTests/SQLKitTests.swift

@@ -206,6 +206,11 @@ final class SQLKitTests: XCTestCase {
         try db.drop(table: "normalized_planet_names").temporary().run().wait()
         XCTAssertEqual(db.results[0], "DROP TEMPORARY TABLE `normalized_planet_names`")
     }
+
+    func testOwnerObjectsForDropIndex() throws {
+        try db.drop(index: "some_crummy_mysql_index").on("some_darn_mysql_table").run().wait()
+        XCTAssertEqual(db.results[0], "DROP INDEX `some_crummy_mysql_index` ON `some_darn_mysql_table`")
+    }
 
     func testAltering() throws {
         // SINGLE