diff --git a/src/SqlQuery.php b/src/SqlQuery.php index 76ce9268..c267110f 100644 --- a/src/SqlQuery.php +++ b/src/SqlQuery.php @@ -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; @@ -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 $values */ $this->logger->log($sqlId, $values); @@ -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 */ private function getSqls(string $sqlFile): array { diff --git a/tests/SqlCommentTest.php b/tests/SqlCommentTest.php new file mode 100644 index 00000000..e1f601ce --- /dev/null +++ b/tests/SqlCommentTest.php @@ -0,0 +1,78 @@ + */ + 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); + } +} diff --git a/tests/sql/create_promise.sql b/tests/sql/create_promise.sql index 07b22e42..a7f3885e 100644 --- a/tests/sql/create_promise.sql +++ b/tests/sql/create_promise.sql @@ -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 ) \ No newline at end of file diff --git a/tests/sql/create_todo.sql b/tests/sql/create_todo.sql index a7f3885e..4dde87bc 100644 --- a/tests/sql/create_todo.sql +++ b/tests/sql/create_todo.sql @@ -1,6 +1,5 @@ -CREATE TABLE IF NOT EXISTS promise +CREATE TABLE IF NOT EXISTS todo ( id TEXT, - title TEXT, - time TEXT + title TEXT ) \ No newline at end of file diff --git a/tests/sql/todo_insert_with_select_comment.sql b/tests/sql/todo_insert_with_select_comment.sql new file mode 100644 index 00000000..4b40c6b4 --- /dev/null +++ b/tests/sql/todo_insert_with_select_comment.sql @@ -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); diff --git a/tests/sql/todo_select_with_leading_comment.sql b/tests/sql/todo_select_with_leading_comment.sql new file mode 100644 index 00000000..069ee3c4 --- /dev/null +++ b/tests/sql/todo_select_with_leading_comment.sql @@ -0,0 +1,3 @@ +-- Query to select todo by id +-- Author: Test +SELECT * FROM todo WHERE id = :id; diff --git a/tests/sql/todo_update_with_insert_comment.sql b/tests/sql/todo_update_with_insert_comment.sql new file mode 100644 index 00000000..660b0d54 --- /dev/null +++ b/tests/sql/todo_update_with_insert_comment.sql @@ -0,0 +1,2 @@ +-- INSERT comment that should not trigger INSERT detection +UPDATE todo SET title = :title WHERE id = :id; diff --git a/tests/sql/todo_with_dashes_in_string.sql b/tests/sql/todo_with_dashes_in_string.sql new file mode 100644 index 00000000..8bba5502 --- /dev/null +++ b/tests/sql/todo_with_dashes_in_string.sql @@ -0,0 +1 @@ +SELECT * FROM todo WHERE title = 'test--value' AND id = :id;