Skip to content

Commit efe451a

Browse files
dereuromarkdereuromark
authored
Fix up timestamp quoting issue. (#923)
* Fix up timestamp quoting issue.
1 parent 3172645 commit efe451a

File tree

3 files changed

+145
-3
lines changed

3 files changed

+145
-3
lines changed

‎src/Db/Adapter/AbstractAdapter.php‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,10 +1040,22 @@ public function castToBool($value): mixed
10401040
*/
10411041
protected function getDefaultValueDefinition(mixed $default, ?string $columnType = null): string
10421042
{
1043+
$datetimeTypes = [
1044+
static::PHINX_TYPE_DATETIME,
1045+
static::PHINX_TYPE_TIMESTAMP,
1046+
static::PHINX_TYPE_TIME,
1047+
static::PHINX_TYPE_DATE,
1048+
];
1049+
10431050
if ($default instanceof Literal) {
10441051
$default = (string)$default;
1045-
} elseif (is_string($default) && stripos($default, 'CURRENT_TIMESTAMP') !== 0) {
1046-
// Ensure a defaults of CURRENT_TIMESTAMP(3) is not quoted.
1052+
} elseif (is_string($default) && stripos($default, 'CURRENT_TIMESTAMP') === 0) {
1053+
// Only skip quoting CURRENT_TIMESTAMP for datetime-related column types.
1054+
// For other types (like string), it should be quoted as a literal string value.
1055+
if (!in_array($columnType, $datetimeTypes, true)) {
1056+
$default = $this->quoteString($default);
1057+
}
1058+
} elseif (is_string($default)) {
10471059
$default = $this->quoteString($default);
10481060
} elseif (is_bool($default)) {
10491061
$default = $this->castToBool($default);

‎src/Db/Adapter/MysqlAdapter.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,8 @@ static function ($value) {
624624
$extra = ' ' . implode(' ', $extras);
625625

626626
if (($row['Default'] !== null)) {
627-
$extra .= $this->getDefaultValueDefinition($row['Default']);
627+
$phinxTypeInfo = $this->getPhinxType($row['Type']);
628+
$extra .= $this->getDefaultValueDefinition($row['Default'], $phinxTypeInfo['name']);
628629
}
629630
$definition = $row['Type'] . ' ' . $null . $extra . $comment;
630631

‎tests/TestCase/Db/Adapter/AbstractAdapterTest.php‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
namespace Migrations\Test\Db\Adapter;
55

66
use Migrations\Db\Adapter\AbstractAdapter;
7+
use Migrations\Db\Literal;
78
use Migrations\Test\TestCase\Db\Adapter\DefaultAdapterTrait;
89
use PDOException;
910
use Phinx\Config\Config;
@@ -144,4 +145,132 @@ public function fetchAll(string $sql): array
144145

145146
$this->assertEquals([], $adapter->getVersionLog());
146147
}
148+
149+
/**
150+
* Test CURRENT_TIMESTAMP default value quoting behavior (bug #1891).
151+
*
152+
* This test verifies that CURRENT_TIMESTAMP is only treated as a SQL function
153+
* for datetime-related column types. For other column types like string,
154+
* 'CURRENT_TIMESTAMP' should be quoted as a literal string value.
155+
*
156+
* @see https://github.com/cakephp/phinx/issues/1891
157+
*/
158+
#[DataProvider('currentTimestampDefaultValueProvider')]
159+
public function testCurrentTimestampDefaultValueQuoting($default, $columnType, $expected)
160+
{
161+
$adapter = new class (['version_order' => Config::VERSION_ORDER_CREATION_TIME]) extends AbstractAdapter {
162+
use DefaultAdapterTrait;
163+
164+
public function quoteString(string $value): string
165+
{
166+
return "'" . $value . "'";
167+
}
168+
169+
// Expose the protected method for testing
170+
public function testGetDefaultValueDefinition(mixed $default, ?string $columnType = null): string
171+
{
172+
return $this->getDefaultValueDefinition($default, $columnType);
173+
}
174+
};
175+
176+
$actual = $adapter->testGetDefaultValueDefinition($default, $columnType);
177+
$this->assertEquals($expected, $actual);
178+
}
179+
180+
/**
181+
* Data provider for CURRENT_TIMESTAMP quoting test.
182+
*
183+
* @return array
184+
*/
185+
public static function currentTimestampDefaultValueProvider(): array
186+
{
187+
return [
188+
// CURRENT_TIMESTAMP on datetime types should NOT be quoted
189+
'CURRENT_TIMESTAMP on datetime' => [
190+
'CURRENT_TIMESTAMP',
191+
AbstractAdapter::PHINX_TYPE_DATETIME,
192+
' DEFAULT CURRENT_TIMESTAMP',
193+
],
194+
'CURRENT_TIMESTAMP on timestamp' => [
195+
'CURRENT_TIMESTAMP',
196+
AbstractAdapter::PHINX_TYPE_TIMESTAMP,
197+
' DEFAULT CURRENT_TIMESTAMP',
198+
],
199+
'CURRENT_TIMESTAMP on time' => [
200+
'CURRENT_TIMESTAMP',
201+
AbstractAdapter::PHINX_TYPE_TIME,
202+
' DEFAULT CURRENT_TIMESTAMP',
203+
],
204+
'CURRENT_TIMESTAMP on date' => [
205+
'CURRENT_TIMESTAMP',
206+
AbstractAdapter::PHINX_TYPE_DATE,
207+
' DEFAULT CURRENT_TIMESTAMP',
208+
],
209+
'CURRENT_TIMESTAMP(3) on datetime' => [
210+
'CURRENT_TIMESTAMP(3)',
211+
AbstractAdapter::PHINX_TYPE_DATETIME,
212+
' DEFAULT CURRENT_TIMESTAMP(3)',
213+
],
214+
215+
// CURRENT_TIMESTAMP on non-datetime types SHOULD be quoted (bug #1891)
216+
'CURRENT_TIMESTAMP on string should be quoted' => [
217+
'CURRENT_TIMESTAMP',
218+
AbstractAdapter::PHINX_TYPE_STRING,
219+
" DEFAULT 'CURRENT_TIMESTAMP'",
220+
],
221+
'CURRENT_TIMESTAMP on text should be quoted' => [
222+
'CURRENT_TIMESTAMP',
223+
AbstractAdapter::PHINX_TYPE_TEXT,
224+
" DEFAULT 'CURRENT_TIMESTAMP'",
225+
],
226+
'CURRENT_TIMESTAMP on char should be quoted' => [
227+
'CURRENT_TIMESTAMP',
228+
AbstractAdapter::PHINX_TYPE_CHAR,
229+
" DEFAULT 'CURRENT_TIMESTAMP'",
230+
],
231+
'CURRENT_TIMESTAMP on integer should be quoted' => [
232+
'CURRENT_TIMESTAMP',
233+
AbstractAdapter::PHINX_TYPE_INTEGER,
234+
" DEFAULT 'CURRENT_TIMESTAMP'",
235+
],
236+
237+
// Regular string defaults should always be quoted
238+
'Regular string default' => [
239+
'default_value',
240+
AbstractAdapter::PHINX_TYPE_STRING,
241+
" DEFAULT 'default_value'",
242+
],
243+
'Regular string on datetime' => [
244+
'some_string',
245+
AbstractAdapter::PHINX_TYPE_DATETIME,
246+
" DEFAULT 'some_string'",
247+
],
248+
249+
// Literal values should not be quoted
250+
'Literal value' => [
251+
Literal::from('NOW()'),
252+
AbstractAdapter::PHINX_TYPE_DATETIME,
253+
' DEFAULT NOW()',
254+
],
255+
256+
// Boolean defaults
257+
'Boolean true' => [
258+
true,
259+
AbstractAdapter::PHINX_TYPE_BOOLEAN,
260+
' DEFAULT 1',
261+
],
262+
'Boolean false' => [
263+
false,
264+
AbstractAdapter::PHINX_TYPE_BOOLEAN,
265+
' DEFAULT 0',
266+
],
267+
268+
// Null columnType (should quote CURRENT_TIMESTAMP when type is unknown)
269+
'CURRENT_TIMESTAMP with null column type' => [
270+
'CURRENT_TIMESTAMP',
271+
null,
272+
" DEFAULT 'CURRENT_TIMESTAMP'",
273+
],
274+
];
275+
}
147276
}

0 commit comments

Comments
 (0)