Skip to content

feat: pgsql query plan analyzer support #479

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions src/Analyzer/Analyzer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

namespace staabm\PHPStanDba\Analyzer;

interface Analyzer
{
public function analyze(string $query): QueryPlanResult;
}
10 changes: 5 additions & 5 deletions src/Analyzer/QueryPlanAnalyzerMysql.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use PHPStan\ShouldNotHappenException;
use staabm\PHPStanDba\QueryReflection\QueryReflection;

final class QueryPlanAnalyzerMysql
final class QueryPlanAnalyzerMysql implements Analyzer
{
/**
* @deprecated use QueryPlanAnalyzer::DEFAULT_UNINDEXED_READS_THRESHOLD instead
Expand Down Expand Up @@ -89,19 +89,19 @@ private function buildResult(string $simulatedQuery, $it): QueryPlanResult
continue;
}

$result->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) {
continue;
}

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);
}
}
}
Expand Down
81 changes: 81 additions & 0 deletions src/Analyzer/QueryPlanAnalyzerPgSql.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php

declare(strict_types=1);

namespace staabm\PHPStanDba\Analyzer;

use PDO;
use PHPStan\ShouldNotHappenException;
use staabm\PHPStanDba\QueryReflection\QueryReflection;

final class QueryPlanAnalyzerPgSql implements Analyzer
{
/** @var PDO */
private $connection;

public function __construct(PDO $connection)
{
$this->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<array<float|int|string|null>> $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;
}
}
44 changes: 36 additions & 8 deletions src/Analyzer/QueryPlanResult.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renaming existing constants is a BC break. Please leave the old ones additionally in and mark them as deprecated

public const ROW_TABLE_SCAN = 'table-scan';
public const ROW_UNINDEXED_READS = 'unindexed-reads';

/**
* @var array<string, self::*>
* @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<string, self::ROW_*>
*/
private $result = [];

Expand All @@ -31,7 +50,7 @@ public function getSimulatedQuery(): string
}

/**
* @param self::* $result
* @param self::ROW_* $result
*
* @return void
*/
Expand All @@ -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;
}
}
Expand All @@ -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;
}
}
Expand All @@ -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;
}
}
13 changes: 9 additions & 4 deletions src/QueryReflection/QueryReflection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions src/QueryReflection/RecordingQueryReflector.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,9 @@ public function getDatasource()

return null;
}

public function getReflector(): QueryReflector
{
return $this->reflector;
}
}
6 changes: 3 additions & 3 deletions src/Rules/QueryPlanAnalyzerRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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();
}

Expand All @@ -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();
}
}
Expand Down
54 changes: 43 additions & 11 deletions tests/rules/QueryPlanAnalyzerRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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');
}
Expand All @@ -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';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also put the mysql tip onto QueryPlanResult then


$this->analyse([__DIR__.'/data/query-plan-analyzer.php'], [
[
Expand Down Expand Up @@ -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');
}
Expand All @@ -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,
],
Expand Down
2 changes: 1 addition & 1 deletion tests/rules/config/.phpunit-phpstan-dba-pdo-mysql.cache

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading