Skip to content

Commit 6913cab

Browse files
Merge pull request #80 from keyqcloud/hotfix/dbi-null-default
Fix DBI::buildFieldDefinition for null defaults
2 parents 2c677c4 + dd17156 commit 6913cab

2 files changed

Lines changed: 62 additions & 1 deletion

File tree

src/Core/DBI.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,11 @@ private static function buildFieldDefinition($name, $attrs, $tableName) {
568568
// Default value
569569
if (array_key_exists('default', $attrs)) {
570570
$field .= ' DEFAULT ';
571-
$field .= (is_string($attrs['default']) ? "'" . $attrs['default'] . "'" : $attrs['default']);
571+
if (is_null($attrs['default'])) {
572+
$field .= 'NULL';
573+
} else {
574+
$field .= (is_string($attrs['default']) ? "'" . $attrs['default'] . "'" : $attrs['default']);
575+
}
572576
}
573577

574578
// Required/NOT NULL

tests/DatabaseTest.php

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,61 @@ public function testDatabaseCreationAndSwitch() {
2323
$password = null;
2424
$this->assertTrue(\Kyte\Core\DBI::createDatabase("TestDatabase2", "TestUser2", $password, true));
2525
}
26+
27+
/**
28+
* Regression test: createTable must accept fields declared with
29+
* 'default' => null and emit DEFAULT NULL in the generated SQL.
30+
*
31+
* Before this was fixed, buildFieldDefinition's default-value branch fell
32+
* through to a fall-through concat that coerced null to '', producing
33+
* broken SQL like `language varchar(5) DEFAULT NOT NULL,`. The Application
34+
* model's `language` field has 'default' => null, so any fresh createTable
35+
* of it (e.g. during install) blew up. gust/lib/Database.php still has an
36+
* identical copy of this bug and should be patched when gust is next
37+
* touched (see design doc section 11, gust evaluation).
38+
*/
39+
public function testCreateTableAcceptsNullDefault() {
40+
// The preceding createDatabase tests leave the mysqli connection
41+
// without an active database context. Re-select kytedev explicitly.
42+
// dbInit won't help here because connect() early-returns once a
43+
// connection object already exists.
44+
\Kyte\Core\DBI::query('USE `' . KYTE_DB_DATABASE . '`');
45+
46+
$tblName = 'NullDefaultProbe_' . substr(bin2hex(random_bytes(4)), 0, 8);
47+
48+
$modelDef = [
49+
'name' => $tblName,
50+
'struct' => [
51+
'id' => [
52+
'type' => 'i',
53+
'required' => true,
54+
'pk' => true,
55+
'size' => 11,
56+
'date' => false,
57+
],
58+
'optional_label' => [
59+
'type' => 's',
60+
'required' => false,
61+
'size' => 64,
62+
'date' => false,
63+
'default' => null,
64+
],
65+
],
66+
];
67+
68+
try {
69+
$this->assertTrue(\Kyte\Core\DBI::createTable($modelDef));
70+
71+
$rows = \Kyte\Core\DBI::query("SHOW CREATE TABLE `$tblName`");
72+
$this->assertNotEmpty($rows);
73+
$createSql = $rows[0]['Create Table'] ?? '';
74+
$this->assertStringContainsStringIgnoringCase(
75+
'DEFAULT NULL',
76+
$createSql,
77+
"Column with 'default' => null should surface as DEFAULT NULL in SHOW CREATE TABLE output."
78+
);
79+
} finally {
80+
\Kyte\Core\DBI::query("DROP TABLE IF EXISTS `$tblName`");
81+
}
82+
}
2683
}

0 commit comments

Comments
 (0)