From dd171566a45b8a5e7d05be12383c2175dcc60241 Mon Sep 17 00:00:00 2001 From: Kenneth Hough Date: Fri, 24 Apr 2026 21:01:03 -0500 Subject: [PATCH] Fix DBI::buildFieldDefinition to emit DEFAULT NULL for null defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before: a field declared with `'default' => null` generated invalid SQL because the is_string() check fell through and concatenated null (which PHP coerces to empty string). Result: `\`language\` varchar(5) DEFAULT NOT NULL,` with a dangling space, which MariaDB rejects with a syntax error. After: emit `DEFAULT NULL` explicitly, which is valid SQL and matches what `'default' => null` obviously intended. The Application model's `language` field carries `'default' => null`, so any fresh call to `DBI::createTable(Application)` has been blowing up since that field was introduced. The bug hasn't surfaced in production because existing customer installs created the Application table before the field was added — only fresh installs hit it. gust/lib/Database.php carries an identical copy of this bug. Patching gust's copy is explicitly out of scope for the MCP/auth migration initiative (see design doc section 11 — gust evaluation) and stays on the "fix when gust is next touched" list. Added a characterization test in tests/DatabaseTest.php that creates a throwaway table with a `'default' => null` column and asserts both success and `DEFAULT NULL` in the resulting SHOW CREATE TABLE output. The test explicitly re-selects the kytedev database because the preceding createDatabase tests leave the mysqli connection without an active database context. 31/31 unit tests green. --- src/Core/DBI.php | 6 ++++- tests/DatabaseTest.php | 57 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/Core/DBI.php b/src/Core/DBI.php index be1ba0d0..48bb9171 100644 --- a/src/Core/DBI.php +++ b/src/Core/DBI.php @@ -568,7 +568,11 @@ private static function buildFieldDefinition($name, $attrs, $tableName) { // Default value if (array_key_exists('default', $attrs)) { $field .= ' DEFAULT '; - $field .= (is_string($attrs['default']) ? "'" . $attrs['default'] . "'" : $attrs['default']); + if (is_null($attrs['default'])) { + $field .= 'NULL'; + } else { + $field .= (is_string($attrs['default']) ? "'" . $attrs['default'] . "'" : $attrs['default']); + } } // Required/NOT NULL diff --git a/tests/DatabaseTest.php b/tests/DatabaseTest.php index 1c7ef5f4..3ee4289d 100644 --- a/tests/DatabaseTest.php +++ b/tests/DatabaseTest.php @@ -23,4 +23,61 @@ public function testDatabaseCreationAndSwitch() { $password = null; $this->assertTrue(\Kyte\Core\DBI::createDatabase("TestDatabase2", "TestUser2", $password, true)); } + + /** + * Regression test: createTable must accept fields declared with + * 'default' => null and emit DEFAULT NULL in the generated SQL. + * + * Before this was fixed, buildFieldDefinition's default-value branch fell + * through to a fall-through concat that coerced null to '', producing + * broken SQL like `language varchar(5) DEFAULT NOT NULL,`. The Application + * model's `language` field has 'default' => null, so any fresh createTable + * of it (e.g. during install) blew up. gust/lib/Database.php still has an + * identical copy of this bug and should be patched when gust is next + * touched (see design doc section 11, gust evaluation). + */ + public function testCreateTableAcceptsNullDefault() { + // The preceding createDatabase tests leave the mysqli connection + // without an active database context. Re-select kytedev explicitly. + // dbInit won't help here because connect() early-returns once a + // connection object already exists. + \Kyte\Core\DBI::query('USE `' . KYTE_DB_DATABASE . '`'); + + $tblName = 'NullDefaultProbe_' . substr(bin2hex(random_bytes(4)), 0, 8); + + $modelDef = [ + 'name' => $tblName, + 'struct' => [ + 'id' => [ + 'type' => 'i', + 'required' => true, + 'pk' => true, + 'size' => 11, + 'date' => false, + ], + 'optional_label' => [ + 'type' => 's', + 'required' => false, + 'size' => 64, + 'date' => false, + 'default' => null, + ], + ], + ]; + + try { + $this->assertTrue(\Kyte\Core\DBI::createTable($modelDef)); + + $rows = \Kyte\Core\DBI::query("SHOW CREATE TABLE `$tblName`"); + $this->assertNotEmpty($rows); + $createSql = $rows[0]['Create Table'] ?? ''; + $this->assertStringContainsStringIgnoringCase( + 'DEFAULT NULL', + $createSql, + "Column with 'default' => null should surface as DEFAULT NULL in SHOW CREATE TABLE output." + ); + } finally { + \Kyte\Core\DBI::query("DROP TABLE IF EXISTS `$tblName`"); + } + } }