Skip to content

Commit af71a9e

Browse files
jakubvojacekstaabm
andauthored
Support for delete, insert, update, replace (staabm#483)
Co-authored-by: Jakub Vojacek <[email protected]> Co-authored-by: Markus Staab <[email protected]>
1 parent c28af8a commit af71a9e

32 files changed

+6928
-6493
lines changed

.phpstan-dba-mysqli.cache

Lines changed: 163 additions & 163 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.phpstan-dba-pdo-mysql.cache

Lines changed: 164 additions & 164 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ This extension provides the following features, as long as you [stick to the rul
1212
* [query plan analysis](https://staabm.github.io/2022/08/16/phpstan-dba-query-plan-analysis.html) to detect performance issues
1313
* builtin support for `doctrine/dbal`, `mysqli`, and `PDO`
1414
* API to configure the same features for your custom sql based database access layer
15+
* Opt-In analysis of write queries (since version 0.2.55+)
1516

1617
In case you are using Doctrine ORM, you might use `phpstan-dba` in tandem with [phpstan-doctrine](https://github.com/phpstan/phpstan-doctrine).
1718

@@ -56,6 +57,7 @@ $config = new RuntimeConfiguration();
5657
// $config->debugMode(true);
5758
// $config->stringifyTypes(true);
5859
// $config->analyzeQueryPlans(true);
60+
// $config->analyzeWriteQueries(true); // requires transaction support in db schema and db driver
5961

6062
// TODO: Put your database credentials here
6163
$mysqli = new mysqli('hostname', 'username', 'password', 'database');

docs/configuration.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,4 @@ If not configured otherwise, the following defaults are used:
88
- when analyzing a php8+ codebase, [`PDO::ERRMODE_EXCEPTION` error handling](https://www.php.net/manual/en/pdo.error-handling.php) is assumed.
99
- when analyzing a php8.1+ codebase, [`mysqli_report(\MYSQLI_REPORT_ERROR | \MYSQLI_REPORT_STRICT);` error handling](https://www.php.net/mysqli_report) is assumed.
1010
- the fetch mode defaults to `QueryReflector::FETCH_TYPE_BOTH`, but can be configured using the [`defaultFetchMode`](https://github.com/staabm/phpstan-dba/tree/main/src/QueryReflection/RuntimeConfiguration.php) option.
11+
- only readable queries are analyzed per default. In case your database schema and database driver support transactions, you may consider enabled writable queries using the [`analyzeWriteQueries`](https://github.com/staabm/phpstan-dba/tree/main/src/QueryReflection/RuntimeConfiguration.php) option.

src/Analyzer/QueryPlanAnalyzerMysql.php

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,26 @@ public function analyze(string $query): QueryPlanResult
5151
$simulatedQuery = 'EXPLAIN '.$query;
5252

5353
if ($this->connection instanceof PDO) {
54-
$stmt = $this->connection->query($simulatedQuery);
54+
$this->connection->beginTransaction();
5555

56-
// @phpstan-ignore-next-line
57-
return $this->buildResult($simulatedQuery, $stmt);
56+
try {
57+
$stmt = $this->connection->query($simulatedQuery);
58+
59+
// @phpstan-ignore-next-line
60+
return $this->buildResult($simulatedQuery, $stmt);
61+
} finally {
62+
$this->connection->rollBack();
63+
}
5864
} else {
59-
$result = $this->connection->query($simulatedQuery);
60-
if ($result instanceof \mysqli_result) {
61-
return $this->buildResult($simulatedQuery, $result);
65+
$this->connection->begin_transaction();
66+
67+
try {
68+
$result = $this->connection->query($simulatedQuery);
69+
if ($result instanceof \mysqli_result) {
70+
return $this->buildResult($simulatedQuery, $result);
71+
}
72+
} finally {
73+
$this->connection->rollback();
6274
}
6375
}
6476

src/Ast/ExpressionFinder.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,6 @@ public function findBindCalls(Expr $expr): array
109109
}
110110

111111
/**
112-
* XXX use astral simpleNameResolver instead.
113-
*
114112
* @param Expr|Variable|MethodCall $node
115113
*
116114
* @return string|null

src/DbSchema/SchemaHasherMysql.php

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,27 @@ public function hashDb(): string
6161

6262
$hash = '';
6363
if ($this->connection instanceof PDO) {
64-
$stmt = $this->connection->query($query);
65-
foreach ($stmt as $row) {
66-
$hash = $row['dbsignature'] ?? '';
64+
$this->connection->beginTransaction();
65+
66+
try {
67+
$stmt = $this->connection->query($query);
68+
foreach ($stmt as $row) {
69+
$hash = $row['dbsignature'] ?? '';
70+
}
71+
} finally {
72+
$this->connection->rollBack();
6773
}
6874
} else {
69-
$result = $this->connection->query($query);
70-
if ($result instanceof \mysqli_result) {
71-
$row = $result->fetch_assoc();
72-
$hash = $row['dbsignature'] ?? '';
75+
$this->connection->begin_transaction();
76+
77+
try {
78+
$result = $this->connection->query($query);
79+
if ($result instanceof \mysqli_result) {
80+
$row = $result->fetch_assoc();
81+
$hash = $row['dbsignature'] ?? '';
82+
}
83+
} finally {
84+
$this->connection->rollback();
7385
}
7486
}
7587

src/QueryReflection/MysqliQueryReflector.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ public function __construct(mysqli $mysqli)
5656
$this->db->set_charset('utf8');
5757
// enable exception throwing on php <8.1
5858
mysqli_report(\MYSQLI_REPORT_ERROR | \MYSQLI_REPORT_STRICT);
59+
$this->db->autocommit(false);
5960
}
6061

6162
public function validateQueryString(string $queryString): ?Error
@@ -145,7 +146,11 @@ private function simulateQuery(string $queryString)
145146
return $this->cache[$queryString] = null;
146147
}
147148

148-
$this->db->begin_transaction(\MYSQLI_TRANS_START_READ_ONLY);
149+
if (QueryReflection::getRuntimeConfiguration()->isAnalyzingWriteQueries()) {
150+
$this->db->begin_transaction();
151+
} else {
152+
$this->db->begin_transaction(\MYSQLI_TRANS_START_READ_ONLY);
153+
}
149154

150155
try {
151156
$result = $this->db->query($simulatedQuery);

src/QueryReflection/PdoMysqlQueryReflector.php

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,24 +48,14 @@ protected function simulateQuery(string $queryString)
4848
return $this->cache[$queryString] = null;
4949
}
5050

51-
try {
52-
$this->pdo->beginTransaction();
53-
} catch (PDOException $e) {
54-
// not all drivers may support transactions
55-
throw new \RuntimeException('Failed to start transaction', $e->getCode(), $e);
56-
}
51+
$this->pdo->beginTransaction();
5752

5853
try {
5954
$stmt = $this->pdo->query($simulatedQuery);
6055
} catch (PDOException $e) {
6156
return $this->cache[$queryString] = $e;
6257
} finally {
63-
try {
64-
$this->pdo->rollBack();
65-
} catch (PDOException $e) {
66-
// not all drivers may support transactions
67-
throw new \RuntimeException('Failed to rollback transaction', $e->getCode(), $e);
68-
}
58+
$this->pdo->rollBack();
6959
}
7060

7161
$this->cache[$queryString] = [];

src/QueryReflection/QueryReflection.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,20 @@ public static function setupReflector(QueryReflector $reflector, RuntimeConfigur
5959

6060
public function validateQueryString(string $queryString): ?Error
6161
{
62-
if ('SELECT' !== self::getQueryType($queryString)) {
63-
return null;
62+
if (self::getRuntimeConfiguration()->isAnalyzingWriteQueries()) {
63+
if (!\in_array(self::getQueryType($queryString), [
64+
'SELECT',
65+
'INSERT',
66+
'DELETE',
67+
'UPDATE',
68+
'REPLACE',
69+
], true)) {
70+
return null;
71+
}
72+
} else {
73+
if ('SELECT' !== self::getQueryType($queryString)) {
74+
return null;
75+
}
6476
}
6577

6678
// this method cannot validate queries which contain placeholders.

src/QueryReflection/QuerySimulation.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,11 @@ public static function simulate(string $queryString): ?string
129129
if (null === $queryString) {
130130
return null;
131131
}
132-
$queryString .= ' LIMIT 0';
132+
133+
// make sure we don't unnecessarily transfer data, as we are only interested in the statement is succeeding
134+
if ('SELECT' === QueryReflection::getQueryType($queryString)) {
135+
$queryString .= ' LIMIT 0';
136+
}
133137

134138
return $queryString;
135139
}

src/QueryReflection/RuntimeConfiguration.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ final class RuntimeConfiguration
4343
* @var bool
4444
*/
4545
private $stringifyTypes = false;
46+
/**
47+
* @var bool
48+
*/
49+
private $writableQueries = false;
4650
/**
4751
* @var bool|0|positive-int
4852
*/
@@ -107,6 +111,21 @@ public function stringifyTypes(bool $stringify): self
107111
return $this;
108112
}
109113

114+
/**
115+
* Enables checking of writable queries (INSERT, UPDATE, DELETE,...).
116+
*
117+
* This feature requires a database and a database driver which supports transactions.
118+
* Otherwise, the analysis might lead to data loss!
119+
*
120+
* Also make sure your mysql tables use the InnoDB engine.
121+
*/
122+
public function analyzeWriteQueries(bool $enabled): self
123+
{
124+
$this->writableQueries = $enabled;
125+
126+
return $this;
127+
}
128+
110129
/**
111130
* Enables query plan analysis, which indicates performance problems.
112131
*
@@ -154,6 +173,11 @@ public function isStringifyTypes(): bool
154173
return $this->stringifyTypes;
155174
}
156175

176+
public function isAnalyzingWriteQueries(): bool
177+
{
178+
return $this->writableQueries;
179+
}
180+
157181
/**
158182
* @return QueryReflector::FETCH_TYPE*
159183
*/

0 commit comments

Comments
 (0)