From f1739353646a4d4833569b8526bb91e6f5e9c5d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20Karf=C3=ADk?= Date: Sun, 20 Nov 2022 17:35:29 +0100 Subject: [PATCH] feat: pgsql query plan analyzer support --- src/Analyzer/Analyzer.php | 10 +++ src/Analyzer/QueryPlanAnalyzerMysql.php | 10 +-- src/Analyzer/QueryPlanAnalyzerPgSql.php | 81 +++++++++++++++++++ src/Analyzer/QueryPlanResult.php | 44 ++++++++-- src/QueryReflection/QueryReflection.php | 13 ++- .../RecordingQueryReflector.php | 5 ++ src/Rules/QueryPlanAnalyzerRule.php | 6 +- tests/rules/QueryPlanAnalyzerRuleTest.php | 54 ++++++++++--- .../.phpunit-phpstan-dba-pdo-mysql.cache | 2 +- tests/rules/data/query-plan-analyzer.php | 18 ++--- 10 files changed, 202 insertions(+), 41 deletions(-) create mode 100644 src/Analyzer/Analyzer.php create mode 100644 src/Analyzer/QueryPlanAnalyzerPgSql.php diff --git a/src/Analyzer/Analyzer.php b/src/Analyzer/Analyzer.php new file mode 100644 index 000000000..996c3000d --- /dev/null +++ b/src/Analyzer/Analyzer.php @@ -0,0 +1,10 @@ +addRow($row['table'], QueryPlanResult::NO_INDEX); + $result->addRow($row['table'], QueryPlanResult::ROW_NO_INDEX); } else { // don't analyse maybe existing data, to make the result consistent with empty db schemas if (QueryPlanAnalyzer::TABLES_WITHOUT_DATA === $allowedRowsNotRequiringIndex) { @@ -97,11 +97,11 @@ private function buildResult(string $simulatedQuery, $it): QueryPlanResult } if (null !== $row['type'] && 'all' === strtolower($row['type']) && $row['rows'] >= $allowedRowsNotRequiringIndex) { - $result->addRow($row['table'], QueryPlanResult::TABLE_SCAN); + $result->addRow($row['table'], QueryPlanResult::ROW_TABLE_SCAN); } elseif (true === $allowedUnindexedReads && $row['rows'] >= QueryPlanAnalyzer::DEFAULT_UNINDEXED_READS_THRESHOLD) { - $result->addRow($row['table'], QueryPlanResult::UNINDEXED_READS); + $result->addRow($row['table'], QueryPlanResult::ROW_UNINDEXED_READS); } elseif (\is_int($allowedUnindexedReads) && $row['rows'] >= $allowedUnindexedReads) { - $result->addRow($row['table'], QueryPlanResult::UNINDEXED_READS); + $result->addRow($row['table'], QueryPlanResult::ROW_UNINDEXED_READS); } } } diff --git a/src/Analyzer/QueryPlanAnalyzerPgSql.php b/src/Analyzer/QueryPlanAnalyzerPgSql.php new file mode 100644 index 000000000..de8e092b2 --- /dev/null +++ b/src/Analyzer/QueryPlanAnalyzerPgSql.php @@ -0,0 +1,81 @@ +connection = $connection; + } + + public function analyze(string $query): QueryPlanResult + { + $simulatedQuery = 'EXPLAIN (FORMAT JSON) '.$query; + + $stmt = $this->connection->query($simulatedQuery); + + return $this->buildResult($simulatedQuery, $stmt); + } + + /** @param \PDOStatement> $stmt */ + private static function buildResult(string $simulatedQuery, $stmt): QueryPlanResult + { + $allowedUnindexedReads = QueryReflection::getRuntimeConfiguration()->getNumberOfAllowedUnindexedReads(); + if (false === $allowedUnindexedReads) { + throw new ShouldNotHappenException(); + } + + $allowedRowsNotRequiringIndex = QueryReflection::getRuntimeConfiguration()->getNumberOfRowsNotRequiringIndex(); + if (false === $allowedRowsNotRequiringIndex) { + throw new ShouldNotHappenException(); + } + + $queryPlanResult = new QueryPlanResult($simulatedQuery); + $result = $stmt->fetch(); + $queryPlans = \is_array($result) && \array_key_exists('QUERY PLAN', $result) + ? json_decode($result['QUERY PLAN'], true) + : []; + \assert(\is_array($queryPlans)); + foreach ($queryPlans as $queryPlan) { + $plan = $queryPlan['Plan']; + $table = $plan['Alias'] ?? null; + + if (null === $table) { + continue; + } + + $rowReads = $plan['Plan Rows']; + $nodeType = $plan['Node Type']; + if ('Seq Scan' === $nodeType && $rowReads >= $allowedRowsNotRequiringIndex) { + $queryPlanResult->addRow($table, QueryPlanResult::ROW_SEQ_SCAN); + } + if ('Bitmap Heap Scan' === $nodeType) { + $queryPlanResult->addRow($table, QueryPlanResult::ROW_BITMAP_HEAP_SCAN); + } + if ('Bitmap Index Scan' === $nodeType) { + $queryPlanResult->addRow($table, QueryPlanResult::ROW_BITMAP_INDEX_SCAN); + } + if ('Index Scan' === $nodeType) { + $queryPlanResult->addRow($table, QueryPlanResult::ROW_INDEX_SCAN); + } + if ('Aggregate' === $nodeType) { + $queryPlanResult->addRow($table, QueryPlanResult::ROW_AGGREGATE); + } + if ('Hash Aggregate' === $nodeType) { + $queryPlanResult->addRow($table, QueryPlanResult::ROW_HASH_AGGREGATE); + } + } + + return $queryPlanResult; + } +} diff --git a/src/Analyzer/QueryPlanResult.php b/src/Analyzer/QueryPlanResult.php index ade98a7c5..c43bc9740 100644 --- a/src/Analyzer/QueryPlanResult.php +++ b/src/Analyzer/QueryPlanResult.php @@ -6,12 +6,31 @@ final class QueryPlanResult { - public const NO_INDEX = 'no-index'; - public const TABLE_SCAN = 'table-scan'; - public const UNINDEXED_READS = 'unindexed-reads'; + public const ROW_NO_INDEX = 'no-index'; + public const ROW_TABLE_SCAN = 'table-scan'; + public const ROW_UNINDEXED_READS = 'unindexed-reads'; /** - * @var array + * @see https://www.postgresql.org/docs/current/sql-explain.html + * @see https://www.postgresql.org/docs/current/using-explain.html + */ + public const ROW_AGGREGATE = 'aggregate'; + public const ROW_BITMAP_HEAP_SCAN = 'bitmap-heap-scan'; + public const ROW_BITMAP_INDEX_SCAN = 'bitmap-index-scan'; + public const ROW_HASH_AGGREGATE = 'hash-aggregate'; + public const ROW_INDEX_SCAN = 'index-scan'; + public const ROW_SEQ_SCAN = 'seq-scan'; + + private const MYSQL_TIPS = [ + self::ROW_NO_INDEX => 'see Mysql Docs https://dev.mysql.com/doc/refman/8.0/en/select-optimization.html', + self::ROW_TABLE_SCAN => 'see Mysql Docs https://dev.mysql.com/doc/refman/8.0/en/table-scan-avoidance.html', + self::ROW_UNINDEXED_READS => 'see Mysql Docs https://dev.mysql.com/doc/refman/8.0/en/select-optimization.html', + ]; + + public const PGSQL_TIP = 'see PostgreSQL Docs https://www.postgresql.org/docs/8.1/performance-tips.html'; + + /** + * @var array */ private $result = []; @@ -31,7 +50,7 @@ public function getSimulatedQuery(): string } /** - * @param self::* $result + * @param self::ROW_* $result * * @return void */ @@ -47,7 +66,7 @@ public function getTablesNotUsingIndex(): array { $tables = []; foreach ($this->result as $table => $result) { - if (self::NO_INDEX === $result) { + if (self::ROW_NO_INDEX === $result || self::ROW_INDEX_SCAN !== $result) { $tables[] = $table; } } @@ -62,7 +81,7 @@ public function getTablesDoingTableScan(): array { $tables = []; foreach ($this->result as $table => $result) { - if (self::TABLE_SCAN === $result) { + if (self::ROW_TABLE_SCAN === $result || self::ROW_SEQ_SCAN === $result) { $tables[] = $table; } } @@ -77,11 +96,20 @@ public function getTablesDoingUnindexedReads(): array { $tables = []; foreach ($this->result as $table => $result) { - if (self::UNINDEXED_READS === $result) { + if (self::ROW_UNINDEXED_READS === $result || self::ROW_INDEX_SCAN !== $result) { $tables[] = $table; } } return $tables; } + + public function getTipForTable(string $table): string + { + $result = $this->result[$table]; + + return \in_array($result, array_keys(self::MYSQL_TIPS), true) + ? self::MYSQL_TIPS[$this->result[$table]] + : self::PGSQL_TIP; + } } diff --git a/src/QueryReflection/QueryReflection.php b/src/QueryReflection/QueryReflection.php index c43139426..796098cc6 100644 --- a/src/QueryReflection/QueryReflection.php +++ b/src/QueryReflection/QueryReflection.php @@ -17,6 +17,7 @@ use PHPStan\Type\TypeUtils; use PHPStan\Type\UnionType; use staabm\PHPStanDba\Analyzer\QueryPlanAnalyzerMysql; +use staabm\PHPStanDba\Analyzer\QueryPlanAnalyzerPgSql; use staabm\PHPStanDba\Analyzer\QueryPlanQueryResolver; use staabm\PHPStanDba\Analyzer\QueryPlanResult; use staabm\PHPStanDba\Ast\ExpressionFinder; @@ -465,15 +466,19 @@ public function analyzeQueryPlan(Scope $scope, Expr $queryExpr, ?Type $parameter if (!$reflector instanceof RecordingReflector) { throw new DbaException('Query plan analysis is only supported with a recording reflector'); } - if ($reflector instanceof PdoPgSqlQueryReflector) { - throw new DbaException('Query plan analysis is not yet supported with the pdo-pgsql reflector, see https://github.com/staabm/phpstan-dba/issues/378'); - } $ds = $reflector->getDatasource(); if (null === $ds) { throw new DbaException(sprintf('Unable to create datasource from %s', \get_class($reflector))); } - $queryPlanAnalyzer = new QueryPlanAnalyzerMysql($ds); + + $innerReflector = method_exists($reflector, 'getReflector') ? $reflector->getReflector() : null; + if ($innerReflector instanceof PdoPgSqlQueryReflector) { + \assert($ds instanceof \PDO); + $queryPlanAnalyzer = new QueryPlanAnalyzerPgSql($ds); + } else { + $queryPlanAnalyzer = new QueryPlanAnalyzerMysql($ds); + } $queryResolver = new QueryPlanQueryResolver(); foreach ($queryResolver->resolve($scope, $queryExpr, $parameterTypes) as $queryString) { diff --git a/src/QueryReflection/RecordingQueryReflector.php b/src/QueryReflection/RecordingQueryReflector.php index 168b5594e..a54406309 100644 --- a/src/QueryReflection/RecordingQueryReflector.php +++ b/src/QueryReflection/RecordingQueryReflector.php @@ -63,4 +63,9 @@ public function getDatasource() return null; } + + public function getReflector(): QueryReflector + { + return $this->reflector; + } } diff --git a/src/Rules/QueryPlanAnalyzerRule.php b/src/Rules/QueryPlanAnalyzerRule.php index c1ce14591..3776742dd 100644 --- a/src/Rules/QueryPlanAnalyzerRule.php +++ b/src/Rules/QueryPlanAnalyzerRule.php @@ -154,7 +154,7 @@ private function analyze(CallLike $callLike, Scope $scope): array $table )) ->line($callLike->getLine()) - ->tip('see Mysql Docs https://dev.mysql.com/doc/refman/8.0/en/select-optimization.html') + ->tip($queryPlanResult->getTipForTable($table)) ->build(); } } else { @@ -165,7 +165,7 @@ private function analyze(CallLike $callLike, Scope $scope): array $table )) ->line($callLike->getLine()) - ->tip('see Mysql Docs https://dev.mysql.com/doc/refman/8.0/en/table-scan-avoidance.html') + ->tip($queryPlanResult->getTipForTable($table)) ->build(); } @@ -176,7 +176,7 @@ private function analyze(CallLike $callLike, Scope $scope): array $table )) ->line($callLike->getLine()) - ->tip('see Mysql Docs https://dev.mysql.com/doc/refman/8.0/en/select-optimization.html') + ->tip($queryPlanResult->getTipForTable($table)) ->build(); } } diff --git a/tests/rules/QueryPlanAnalyzerRuleTest.php b/tests/rules/QueryPlanAnalyzerRuleTest.php index 435778955..6d9d70a4f 100644 --- a/tests/rules/QueryPlanAnalyzerRuleTest.php +++ b/tests/rules/QueryPlanAnalyzerRuleTest.php @@ -4,6 +4,7 @@ use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; +use staabm\PHPStanDba\Analyzer\QueryPlanResult; use staabm\PHPStanDba\QueryReflection\QueryReflection; use staabm\PHPStanDba\Rules\QueryPlanAnalyzerRule; @@ -48,10 +49,6 @@ public static function getAdditionalConfigFiles(): array public function testNotUsingIndex(): void { - if ('pdo-pgsql' === getenv('DBA_REFLECTOR')) { - $this->markTestSkipped('query plan analyzer is not yet implemented for pgsql'); - } - if ('recording' !== getenv('DBA_MODE')) { $this->markTestSkipped('query plan analyzer requires a active database connection'); } @@ -60,7 +57,9 @@ public function testNotUsingIndex(): void $this->numberOfRowsNotRequiringIndex = 2; $proposal = "\n\nConsider optimizing the query.\nIn some cases this is not a problem and this error should be ignored."; - $tip = 'see Mysql Docs https://dev.mysql.com/doc/refman/8.0/en/select-optimization.html'; + $tip = 'pdo-pgsql' === getenv('DBA_REFLECTOR') + ? QueryPlanResult::PGSQL_TIP + : 'see Mysql Docs https://dev.mysql.com/doc/refman/8.0/en/select-optimization.html'; $this->analyse([__DIR__.'/data/query-plan-analyzer.php'], [ [ @@ -93,10 +92,6 @@ public function testNotUsingIndex(): void public function testNotUsingIndexInDebugMode(): void { - if ('pdo-pgsql' === getenv('DBA_REFLECTOR')) { - $this->markTestSkipped('query plan analyzer is not yet implemented for pgsql'); - } - if ('recording' !== getenv('DBA_MODE')) { $this->markTestSkipped('query plan analyzer requires a active database connection'); } @@ -106,16 +101,53 @@ public function testNotUsingIndexInDebugMode(): void $this->numberOfRowsNotRequiringIndex = 2; $proposal = "\n\nConsider optimizing the query.\nIn some cases this is not a problem and this error should be ignored."; + if ('pdo-pgsql' === getenv('DBA_REFLECTOR')) { + $this->analyse([__DIR__.'/data/query-plan-analyzer.php'], [ + [ + "Query is not using an index on table 'ada'.".$proposal."\n\nSimulated query: EXPLAIN (FORMAT JSON) SELECT * FROM ada WHERE email = 'test@example.com'", + 12, + QueryPlanResult::PGSQL_TIP, + ], + [ + "Query is not using an index on table 'ada'.".$proposal."\n\nSimulated query: EXPLAIN (FORMAT JSON) SELECT *,adaid FROM ada WHERE email = 'test@example.com'", + 17, + QueryPlanResult::PGSQL_TIP, + ], + [ + "Query is not using an index on table 'ada'.".$proposal."\n\nSimulated query: EXPLAIN (FORMAT JSON) SELECT * FROM ada WHERE email = '1970-01-01'", + 22, + QueryPlanResult::PGSQL_TIP, + ], + [ + "Query is not using an index on table 'ada'.".$proposal."\n\nSimulated query: EXPLAIN (FORMAT JSON) SELECT * FROM ada WHERE email = '1970-01-01'", + 23, + QueryPlanResult::PGSQL_TIP, + ], + [ + "Query is not using an index on table 'ada'.".$proposal."\n\nSimulated query: EXPLAIN (FORMAT JSON) SELECT * FROM ada WHERE email = '1970-01-01'", + 28, + QueryPlanResult::PGSQL_TIP, + ], + [ + 'Unresolvable Query: Cannot simulate parameter value for type: mixed.', + 61, + 'Make sure all variables involved have a non-mixed type and array-types are specified.', + ], + ]); + + return; + } + $tip = 'see Mysql Docs https://dev.mysql.com/doc/refman/8.0/en/select-optimization.html'; $this->analyse([__DIR__.'/data/query-plan-analyzer.php'], [ [ - "Query is not using an index on table 'ada'.".$proposal."\n\nSimulated query: EXPLAIN SELECT * FROM `ada` WHERE email = 'test@example.com'", + "Query is not using an index on table 'ada'.".$proposal."\n\nSimulated query: EXPLAIN SELECT * FROM ada WHERE email = 'test@example.com'", 12, $tip, ], [ - "Query is not using an index on table 'ada'.".$proposal."\n\nSimulated query: EXPLAIN SELECT *,adaid FROM `ada` WHERE email = 'test@example.com'", + "Query is not using an index on table 'ada'.".$proposal."\n\nSimulated query: EXPLAIN SELECT *,adaid FROM ada WHERE email = 'test@example.com'", 17, $tip, ], diff --git a/tests/rules/config/.phpunit-phpstan-dba-pdo-mysql.cache b/tests/rules/config/.phpunit-phpstan-dba-pdo-mysql.cache index f5419db5a..e59ccccef 100644 --- a/tests/rules/config/.phpunit-phpstan-dba-pdo-mysql.cache +++ b/tests/rules/config/.phpunit-phpstan-dba-pdo-mysql.cache @@ -3601,4 +3601,4 @@ Simulated query: SELECT FROM WHERE LIMIT 0', 'debugMode' => false, 'stringifyTypes' => false, ), -); \ No newline at end of file +); diff --git a/tests/rules/data/query-plan-analyzer.php b/tests/rules/data/query-plan-analyzer.php index ba4e69e64..15d8979c4 100644 --- a/tests/rules/data/query-plan-analyzer.php +++ b/tests/rules/data/query-plan-analyzer.php @@ -9,12 +9,12 @@ class Foo { public function noindex(PDO $pdo): void { - $pdo->query("SELECT * FROM `ada` WHERE email = 'test@example.com';"); + $pdo->query("SELECT * FROM ada WHERE email = 'test@example.com';"); } public function noindexDbal(Connection $conn): void { - $conn->executeQuery("SELECT *,adaid FROM `ada` WHERE email = 'test@example.com';"); + $conn->executeQuery("SELECT *,adaid FROM ada WHERE email = 'test@example.com';"); } public function noindexPreparedDbal(Connection $conn, string $email): void @@ -35,7 +35,7 @@ public function syntaxError(Connection $conn): void public function indexed(PDO $pdo, int $adaid): void { - $pdo->query('SELECT * FROM `ada` WHERE adaid = '.$adaid); + $pdo->query('SELECT * FROM ada WHERE adaid = '.$adaid); } public function indexedPrepared(Connection $conn, int $adaidl): void @@ -45,10 +45,10 @@ public function indexedPrepared(Connection $conn, int $adaidl): void public function writes(PDO $pdo, int $adaid): void { - $pdo->query('UPDATE `ada` SET email="test" WHERE adaid = '.$adaid); - $pdo->query('INSERT INTO `ada` SET email="test" WHERE adaid = '.$adaid); - $pdo->query('REPLACE INTO `ada` SET email="test" WHERE adaid = '.$adaid); - $pdo->query('DELETE FROM `ada` WHERE adaid = '.$adaid); + $pdo->query('UPDATE ada SET email="test" WHERE adaid = '.$adaid); + $pdo->query('INSERT INTO ada SET email="test" WHERE adaid = '.$adaid); + $pdo->query('REPLACE INTO ada SET email="test" WHERE adaid = '.$adaid); + $pdo->query('DELETE FROM ada WHERE adaid = '.$adaid); } public function unknownQuery(Connection $conn, string $query): void @@ -64,12 +64,12 @@ public function nonSimulatableQuery(Connection $conn, $email): void public function bug442(Connection $conn, string $table) { // just make sure we don't fatal error - $conn->fetchAllAssociative("SELECT * FROM `$table`"); + $conn->fetchAllAssociative("SELECT * FROM " . $table); $query = 'SELECT email, adaid FROM '. $table .' WHERE adaid = ?'; $conn->fetchAssociative($query, [1]); - $query = "SELECT email, adaid FROM `$table` WHERE adaid = ?"; + $query = "SELECT email, adaid FROM " . $table . " WHERE adaid = ?"; $conn->fetchAssociative($query, [1]); } }