From feb7f81ec0ff4de6da3e66cc8ad8aebb4efb5a9d Mon Sep 17 00:00:00 2001 From: "Mark A. Connelly" <16519297+markconnellypro@users.noreply.github.com> Date: Mon, 4 Mar 2024 00:20:00 -0500 Subject: [PATCH] Remove isWriteType override for Postgre; Omit RETURNING from isWriteType for all database types; Modify tests for uniform handling for all database types --- system/Database/BaseConnection.php | 2 +- system/Database/Postgre/Connection.php | 16 --- .../Database/Live/WriteTypeQueryTest.php | 104 +++--------------- 3 files changed, 16 insertions(+), 106 deletions(-) diff --git a/system/Database/BaseConnection.php b/system/Database/BaseConnection.php index b289c1a06a5d..ae5c4b3805f1 100644 --- a/system/Database/BaseConnection.php +++ b/system/Database/BaseConnection.php @@ -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); } /** diff --git a/system/Database/Postgre/Connection.php b/system/Database/Postgre/Connection.php index 3c2f28825388..d488c96ec1d4 100644 --- a/system/Database/Postgre/Connection.php +++ b/system/Database/Postgre/Connection.php @@ -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); - } } diff --git a/tests/system/Database/Live/WriteTypeQueryTest.php b/tests/system/Database/Live/WriteTypeQueryTest.php index df3ea16d02c6..fe59f19ffd2e 100644 --- a/tests/system/Database/Live/WriteTypeQueryTest.php +++ b/tests/system/Database/Live/WriteTypeQueryTest.php @@ -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'; @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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