Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: isWriteType() to recognize CTE; always excluding RETURNING #8599

Merged
merged 8 commits into from
Mar 7, 2024
+275 −30
Merged

fix: isWriteType() to recognize CTE; always excluding RETURNING #8599

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e70b389
fix: isWriteType() to recognize CTE; [Postgre] allow beginning whites…
markconnellypro Mar 2, 2024
ad4d54d
Fix linting errors in tests
markconnellypro Mar 2, 2024
8b05cde
Refine isWriteType regex between WITH and other operation keyword
markconnellypro Mar 2, 2024
123d987
Separate each test into its own function;
markconnellypro Mar 4, 2024
8e1f62f
Remove RETURNING from testInsertMulti()
markconnellypro Mar 4, 2024
d2c6162
Fix rector errors in tests
markconnellypro Mar 4, 2024
b0dd905
Change testAssertTypeReturning to testReturning; remove dynamic $asse…
markconnellypro Mar 4, 2024
feb7f81
Remove isWriteType override for Postgre;
markconnellypro Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion system/Database/BaseConnection.php
View file
Open in desktop
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*"?(SET|INSERT|UPDATE|DELETE|REPLACE|CREATE|DROP|TRUNCATE|LOAD|COPY|ALTER|RENAME|GRANT|REVOKE|LOCK|UNLOCK|REINDEX|MERGE)\s/i', $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/is', $sql);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion system/Database/Postgre/Connection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ protected function _transRollback(): bool
*/
public function isWriteType($sql): bool
{
if (preg_match('#^(INSERT|UPDATE).*RETURNING\s.+(\,\s?.+)*$#is', $sql)) {
if (preg_match('#^\s*(WITH\s.+(\s|[)]))?(INSERT|UPDATE|DELETE).*RETURNING\s.+(\,\s?.+)*$#is', $sql)) {
return false;
}

Expand Down
141 changes: 135 additions & 6 deletions tests/system/Database/Live/WriteTypeQueryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,49 @@

$this->assertTrue($this->db->isWriteType($sql));

if ($this->db->DBDriver === 'Postgre') {
$sql = "INSERT INTO my_table (col1, col2) VALUES ('Joe', 'Cool') RETURNING id;";
$sql = "WITH seqvals AS (SELECT '3' AS seqval)INSERT INTO my_table (col1, col2) SELECT 'Joe', seqval FROM seqvals;";
kenjis marked this conversation as resolved.
Show resolved Hide resolved

$this->assertTrue($this->db->isWriteType($sql));

$sql = <<<'SQL'
WITH seqvals AS (SELECT '3' AS seqval)
INSERT INTO my_table (col1, col2)
SELECT 'Joe', seqval
FROM seqvals;
SQL;

$this->assertFalse($this->db->isWriteType($sql));
$this->assertTrue($this->db->isWriteType($sql));

$assertionType = 'assertTrue';
if ($this->db->DBDriver === 'Postgre') {
$assertionType = 'assertFalse';
}
kenjis marked this conversation as resolved.
Show resolved Hide resolved

$sql = "INSERT INTO my_table (col1, col2) VALUES ('Joe', 'Cool') RETURNING id;";

$this->{$assertionType}($this->db->isWriteType($sql));

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

$this->{$assertionType}($this->db->isWriteType($sql));

$sql = <<<'SQL'
INSERT INTO my_table (col1, col2)
VALUES ('Joe', 'Cool')
RETURNING id;
SQL;

$this->{$assertionType}($this->db->isWriteType($sql));

$sql = <<<'SQL'
WITH seqvals AS (SELECT '3' AS seqval)
INSERT INTO my_table (col1, col2)
SELECT 'Joe', seqval
FROM seqvals
RETURNING id;
SQL;

$this->{$assertionType}($this->db->isWriteType($sql));
}

public function testUpdate(): void
Expand All @@ -63,11 +101,52 @@

$this->assertTrue($this->db->isWriteType($sql));

if ($this->db->DBDriver === 'Postgre') {
$sql = "UPDATE my_table SET col1 = 'foo' WHERE id = 2 RETURNING *;";
$sql = "WITH seqvals AS (SELECT '3' AS seqval)UPDATE my_table SET col1 = seqval FROM seqvals WHERE id = 2;";

$this->assertTrue($this->db->isWriteType($sql));

$sql = <<<'SQL'
WITH seqvals AS (SELECT '3' AS seqval)
UPDATE my_table
SET col1 = seqval
FROM seqvals
WHERE id = 2;
SQL;

$this->assertFalse($this->db->isWriteType($sql));
$this->assertTrue($this->db->isWriteType($sql));

$assertionType = 'assertTrue';
if ($this->db->DBDriver === 'Postgre') {
$assertionType = 'assertFalse';
}

$sql = "UPDATE my_table SET col1 = 'foo' WHERE id = 2 RETURNING *;";

$this->{$assertionType}($this->db->isWriteType($sql));

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

$this->{$assertionType}($this->db->isWriteType($sql));

$sql = <<<'SQL'
UPDATE my_table
SET col1 = 'foo'
WHERE id = 2
RETURNING *;
SQL;

$this->{$assertionType}($this->db->isWriteType($sql));

$sql = <<<'SQL'
WITH seqvals AS (SELECT '3' AS seqval)
UPDATE my_table
SET col1 = seqval
FROM seqvals
WHERE id = 2
RETURNING *;
SQL;

$this->{$assertionType}($this->db->isWriteType($sql));
}

public function testDelete(): void
Expand All @@ -76,6 +155,56 @@
$sql = $builder->testMode()->delete(['id' => 1], null, true);

$this->assertTrue($this->db->isWriteType($sql));

$sql = 'DELETE FROM my_table WHERE id = 2;';

$this->assertTrue($this->db->isWriteType($sql));

$sql = "WITH seqvals AS (SELECT '3' AS seqval)DELETE FROM my_table JOIN seqvals ON col1 = seqval;";

$this->assertTrue($this->db->isWriteType($sql));

$sql = <<<'SQL'
WITH seqvals AS
(SELECT '3' AS seqval)
DELETE FROM my_table
JOIN seqvals
ON col1 = seqval;
SQL;

$this->assertTrue($this->db->isWriteType($sql));

$assertionType = 'assertTrue';
if ($this->db->DBDriver === 'Postgre') {
$assertionType = 'assertFalse';
}

$sql = 'DELETE FROM my_table WHERE id = 2 RETURNING *;';

$this->{$assertionType}($this->db->isWriteType($sql));

$sql = "WITH seqvals AS (SELECT '3' AS seqval)DELETE FROM my_table JOIN seqvals ON col1 = seqval RETURNING *;";

$this->{$assertionType}($this->db->isWriteType($sql));

$sql = <<<'SQL'
DELETE FROM my_table
WHERE id = 2
RETURNING *;
SQL;

$this->{$assertionType}($this->db->isWriteType($sql));

$sql = <<<'SQL'
WITH seqvals AS
(SELECT '3' AS seqval)
DELETE FROM my_table
JOIN seqvals
ON col1 = seqval
RETURNING *;
SQL;

$this->{$assertionType}($this->db->isWriteType($sql));
}

public function testReplace(): void
Expand Down Expand Up @@ -177,7 +306,7 @@
$this->assertTrue($this->db->isWriteType($sql));
}

public function testUnlock(): void

Check warning on line 309 in tests/system/Database/Live/WriteTypeQueryTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.1, SQLite3, 8.0) / tests 

Took 3.08s from 0.50s limit to run CodeIgniter\\Database\\Live\\WriteTypeQueryTest::testUnlock
{
// i think this is only a valid command for MySQL?
$sql = 'UNLOCK TABLES;';
Expand Down
Loading