Skip to content

Commit da25ed2

Browse files
michalsnmichalsn
authored
fix: Connection::getFieldData() default value convention for SQLSRV and OCI8 (codeigniter4#9680)
* fix: Connection::getFieldData() default value convention for SQLSRV and OCI8 * add normalizeDefault() method for OCI8 * apply code review suggestions * apply code review suggestions
1 parent 8a6426c commit da25ed2

File tree

5 files changed

+108
-8
lines changed

5 files changed

+108
-8
lines changed

‎system/Database/OCI8/Connection.php‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,12 +349,25 @@ protected function _fieldData(string $table): array
349349
$retval[$i]->max_length = $length;
350350

351351
$retval[$i]->nullable = $query[$i]->NULLABLE === 'Y';
352-
$retval[$i]->default = $query[$i]->DATA_DEFAULT;
352+
$retval[$i]->default = $this->normalizeDefault($query[$i]->DATA_DEFAULT);
353353
}
354354

355355
return $retval;
356356
}
357357

358+
/**
359+
* Removes trailing whitespace from default values
360+
* returned in database column metadata queries.
361+
*/
362+
private function normalizeDefault(?string $default): ?string
363+
{
364+
if ($default === null) {
365+
return $default;
366+
}
367+
368+
return rtrim($default);
369+
}
370+
358371
/**
359372
* Returns an array of objects with index data
360373
*

‎system/Database/SQLSRV/Connection.php‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,12 +384,42 @@ protected function _fieldData(string $table): array
384384
);
385385

386386
$retVal[$i]->nullable = $query[$i]->IS_NULLABLE !== 'NO';
387-
$retVal[$i]->default = $query[$i]->COLUMN_DEFAULT;
387+
$retVal[$i]->default = $this->normalizeDefault($query[$i]->COLUMN_DEFAULT);
388388
}
389389

390390
return $retVal;
391391
}
392392

393+
/**
394+
* Normalizes SQL Server COLUMN_DEFAULT values.
395+
* Removes wrapping parentheses and handles basic conversions.
396+
*/
397+
private function normalizeDefault(?string $default): ?string
398+
{
399+
if ($default === null) {
400+
return null;
401+
}
402+
403+
$default = trim($default);
404+
405+
// Remove outer parentheses (handles both single and double wrapping)
406+
while (preg_match('/^\((.*)\)$/', $default, $matches)) {
407+
$default = trim($matches[1]);
408+
}
409+
410+
// Handle NULL literal
411+
if (strcasecmp($default, 'NULL') === 0) {
412+
return null;
413+
}
414+
415+
// Handle string literals - remove quotes and unescape
416+
if (preg_match("/^'(.*)'$/s", $default, $matches)) {
417+
return str_replace("''", "'", $matches[1]);
418+
}
419+
420+
return $default;
421+
}
422+
393423
/**
394424
* Begin Transaction
395425
*/

‎tests/system/Database/Live/ForgeTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,7 +1093,7 @@ public function testAddFields(): void
10931093
'type' => 'int',
10941094
'max_length' => 10,
10951095
'nullable' => false,
1096-
'default' => '((0))', // Why?
1096+
'default' => '0',
10971097
],
10981098
];
10991099
} elseif ($this->db->DBDriver === 'OCI8') {
@@ -1124,7 +1124,7 @@ public function testAddFields(): void
11241124
'type' => 'NUMBER',
11251125
'max_length' => '11',
11261126
'nullable' => false,
1127-
'default' => '0 ', // Why?
1127+
'default' => '0',
11281128
],
11291129
];
11301130

‎tests/system/Database/Live/SQLSRV/GetFieldDataTestCase.php‎

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414
namespace CodeIgniter\Database\Live\SQLSRV;
1515

1616
use CodeIgniter\Database\Live\AbstractGetFieldDataTestCase;
17+
use CodeIgniter\Database\SQLSRV\Connection;
1718
use Config\Database;
19+
use PHPUnit\Framework\Attributes\DataProvider;
1820
use PHPUnit\Framework\Attributes\Group;
1921

2022
/**
@@ -68,31 +70,31 @@ public function testGetFieldDataDefault(): void
6870
'type' => 'int',
6971
'max_length' => 10,
7072
'nullable' => false,
71-
'default' => '((0))', // int 0
73+
'default' => '0',
7274
// 'primary_key' => 0,
7375
],
7476
(object) [
7577
'name' => 'text_default_null',
7678
'type' => 'varchar',
7779
'max_length' => 64,
7880
'nullable' => true,
79-
'default' => '(NULL)', // NULL value
81+
'default' => null,
8082
// 'primary_key' => 0,
8183
],
8284
(object) [
8385
'name' => 'text_default_text_null',
8486
'type' => 'varchar',
8587
'max_length' => 64,
8688
'nullable' => false,
87-
'default' => "('null')", // string "null"
89+
'default' => 'null', // string "null"
8890
// 'primary_key' => 0,
8991
],
9092
(object) [
9193
'name' => 'text_default_abc',
9294
'type' => 'varchar',
9395
'max_length' => 64,
9496
'nullable' => false,
95-
'default' => "('abc')", // string "abc"
97+
'default' => 'abc',
9698
// 'primary_key' => 0,
9799
],
98100
];
@@ -235,4 +237,58 @@ public function testGetFieldDataType(): void
235237
];
236238
$this->assertSameFieldData($expected, $fields);
237239
}
240+
241+
#[DataProvider('provideNormalizeDefault')]
242+
public function testNormalizeDefault(?string $input, ?string $expected): void
243+
{
244+
$this->assertInstanceOf(Connection::class, $this->db);
245+
246+
$normalizeDefault = self::getPrivateMethodInvoker($this->db, 'normalizeDefault');
247+
$this->assertSame($expected, $normalizeDefault($input));
248+
}
249+
250+
/**
251+
* @return iterable<string, array{string|null, string|null}>
252+
*/
253+
public static function provideNormalizeDefault(): iterable
254+
{
255+
return [
256+
// Null cases
257+
'null input' => [null, null],
258+
'NULL literal wrapped in parentheses' => ['(NULL)', null],
259+
'null literal lowercase' => ['(null)', null],
260+
'null literal mixed case' => ['(Null)', null],
261+
'null literal random case' => ['(nULL)', null],
262+
'null string' => ["('null')", 'null'],
263+
264+
// String literal cases
265+
'simple string' => ["('hello')", 'hello'],
266+
'empty string' => ['(())', ''],
267+
'string with space' => ["('hello world')", 'hello world'],
268+
'empty string literal' => ["('')", ''],
269+
'string with escaped quote' => ["('can''t')", "can't"],
270+
'string with multiple escaped quotes' => ["('it''s a ''test''')", "it's a 'test'"],
271+
'concatenated multiline expression' => ["('line1'+char(10)+'line2')", "line1'+char(10)+'line2"],
272+
273+
// Numeric cases
274+
'zero with double parentheses' => ['((0))', '0'],
275+
'positive integer with double parentheses' => ['((123))', '123'],
276+
'negative integer with double parentheses' => ['((-456))', '-456'],
277+
'float with double parentheses' => ['((3.14))', '3.14'],
278+
279+
// Function/expression cases
280+
'function call' => ['(getdate())', 'getdate()'],
281+
'newid function' => ['(newid())', 'newid()'],
282+
'user_name function' => ['(user_name())', 'user_name()'],
283+
'current_timestamp' => ['(current_timestamp)', 'current_timestamp'],
284+
'mathematical expression' => ['((1+1))', '1+1'],
285+
'multiplication expression' => ['((100*2))', '100*2'],
286+
287+
// Edge cases
288+
'multiple nested parentheses' => ["((('nested')))", 'nested'],
289+
'value without parentheses' => ['plain_value', 'plain_value'],
290+
'value with parentheses' => ['( plain_value )', 'plain_value'],
291+
'function with parameters' => ['(complex_func(1, 2))', 'complex_func(1, 2)'],
292+
];
293+
}
238294
}

‎user_guide_src/source/changelogs/v4.6.4.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ Bugs Fixed
3131
**********
3232

3333
- **Database:** Fixed a bug in ``Database::connect()`` which was causing to store non-shared connection instances in shared cache.
34+
- **Database:** Fixed a bug in ``Connection::getFieldData()`` for ``SQLSRV`` and ``OCI8`` where extra characters were returned in column default values (specific to those handlers), instead of following the convention used by other drivers.
3435

3536
See the repo's
3637
`CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_

0 commit comments

Comments
 (0)