Skip to content
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

Allow parsing KILL statement #557

Merged
merged 1 commit into from
May 23, 2024
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
5 changes: 5 additions & 0 deletions src/Parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class Parser extends Core
'BACKUP' => 'PhpMyAdmin\\SqlParser\\Statements\\BackupStatement',
'CHECK' => 'PhpMyAdmin\\SqlParser\\Statements\\CheckStatement',
'CHECKSUM' => 'PhpMyAdmin\\SqlParser\\Statements\\ChecksumStatement',
'KILL' => 'PhpMyAdmin\\SqlParser\\Statements\\KillStatement',
'OPTIMIZE' => 'PhpMyAdmin\\SqlParser\\Statements\\OptimizeStatement',
'REPAIR' => 'PhpMyAdmin\\SqlParser\\Statements\\RepairStatement',
'RESTORE' => 'PhpMyAdmin\\SqlParser\\Statements\\RestoreStatement',
Expand Down Expand Up @@ -210,6 +211,10 @@ class Parser extends Core
'class' => 'PhpMyAdmin\\SqlParser\\Components\\JoinKeyword',
'field' => 'join',
],
'KILL' => [
'class' => 'PhpMyAdmin\\SqlParser\\Components\\Expression',
'field' => 'processListId',
],
'LEFT JOIN' => [
'class' => 'PhpMyAdmin\\SqlParser\\Components\\JoinKeyword',
'field' => 'join',
Expand Down
43 changes: 43 additions & 0 deletions src/Statements/KillStatement.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

declare(strict_types=1);

namespace PhpMyAdmin\SqlParser\Statements;

use PhpMyAdmin\SqlParser\Components\Expression;
use PhpMyAdmin\SqlParser\Components\OptionsArray;
use PhpMyAdmin\SqlParser\Statement;

use function trim;

/**
* `KILL` statement.
*
* KILL [CONNECTION | QUERY] processlist_id
*/
class KillStatement extends Statement
Copy link
Contributor

Choose a reason for hiding this comment

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

I do believe we need some parsing logic here, so we can accurately validate the statement

{
/**
* Options of this statement.
*
* @var array<string, int|array<int, int|string>>
* @psalm-var array<string, (positive-int|array{positive-int, ('var'|'var='|'expr'|'expr=')})>
*/
public static $OPTIONS = [
'CONNECTION' => 1,
'QUERY' => 1,
];
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be awesome if we could extend support of Maria KILL options

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah, makes sense.


/** @var Expression|null */
public $processListId = null;
Copy link
Contributor

@iifawzi iifawzi May 11, 2024

Choose a reason for hiding this comment

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

I believe we should instead utilize var options instead of using custom option

    public static $OPTIONS = [
        'CONNECTION' => [
            1,
            'var',
        ],
        'QUERY' => [
            1,
            'var',
        ],
    ];


public function build(): string
{
$option = $this->options === null || $this->options->isEmpty()
? ''
: ' ' . OptionsArray::build($this->options);
$expression = $this->processListId === null ? '' : ' ' . Expression::build($this->processListId);

return trim('KILL' . $option . $expression);
}
}
5 changes: 4 additions & 1 deletion src/Utils/Query.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use PhpMyAdmin\SqlParser\Statements\DropStatement;
use PhpMyAdmin\SqlParser\Statements\ExplainStatement;
use PhpMyAdmin\SqlParser\Statements\InsertStatement;
use PhpMyAdmin\SqlParser\Statements\KillStatement;
use PhpMyAdmin\SqlParser\Statements\LoadStatement;
use PhpMyAdmin\SqlParser\Statements\OptimizeStatement;
use PhpMyAdmin\SqlParser\Statements\RenameStatement;
Expand Down Expand Up @@ -65,7 +66,7 @@
* limit?: bool,
* offset?: bool,
* order?: bool,
* querytype: ('ALTER'|'ANALYZE'|'CALL'|'CHECK'|'CHECKSUM'|'CREATE'|'DELETE'|'DROP'|'EXPLAIN'|'INSERT'|'LOAD'|'OPTIMIZE'|'REPAIR'|'REPLACE'|'SELECT'|'SET'|'SHOW'|'UPDATE'|false),
* querytype: ('ALTER'|'ANALYZE'|'CALL'|'CHECK'|'CHECKSUM'|'CREATE'|'DELETE'|'DROP'|'EXPLAIN'|'INSERT'|'KILL'|'LOAD'|'OPTIMIZE'|'REPAIR'|'REPLACE'|'SELECT'|'SET'|'SHOW'|'UPDATE'|false),
* reload?: bool,
* select_from?: bool,
* union?: bool
Expand Down Expand Up @@ -431,6 +432,8 @@ public static function getFlags($statement, $all = false)
$flags['is_affected'] = true;
} elseif ($statement instanceof SetStatement) {
$flags['querytype'] = 'SET';
} elseif ($statement instanceof KillStatement) {
$flags['querytype'] = 'KILL';
}

if (
Expand Down
59 changes: 59 additions & 0 deletions tests/Parser/KillStatementTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php

declare(strict_types=1);

namespace PhpMyAdmin\SqlParser\Tests\Parser;

use PhpMyAdmin\SqlParser\Parser;
use PhpMyAdmin\SqlParser\Statements\KillStatement;
use PhpMyAdmin\SqlParser\Tests\TestCase;

class KillStatementTest extends TestCase
{
/**
* @dataProvider killProvider
*/
public function testKill(string $test): void
{
$this->runParserTest($test);
}

/**
* @return string[][]
*/
public function killProvider(): array
{
return [
['parser/parseKill'],
['parser/parseKillConnection'],
['parser/parseKillQuery'],
];
}

/**
* @dataProvider buildKillProvider
*/
public function testBuildKill(string $sql): void
{
$parser = new Parser($sql);
$this->assertCount(1, $parser->statements);
$statement = $parser->statements[0];
$this->assertInstanceOf(KillStatement::class, $statement);
$builtSql = $statement->build();
$this->assertEquals($sql, $builtSql);
}

/**
* @return array<int, array<int, string>>
* @psalm-return list<list<string>>
*/
public function buildKillProvider(): array
{
return [
['KILL (SELECT 3 + 4)'],
Copy link
Contributor

Choose a reason for hiding this comment

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

is it a valid use case? why would someone need to kill a query or connection that way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The use case may be questionable, but the query is valid and working.

Copy link
Contributor

Choose a reason for hiding this comment

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

or maybe someone can utilize this by dynamically killing queries that are locking others? by selecting their IDs from the activity? if that's the case, maybe we can also parse the nested select statements? and delegate the building of these queries to their classes? ( most probably it's only SELECT statement that would be allowed ). what do you think?

['KILL QUERY 3'],
['KILL CONNECTION 3'],
['KILL'],
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be invalid, hence why the parsing logic is so important

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant KILL is what expected to be invalid

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is parsed with an error, I was unsure whether it should be able to build a query that was parsed with error. It is probably not needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's been a while since I interacted with the parser, so I might be mistaken. How this's parsed with an error? if there's no parse function I don't think the statement would be even parsed or validated

Copy link
Contributor

Choose a reason for hiding this comment

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

image

I believe we need the parsing logic so we can throw an error if it's incorrect, the same as we did with the CREATE statement for example

];
}
}
1 change: 1 addition & 0 deletions tests/data/parser/parseKill.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
KILL 1
90 changes: 90 additions & 0 deletions tests/data/parser/parseKill.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
{
"query": "KILL 1",
"lexer": {
"@type": "PhpMyAdmin\\SqlParser\\Lexer",
"str": "KILL 1",
"len": 6,
"last": 6,
"list": {
"@type": "PhpMyAdmin\\SqlParser\\TokensList",
"tokens": [
{
"@type": "PhpMyAdmin\\SqlParser\\Token",
"token": "KILL",
"value": "KILL",
"keyword": "KILL",
"type": 1,
"flags": 3,
"position": 0
},
{
"@type": "PhpMyAdmin\\SqlParser\\Token",
"token": " ",
"value": " ",
"keyword": null,
"type": 3,
"flags": 0,
"position": 4
},
{
"@type": "PhpMyAdmin\\SqlParser\\Token",
"token": "1",
"value": 1,
"keyword": null,
"type": 6,
"flags": 0,
"position": 5
},
{
"@type": "PhpMyAdmin\\SqlParser\\Token",
"token": null,
"value": null,
"keyword": null,
"type": 9,
"flags": 0,
"position": null
}
],
"count": 4,
"idx": 4
},
"delimiter": ";",
"delimiterLen": 1,
"strict": false,
"errors": []
},
"parser": {
"@type": "PhpMyAdmin\\SqlParser\\Parser",
"list": {
"@type": "@1"
},
"statements": [
{
"@type": "PhpMyAdmin\\SqlParser\\Statements\\KillStatement",
"processListId": {
"@type": "PhpMyAdmin\\SqlParser\\Components\\Expression",
"database": null,
"table": null,
"column": null,
"expr": "1",
"alias": null,
"function": null,
"subquery": null
},
"options": {
"@type": "PhpMyAdmin\\SqlParser\\Components\\OptionsArray",
"options": []
},
"first": 0,
"last": 2
}
],
"brackets": 0,
"strict": false,
"errors": []
},
"errors": {
"lexer": [],
"parser": []
}
}
1 change: 1 addition & 0 deletions tests/data/parser/parseKillConnection.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
KILL CONNECTION 1
110 changes: 110 additions & 0 deletions tests/data/parser/parseKillConnection.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
{
"query": "KILL CONNECTION 1",
"lexer": {
"@type": "PhpMyAdmin\\SqlParser\\Lexer",
"str": "KILL CONNECTION 1",
"len": 17,
"last": 17,
"list": {
"@type": "PhpMyAdmin\\SqlParser\\TokensList",
"tokens": [
{
"@type": "PhpMyAdmin\\SqlParser\\Token",
"token": "KILL",
"value": "KILL",
"keyword": "KILL",
"type": 1,
"flags": 3,
"position": 0
},
{
"@type": "PhpMyAdmin\\SqlParser\\Token",
"token": " ",
"value": " ",
"keyword": null,
"type": 3,
"flags": 0,
"position": 4
},
{
"@type": "PhpMyAdmin\\SqlParser\\Token",
"token": "CONNECTION",
"value": "CONNECTION",
"keyword": "CONNECTION",
"type": 1,
"flags": 1,
"position": 5
},
{
"@type": "PhpMyAdmin\\SqlParser\\Token",
"token": " ",
"value": " ",
"keyword": null,
"type": 3,
"flags": 0,
"position": 15
},
{
"@type": "PhpMyAdmin\\SqlParser\\Token",
"token": "1",
"value": 1,
"keyword": null,
"type": 6,
"flags": 0,
"position": 16
},
{
"@type": "PhpMyAdmin\\SqlParser\\Token",
"token": null,
"value": null,
"keyword": null,
"type": 9,
"flags": 0,
"position": null
}
],
"count": 6,
"idx": 6
},
"delimiter": ";",
"delimiterLen": 1,
"strict": false,
"errors": []
},
"parser": {
"@type": "PhpMyAdmin\\SqlParser\\Parser",
"list": {
"@type": "@1"
},
"statements": [
{
"@type": "PhpMyAdmin\\SqlParser\\Statements\\KillStatement",
"processListId": {
"@type": "PhpMyAdmin\\SqlParser\\Components\\Expression",
"database": null,
"table": null,
"column": null,
"expr": "1",
"alias": null,
"function": null,
"subquery": null
},
"options": {
"@type": "PhpMyAdmin\\SqlParser\\Components\\OptionsArray",
"options": {
"1": "CONNECTION"
}
},
"first": 0,
"last": 4
}
],
"brackets": 0,
"strict": false,
"errors": []
},
"errors": {
"lexer": [],
"parser": []
}
}
1 change: 1 addition & 0 deletions tests/data/parser/parseKillQuery.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
KILL QUERY 1
Loading
Loading