Skip to content
Merged
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
22 changes: 20 additions & 2 deletions src/SqlQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
final class SqlQuery implements SqlQueryInterface
{
private const C_STYLE_COMMENT = '/\/\*(.*?)\*\//u';
private const LINE_COMMENT = '/^\s*--[^\r\n]*/m';

private PDOStatement|null $pdoStatement = null;

Expand Down Expand Up @@ -123,8 +124,8 @@ private function perform(string $sqlId, array $values, FetchInterface|null $fetc

$this->pdoStatement = $pdoStatement;
$lastQuery = $pdoStatement->queryString;
$query = trim((string) preg_replace(self::C_STYLE_COMMENT, '', $lastQuery));
$isSelect = stripos($query, 'select') === 0 || stripos($query, 'with') === 0;
$queryForDetection = $this->removeCommentsForDetection($lastQuery);
$isSelect = stripos($queryForDetection, 'select') === 0 || stripos($queryForDetection, 'with') === 0;
$result = $isSelect ? $this->fetchAll($pdoStatement, $fetch) : [];
/** @var array<string, mixed> $values */
$this->logger->log($sqlId, $values);
Expand All @@ -143,6 +144,23 @@ private function fetchAll(PDOStatement $pdoStatement, FetchInterface|null $fetch
return $fetch->fetchAll($pdoStatement, $this->injector);
}

/**
* Remove comments from SQL query for query type detection
*
* This strips both C-style (/* *\/) and line comments (--) to prevent
* misidentification of query type. The original query sent to the database
* remains unchanged to preserve performance hints and annotations.
*/
private function removeCommentsForDetection(string $sql): string
{
// Remove C-style comments
$sql = (string) preg_replace(self::C_STYLE_COMMENT, '', $sql);
// Remove line comments (-- at start of line, optionally after whitespace)
$sql = (string) preg_replace(self::LINE_COMMENT, '', $sql);

return trim($sql);
}

/** @return non-empty-array<string> */
private function getSqls(string $sqlFile): array
{
Expand Down
78 changes: 78 additions & 0 deletions tests/SqlCommentTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<?php

declare(strict_types=1);

namespace Ray\MediaQuery;

use Aura\Sql\ExtendedPdo;
use Pagerfanta\View\DefaultView;
use PDO;
use PHPUnit\Framework\TestCase;
use Ray\AuraSqlModule\Pagerfanta\AuraSqlPager;
use Ray\AuraSqlModule\Pagerfanta\AuraSqlPagerFactory;
use Ray\Di\Injector;
use Ray\InputQuery\ToArray;

use function file_get_contents;

class SqlCommentTest extends TestCase
{
private SqlQuery $sqlQuery;

/** @var array<string, mixed> */
private array $insertData = ['id' => '1', 'title' => 'test'];

protected function setUp(): void
{
$sqlDir = __DIR__ . '/sql';
$pdo = new ExtendedPdo('sqlite::memory:', '', '', [PDO::ATTR_STRINGIFY_FETCHES => true]);
$pdo->query((string) file_get_contents($sqlDir . '/create_todo.sql'));
$pdo->perform((string) file_get_contents($sqlDir . '/todo_add.sql'), $this->insertData);

$this->sqlQuery = new SqlQuery(
$pdo,
__DIR__ . '/sql',
new MediaQueryLogger(),
new AuraSqlPagerFactory(new AuraSqlPager(new DefaultView(), [])),
new ParamConverter(new ToArray()),
new Injector(),
new PerformTemplatedSql('{{ sql }}'),
);
}

public function testInsertWithSelectInComment(): void
{
// Critical test: -- SELECT comment should not cause INSERT to be detected as SELECT
$data = ['id' => '2', 'title' => 'test2'];
$this->sqlQuery->exec('todo_insert_with_select_comment', $data);

// Verify the INSERT worked
$result = $this->sqlQuery->getRow('todo_item', ['id' => '2']);
$this->assertSame($data, $result);
}

public function testSelectWithLeadingDashComment(): void
{
$result = $this->sqlQuery->getRow('todo_select_with_leading_comment', ['id' => '1']);
$this->assertSame($this->insertData, $result);
}

public function testUpdateWithInsertComment(): void
{
$updatedTitle = 'updated';
$this->sqlQuery->exec('todo_update_with_insert_comment', ['id' => '1', 'title' => $updatedTitle]);

// Verify the UPDATE worked
$result = $this->sqlQuery->getRow('todo_item', ['id' => '1']);
$this->assertSame(['id' => '1', 'title' => $updatedTitle], $result);
}

public function testDashesInStringLiteral(): void
{
// Test that -- inside string literals is not stripped
// This query should work correctly despite having -- in the WHERE clause
$result = $this->sqlQuery->getRow('todo_with_dashes_in_string', ['id' => '1']);
// Since 'test--value' won't match 'test', we expect null
$this->assertNull($result);
}
}
7 changes: 4 additions & 3 deletions tests/sql/create_promise.sql
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
CREATE TABLE IF NOT EXISTS todo
CREATE TABLE IF NOT EXISTS promise
(
id INTEGER,
title TEXT
id TEXT,
title TEXT,
time TEXT
)
5 changes: 2 additions & 3 deletions tests/sql/create_todo.sql
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
CREATE TABLE IF NOT EXISTS promise
CREATE TABLE IF NOT EXISTS todo
(
id TEXT,
title TEXT,
time TEXT
title TEXT
)
3 changes: 3 additions & 0 deletions tests/sql/todo_insert_with_select_comment.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
-- SELECT for testing INSERT detection
-- This comment should not affect query type detection
INSERT INTO todo (id, title) VALUES (:id, :title);
3 changes: 3 additions & 0 deletions tests/sql/todo_select_with_leading_comment.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
-- Query to select todo by id
-- Author: Test
SELECT * FROM todo WHERE id = :id;
2 changes: 2 additions & 0 deletions tests/sql/todo_update_with_insert_comment.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- INSERT comment that should not trigger INSERT detection
UPDATE todo SET title = :title WHERE id = :id;
1 change: 1 addition & 0 deletions tests/sql/todo_with_dashes_in_string.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
SELECT * FROM todo WHERE title = 'test--value' AND id = :id;
Loading