fix(plugin-mysql): read a nullable column's DEFAULT NULL back as NULL on MySQL and MariaDB

CHANGELOG.md

@@ -82,6 +82,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Unexpanded schemas hidden by the sidebar filter in the Tree layout.
 - Empty object sections opened as "No items" under every match while filtering the sidebar tree.
 - **Drop View** offered in Recent for a sequence or materialized view opened from Open Quickly.
+- Column default of NULL shown as Empty on MySQL and MariaDB, and a NULL default that never stuck. (#3058)
+- String column defaults misread on MariaDB 10.2.7 and later, and expression defaults on MariaDB 10.2.1 to 10.2.6.
+- `ERROR 1064` editing a MySQL 8 column whose expression default holds a quoted string.
+- `ERROR 1067` saving a column made NOT NULL while its default was NULL.
+- NULL default on a MySQL `TEXT`, `BLOB`, `JSON` or `GEOMETRY` column saved as the expression `(NULL)`.
 - Unresponsive app and a dropped keystroke when typing in the row inspector's JSON field. (#3051)
 - Raw Oracle driver error in the schema switch failure dialog. (#3053)
 - Oracle health check closing a connection a statement was still running on. (#3053)

Plugins/MySQLDriverPlugin/MySQLCatalogDefault.swift

@@ -0,0 +1,171 @@
+//
+//  MySQLCatalogDefault.swift
+//  MySQLDriverPlugin
+//
+//  A column default as the catalog reports it, turned into the SQL that recreates it.
+//
+
+import Foundation
+import TableProPluginKit
+
+/// A catalog default, tagged with the form the read that produced it uses.
+///
+/// Nothing in the value alone says which form it is in, and the two forms disagree on what SQL NULL
+/// and a bare word mean, so the form travels with the value rather than as a flag beside it.
+internal enum MySQLCatalogDefault: Equatable, Sendable {
+    /// `SHOW FULL COLUMNS` on every server, and `INFORMATION_SCHEMA.COLUMNS` on MySQL and on MariaDB
+    /// before 10.2.7. A literal arrives unquoted, MySQL marks an expression `DEFAULT_GENERATED` in
+    /// `EXTRA`, and SQL NULL stands for both `DEFAULT NULL` and no default at all.
+    case bare(String?)
+
+    /// `INFORMATION_SCHEMA.COLUMNS` on MariaDB from 10.2.7. A literal arrives quoted, an expression
+    /// bare, `DEFAULT NULL` as the unquoted text `NULL`, and SQL NULL only for no default. MariaDB's
+    /// `SHOW FULL COLUMNS` does not follow it: measured on 12.3, it reports `'abc'` as `abc` and
+    /// `uuid()` as `uuid()` with an empty `EXTRA`.
+    case quoted(String?)
+
+    var value: String? {
+        switch self {
+        case .bare(let value), .quoted(let value): value
+        }
+    }
+}
+
+/// The column defaults `SHOW CREATE TABLE` states, and which columns a column read takes them for.
+internal struct MySQLCreateTableDefaults: Equatable, Sendable {
+    enum Scope: Equatable, Sendable {
+        /// A MariaDB whose catalog did not answer in its quoted form: the statement is the only exact
+        /// source, for every column. A column with no `DEFAULT` clause there has none.
+        case everyColumn
+        /// MySQL: only the expression defaults its catalog cannot report exactly. Every other default
+        /// reads better from the catalog, where a number stays `5` rather than the `'5'` this prints.
+        case expressionDefaults
+    }
+
+    let clauses: [String: String]
+    let scope: Scope
+
+    func catalogDefault(forColumn name: String, extra: String?) -> MySQLCatalogDefault? {
+        switch scope {
+        case .everyColumn:
+            return .quoted(clauses[name])
+        case .expressionDefaults:
+            guard mysqlIsExpressionDefault(extra: extra), let clause = clauses[name] else { return nil }
+            return .quoted(clause)
+        }
+    }
+}
+
+/// The SQL that follows `DEFAULT` for a column, or nil when the column has no default.
+///
+/// A nullable column the catalog gives SQL NULL has `DEFAULT NULL`. MySQL writes that clause itself
+/// for a nullable column declared without one, and MariaDB keeps it through `DROP DEFAULT`. The one
+/// state this cannot see is a MySQL nullable column whose default was removed with `ALTER COLUMN …
+/// DROP DEFAULT`: the catalog reports it the same way, and on `TEXT` so does `SHOW CREATE TABLE`, yet
+/// an `INSERT` that omits it fails with `ERROR 1364`. It reads as `NULL` here and everywhere this
+/// value goes, schema compare and exported DDL included. Nothing this app writes creates that state,
+/// because a `MODIFY` with no `DEFAULT` clause on a nullable column is `DEFAULT NULL` again.
+///
+/// A generated or AUTO_INCREMENT column has no default whatever the catalog says. MariaDB reports a
+/// generated column's as `NULL`, and the server refuses a `DEFAULT` on either.
+internal func mysqlColumnDefault(
+    _ catalog: MySQLCatalogDefault,
+    extra: String?,
+    dataType: String,
+    isNullable: Bool
+) -> String? {
+    guard !mysqlColumnIsGenerated(extra: extra), mysqlIdentityKind(extra: extra) == nil else { return nil }
+    guard let value = catalog.value else { return isNullable ? "NULL" : nil }
+    guard case .bare = catalog else { return value }
+    return mysqlBareCatalogDefault(value, extra: extra, dataType: dataType)
+}
+
+private func mysqlBareCatalogDefault(_ value: String, extra: String?, dataType: String) -> String {
+    // MySQL 8.0.13 marks a plain `DEFAULT CURRENT_TIMESTAMP` DEFAULT_GENERATED like any other
+    // expression, so this has to be answered before the marker is consulted or the one expression
+    // MySQL insists on bare comes back parenthesised.
+    if mysqlTemporalType(dataType), mysqlCurrentTimestampExpression(value, dataType: dataType) != nil {
+        return value
+    }
+    if mysqlIsExpressionDefault(extra: extra) {
+        let expression = mysqlUnescapedCatalogExpression(value)
+        return expression.hasPrefix("(") ? expression : "(\(expression))"
+    }
+    return mysqlCatalogReportsLiteralAsSQL(dataType: dataType)
+        ? value : "'\(mysqlEscapeStringLiteral(value))'"
+}
+
+/// MySQL's marker for an expression default, in `EXTRA` of both catalog reads.
+internal func mysqlIsExpressionDefault(extra: String?) -> Bool {
+    extra?.uppercased().contains("DEFAULT_GENERATED") == true
+}
+
+/// Whether a MySQL expression default can only be recreated from `SHOW CREATE TABLE`.
+///
+/// The catalog keeps an expression default escaped, which `mysqlUnescapedCatalogExpression` undoes
+/// exactly, and any non-ASCII text in it encoded twice, which nothing can undo: measured on 8.4.11,
+/// `concat('日','x')` comes back with `日` as `æ\u{97}¥`. `SHOW CREATE TABLE` prints it exactly, so an
+/// expression holding non-ASCII text is read from there and every other one from the catalog.
+internal func mysqlExpressionDefaultNeedsCreateTable(_ value: String?, extra: String?, dataType: String) -> Bool {
+    guard mysqlIsExpressionDefault(extra: extra), let value else { return false }
+    return !value.unicodeScalars.allSatisfy(\.isASCII)
+}
+
+/// Whether a default a MariaDB reports bare may be an expression rather than the string it reads
+/// as. Before 10.2.7 its catalog quotes nothing, so `uuid()` and `'uuid()'` both come back `uuid()`.
+/// A default with no parenthesis in it cannot be an expression there, and `CURRENT_TIMESTAMP` on a
+/// temporal column reads right either way.
+internal func mariaDBBareDefaultMayBeExpression(_ value: String?, dataType: String) -> Bool {
+    guard let value, value.contains("(") else { return false }
+    return !(mysqlTemporalType(dataType) && mysqlCurrentTimestampExpression(value, dataType: dataType) != nil)
+}
+
+/// An expression default as `SHOW CREATE TABLE` prints it, from the catalog's escaped copy, for a
+/// read that has no `SHOW CREATE TABLE` to take it from.
+///
+/// MySQL stores an expression default with every quote and backslash escaped by a backslash.
+/// Measured on 8.4.11, `DEFAULT (concat('a','b'))` comes back as `concat(_utf8mb4\'a\',_utf8mb4\'b\')`,
+/// which no statement accepts, so every later edit of the column failed. Undoing it is a pairwise
+/// scan in which a backslash takes the next character literally. The escaped form never holds a
+/// bare quote, so meeting one means the text was never escaped, and it is returned as it came.
+///
+/// Non-ASCII text is returned as it came too. The catalog encodes it twice, so unescaping it would
+/// produce a statement the server accepts with different text in it, and a default that changes
+/// without a word is worse than an edit the server refuses.
+internal func mysqlUnescapedCatalogExpression(_ value: String) -> String {
+    guard value.unicodeScalars.allSatisfy(\.isASCII) else { return value }
+    var result = ""
+    var index = value.startIndex
+    while index < value.endIndex {
+        let character = value[index]
+        if character == "'" { return value }
+        let next = value.index(after: index)
+        guard character == "\\" else {
+            result.append(character)
+            index = next
+            continue
+        }
+        guard next < value.endIndex else { return value }
+        result.append(value[next])
+        index = value.index(after: next)
+    }
+    return result
+}
+
+/// Whether this column type's catalog default is already the SQL that recreates it.
+///
+/// A string default comes back stripped of its quotes and has to be given them again. A number, a
+/// `BIT` default (`b'1'`) and a binary default (`0x61`) all come back as the literal they are, and
+/// quoting one changes what it means: `0x61` quoted stores the four characters rather than the byte.
+internal func mysqlCatalogReportsLiteralAsSQL(dataType: String) -> Bool {
+    let base = dataType.uppercased().split(separator: "(", maxSplits: 1).first.map(String.init)?
+        .trimmingCharacters(in: .whitespaces) ?? dataType.uppercased()
+    switch base {
+    case "TINYINT", "SMALLINT", "MEDIUMINT", "INT", "INTEGER", "BIGINT",
+         "DECIMAL", "DEC", "NUMERIC", "FIXED", "FLOAT", "DOUBLE", "REAL", "YEAR",
+         "BIT", "BINARY", "VARBINARY", "BOOL", "BOOLEAN":
+        return true
+    default:
+        return false
+    }
+}

Plugins/MySQLDriverPlugin/MySQLColumnDefinitionSQL.swift

@@ -100,11 +100,14 @@ internal func mysqlWholeStringLiteral(_ value: String) -> String? {
 /// column's own fractional-second precision, because MySQL rejects the pair when they differ, and
 /// is the one expression MySQL accepts bare. Every other expression is parenthesised on MySQL,
 /// which is what its grammar requires from 8.0.13; MariaDB takes them either way and writes them
-/// bare itself. A type that cannot carry a bare default is parenthesised whatever the value is.
+/// bare itself. A type that cannot carry a bare default is parenthesised whatever the value is, with
+/// one exception: `NULL`. Every type takes it bare, and parenthesised it is no longer the literal:
+/// MySQL 8 stores `DEFAULT (NULL)` as an expression default and MySQL 5.7 refuses the syntax.
 ///
 /// Nothing else is rewritten: the value already holds the SQL, and re-quoting it is what turned
 /// `(UUID())` into the six-character string `uuid()`.
 internal func mysqlDefaultValueLiteral(_ value: String, dataType: String, isMariaDB: Bool) -> String {
+    if mysqlIsNullLiteral(value) { return "NULL" }
     if mysqlTemporalType(dataType), let expression = mysqlCurrentTimestampExpression(value, dataType: dataType) {
         return expression
     }
@@ -114,51 +117,8 @@ internal func mysqlDefaultValueLiteral(_ value: String, dataType: String, isMari
     return value.hasPrefix("(") ? value : "(\(value))"
 }
 
-/// A column default as the catalog reports it, turned into the SQL that recreates it.
-///
-/// The two servers report it differently and neither says which it is in the value alone. MySQL
-/// leaves a literal bare and marks an expression `DEFAULT_GENERATED` in `EXTRA`. MariaDB from 10.2.7
-/// quotes literals and leaves expressions bare, with `EXTRA` empty; before that it quotes nothing,
-/// so it reads like MySQL without the marker and every default is a literal.
-internal func mysqlDefaultValueFromCatalog(
-    _ value: String?,
-    extra: String?,
-    dataType: String,
-    quotesLiterals: Bool
-) -> String? {
-    guard let value else { return nil }
-    if quotesLiterals { return value }
-
-    // MySQL 8.0.13 marks a plain `DEFAULT CURRENT_TIMESTAMP` DEFAULT_GENERATED like any other
-    // expression, so this has to be answered before the marker is consulted or the one expression
-    // MySQL insists on bare comes back parenthesised.
-    if mysqlTemporalType(dataType), mysqlCurrentTimestampExpression(value, dataType: dataType) != nil {
-        return value
-    }
-
-    guard extra?.uppercased().contains("DEFAULT_GENERATED") != true else {
-        return value.hasPrefix("(") ? value : "(\(value))"
-    }
-    return mysqlCatalogReportsLiteralAsSQL(dataType: dataType)
-        ? value : "'\(mysqlEscapeStringLiteral(value))'"
-}
-
-/// Whether this column type's catalog default is already the SQL that recreates it.
-///
-/// A string default comes back stripped of its quotes and has to be given them again. A number, a
-/// `BIT` default (`b'1'`) and a binary default (`0x61`) all come back as the literal they are, and
-/// quoting one changes what it means: `0x61` quoted stores the four characters rather than the byte.
-internal func mysqlCatalogReportsLiteralAsSQL(dataType: String) -> Bool {
-    let base = dataType.uppercased().split(separator: "(", maxSplits: 1).first.map(String.init)?
-        .trimmingCharacters(in: .whitespaces) ?? dataType.uppercased()
-    switch base {
-    case "TINYINT", "SMALLINT", "MEDIUMINT", "INT", "INTEGER", "BIGINT",
-         "DECIMAL", "DEC", "NUMERIC", "FIXED", "FLOAT", "DOUBLE", "REAL", "YEAR",
-         "BIT", "BINARY", "VARBINARY", "BOOL", "BOOLEAN":
-        return true
-    default:
-        return false
-    }
+internal func mysqlIsNullLiteral(_ value: String) -> Bool {
+    value.trimmingCharacters(in: .whitespaces).caseInsensitiveCompare("NULL") == .orderedSame
 }
 
 /// The only types on which a bare `CURRENT_TIMESTAMP` is the temporal expression rather than the

Plugins/MySQLDriverPlugin/MySQLCreateTableScanner.swift

@@ -6,6 +6,35 @@
 import Foundation
 
 internal enum MySQLCreateTableScanner {
+    /// Each column's `DEFAULT` operand as `SHOW CREATE TABLE` spells it, keyed by column name, or nil
+    /// when the statement does not create a table. A column with no `DEFAULT` clause is absent.
+    ///
+    /// Read one definition per line, because OceanBase prints a `SET` or `ENUM` member list with its
+    /// quotes unbalanced (`set('a','b'c')`), and a quote-aware split would run that column into the
+    /// next. A quoted column name is the one thing every server quotes correctly, and it may hold a
+    /// line break, so a line that opens a name without closing it runs on into the next: read on its
+    /// own, the rest of that name would reach another column as its default.
+    static func columnDefaultClauses(fromCreateTable sql: String) -> [String: String]? {
+        let lines = sql.split(separator: "\n", omittingEmptySubsequences: false)
+        guard let header = lines.first, declaresTable(header) else { return nil }
+        var clauses: [String: String] = [:]
+        var first = lines.index(after: lines.startIndex)
+        while first < lines.endIndex {
+            var last = first
+            while last + 1 < lines.endIndex,
+                  opensUnclosedName(sql[lines[first].startIndex..<lines[last].endIndex]) {
+                last += 1
+            }
+            var definition = sql[lines[first].startIndex..<lines[last].endIndex].drop(while: \.isWhitespace)
+            first = last + 1
+            guard let name = columnName(consumingFrom: &definition),
+                  let operand = defaultOperand(in: withoutTrailingSeparator(definition))
+            else { continue }
+            clauses[name] = operand
+        }
+        return clauses
+    }
+
     static func firstGroup(in text: Substring) -> Substring? {
         var depth = 0
         var start: Substring.Index?
@@ -163,4 +192,52 @@ internal enum MySQLCreateTableScanner {
         }
         return marks
     }
+
+    private static func declaresTable(_ header: Substring) -> Bool {
+        let words = header.prefix { $0 != "`" && $0 != "\"" && $0 != "(" }
+            .split(whereSeparator: \.isWhitespace)
+            .map { $0.uppercased() }
+        guard words.first == "CREATE", let tableIndex = words.firstIndex(of: "TABLE") else { return false }
+        return !words[..<tableIndex].contains("VIEW")
+    }
+
+    private static func columnName(consumingFrom definition: inout Substring) -> String? {
+        if let quote = definition.first, quote == "`" || quote == "\"" {
+            return consumeQuotedName(from: &definition, quote: quote)
+        }
+        guard let first = definition.first, first.isLetter || first.isNumber || first == "_" || first == "$" else {
+            return nil
+        }
+        let name = definition.prefix { !$0.isWhitespace }
+        definition = definition.dropFirst(name.count)
+        return String(name)
+    }
+
+    private static func opensUnclosedName(_ text: Substring) -> Bool {
+        var definition = text.drop(while: \.isWhitespace)
+        guard let quote = definition.first, quote == "`" || quote == "\"" else { return false }
+        return consumeQuotedName(from: &definition, quote: quote) == nil
+    }
+
+    private static func withoutTrailingSeparator(_ definition: Substring) -> Substring {
+        var trimmed = definition
+        while let last = trimmed.last, last.isWhitespace {
+            trimmed = trimmed.dropLast()
+        }
+        return trimmed.last == "," ? trimmed.dropLast() : trimmed
+    }
+
+    private static func defaultOperand(in definition: Substring) -> String? {
+        let tokens = topLevelTokens(of: definition)
+        for (index, token) in tokens.enumerated() {
+            let upper = token.uppercased()
+            if upper == "DEFAULT" {
+                return tokens.indices.contains(index + 1) ? String(tokens[index + 1]) : nil
+            }
+            if upper.hasPrefix("DEFAULT("), token.count > "DEFAULT".count {
+                return String(token.dropFirst("DEFAULT".count))
+            }
+        }
+        return nil
+    }
 }

Plugins/MySQLDriverPlugin/MySQLPluginDriver+OceanBaseDefaults.swift

@@ -37,8 +37,10 @@ internal extension MySQLPluginDriver {
         catalogDefault: String?,
         extra: String?,
         dataType: String,
+        isNullable: Bool,
         column: String,
-        createTableClauses: [String: String]?
+        createTableClauses: [String: String]?,
+        createTableDefaults: MySQLCreateTableDefaults?
     ) -> String? {
         if flavor.isOceanBase, let catalogDefault {
             if let currentTimestamp = OceanBaseColumnDefaults.currentTimestampDefault(catalogDefault, dataType: dataType) {
@@ -60,8 +62,12 @@ internal extension MySQLPluginDriver {
                 return binaryLiteral
             }
         }
-        return mysqlDefaultValueFromCatalog(
-            catalogDefault, extra: extra, dataType: dataType, quotesLiterals: catalogQuotesDefaults
+        return mysqlColumnDefault(
+            createTableDefaults?.catalogDefault(forColumn: column, extra: extra)
+                ?? (catalogQuotesDefaults ? .quoted(catalogDefault) : .bare(catalogDefault)),
+            extra: extra,
+            dataType: dataType,
+            isNullable: isNullable
         )
     }
 
@@ -79,6 +85,6 @@ internal extension MySQLPluginDriver {
     private func oceanbaseDefaultClauses(table: String, schema: String?) async throws -> [String: String] {
         let result = try await execute(query: "SHOW CREATE TABLE \(qualifiedName(table, schema: schema))")
         guard let createTable = result.rows.first?[safe: 1]?.asText else { return [:] }
-        return OceanBaseColumnDefaults.defaultClauses(fromCreateTable: createTable) ?? [:]
+        return MySQLCreateTableScanner.columnDefaultClauses(fromCreateTable: createTable) ?? [:]
     }
 }