Skip to content

Commit

Permalink
Remove isWriteType override for Postgre;
Browse files Browse the repository at this point in the history 
Omit RETURNING from isWriteType for all database types;
Modify tests for uniform handling for all database types
  • Loading branch information
@markconnellypro
markconnellypro committed Mar 4, 2024
1 parent b0dd905 commit feb7f81
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 106 deletions.
2 changes: 1 addition & 1 deletion system/Database/BaseConnection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1657,7 +1657,7 @@ public function resetDataCache()
*/
public function isWriteType($sql): bool
{
return (bool) preg_match('/^\s*(WITH\s.+(\s|[)]))?"?(SET|INSERT|UPDATE|DELETE|REPLACE|CREATE|DROP|TRUNCATE|LOAD|COPY|ALTER|RENAME|GRANT|REVOKE|LOCK|UNLOCK|REINDEX|MERGE)\s/is', $sql);
return (bool) preg_match('/^\s*(WITH\s.+(\s|[)]))?"?(SET|INSERT|UPDATE|DELETE|REPLACE|CREATE|DROP|TRUNCATE|LOAD|COPY|ALTER|RENAME|GRANT|REVOKE|LOCK|UNLOCK|REINDEX|MERGE)\s(?!.*\sRETURNING\s)/is', $sql);
}

/**
Expand Down
16 changes: 0 additions & 16 deletions system/Database/Postgre/Connection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,20 +567,4 @@ protected function _transRollback(): bool
{
return (bool) pg_query($this->connID, 'ROLLBACK');
}

/**
* Determines if a query is a "write" type.
*
* Overrides BaseConnection::isWriteType, adding additional read query types.
*
* @param string $sql
*/
public function isWriteType($sql): bool
{
if (preg_match('#^\s*(WITH\s.+(\s|[)]))?(INSERT|UPDATE|DELETE).*RETURNING\s.+(\,\s?.+)*$#is', $sql)) {
return false;
}

return parent::isWriteType($sql);
}
}
104 changes: 15 additions & 89 deletions tests/system/Database/Live/WriteTypeQueryTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,6 @@ final class WriteTypeQueryTest extends CIUnitTestCase
protected $refresh = true;
protected $seed = CITestSeeder::class;

/**
* Whether CodeIgniter considers RETURNING in isWriteType.
*
* Currently, only Postgre is supported by CodeIgniter.
* This method should be updated if support for RETURNING
* is expanded for other databases.
*
* @param string $dbDriver
*/
private function testReturning($dbDriver): bool
{
return $dbDriver === 'Postgre';
}

public function testSet(): void
{
$sql = 'SET FOREIGN_KEY_CHECKS=0';
Expand Down Expand Up @@ -110,11 +96,7 @@ public function testInsertOneReturning(): void
{
$sql = "INSERT INTO my_table (col1, col2) VALUES ('Joe', 'Cool') RETURNING id;";

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testInsertMultiReturning(): void
Expand All @@ -125,33 +107,21 @@ public function testInsertMultiReturning(): void
RETURNING id;
SQL;

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testInsertWithOneReturning(): void
{
$sql = "WITH seqvals AS (SELECT '3' AS seqval) INSERT INTO my_table (col1, col2) SELECT 'Joe', seqval FROM seqvals RETURNING id;";

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testInsertWithOneReturningNoSpace(): void
{
$sql = "WITH seqvals AS (SELECT '3' AS seqval)INSERT INTO my_table (col1, col2) SELECT 'Joe', seqval FROM seqvals RETURNING id;";

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testInsertWithMultiReturning(): void
Expand All @@ -164,11 +134,7 @@ public function testInsertWithMultiReturning(): void
RETURNING id;
SQL;

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testUpdateBuilder(): void
Expand Down Expand Up @@ -229,11 +195,7 @@ public function testUpdateOneReturning(): void
{
$sql = "UPDATE my_table SET col1 = 'foo' WHERE id = 2 RETURNING *;";

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testUpdateMultiReturning(): void
Expand All @@ -245,33 +207,21 @@ public function testUpdateMultiReturning(): void
RETURNING *;
SQL;

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testUpdateWithOneReturning(): void
{
$sql = "WITH seqvals AS (SELECT '3' AS seqval) UPDATE my_table SET col1 = seqval FROM seqvals WHERE id = 2 RETURNING *;";

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testUpdateWithOneReturningNoSpace(): void
{
$sql = "WITH seqvals AS (SELECT '3' AS seqval)UPDATE my_table SET col1 = seqval FROM seqvals WHERE id = 2 RETURNING *;";

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testUpdateWithMultiReturning(): void
Expand All @@ -285,11 +235,7 @@ public function testUpdateWithMultiReturning(): void
RETURNING *;
SQL;

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testDeleteBuilder(): void
Expand Down Expand Up @@ -348,11 +294,7 @@ public function testDeleteOneReturning(): void
{
$sql = 'DELETE FROM my_table WHERE id = 2 RETURNING *;';

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testDeleteMultiReturning(): void
Expand All @@ -363,33 +305,21 @@ public function testDeleteMultiReturning(): void
RETURNING *;
SQL;

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testDeleteWithOneReturning(): void
{
$sql = "WITH seqvals AS (SELECT '3' AS seqval) DELETE FROM my_table JOIN seqvals ON col1 = seqval RETURNING *;";

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testDeleteWithOneReturningNoSpace(): void
{
$sql = "WITH seqvals AS (SELECT '3' AS seqval)DELETE FROM my_table JOIN seqvals ON col1 = seqval RETURNING *;";

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testDeleteWithMultiReturning(): void
Expand All @@ -403,11 +333,7 @@ public function testDeleteWithMultiReturning(): void
RETURNING *;
SQL;

if ($this->testReturning($this->db->DBDriver)) {
$this->assertFalse($this->db->isWriteType($sql));
} else {
$this->assertTrue($this->db->isWriteType($sql));
}
$this->assertFalse($this->db->isWriteType($sql));
}

public function testReplace(): void
Expand Down

0 comments on commit feb7f81

Please sign in to comment.