Skip to content
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
7 changes: 7 additions & 0 deletions src/Components/AlterOperation.php
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ public static function parse(Parser $parser, TokensList $list, array $options =
* @var int
*/
$brackets = 0;
$isFirstUnknownToken = true;

/**
* The state of the parser.
Expand Down Expand Up @@ -434,6 +435,12 @@ public static function parse(Parser $parser, TokensList $list, array $options =

$state = 2;
} elseif ($state === 2) {
if ($isFirstUnknownToken && $ret->options->has('ADD') === true && $token->keyword === 'VARCHAR') {
// Validate the column type while preserving the original tokens used to build ALTER.
DataType::parse($parser, clone $list);
}

$isFirstUnknownToken = false;
if (is_string($token->value) || is_int($token->value)) {
$arrayKey = $token->value;
} else {
Expand Down
33 changes: 33 additions & 0 deletions src/Components/DataType.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use PhpMyAdmin\SqlParser\TokensList;

use function implode;
use function preg_match;
use function strtolower;
use function strtoupper;
use function trim;
Expand Down Expand Up @@ -133,7 +134,39 @@ public static function parse(Parser $parser, TokensList $list, array $options =
$state = 1;
} elseif ($state === 1) {
if (($token->type === Token::TYPE_OPERATOR) && ($token->value === '(')) {
$parametersStart = $list->idx + 1;
$parameters = ArrayObj::parse($parser, $list);
if ($ret->name === 'VARCHAR') {
$hasLength = false;
for ($idx = $parametersStart; $idx < $list->idx; ++$idx) {
$parameter = $list->tokens[$idx];
if (
$parameter->type === Token::TYPE_WHITESPACE || $parameter->type === Token::TYPE_COMMENT
) {
continue;
}

// Check the original lexeme: conversion can turn 20.0 or '20' into 20.
if (
$hasLength || $parameter->type !== Token::TYPE_NUMBER
|| preg_match('/^[0-9]+$/D', $parameter->token) !== 1
) {
$parser->error('VARCHAR length must be a single nonnegative integer.', $parameter);
$hasLength = true;
break;
}

$hasLength = true;
}

if (! $hasLength) {
$parser->error(
'VARCHAR length must be a single nonnegative integer.',
$list->tokens[$list->idx] ?? null
);
}
}

++$list->idx;
$ret->parameters = ($ret->name === 'ENUM') || ($ret->name === 'SET') ?
$parameters->raw : $parameters->values;
Expand Down
185 changes: 185 additions & 0 deletions tests/Components/DataTypeTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
<?php

declare(strict_types=1);

namespace PhpMyAdmin\SqlParser\Tests\Components;

use PhpMyAdmin\SqlParser\Components\DataType;
use PhpMyAdmin\SqlParser\Exceptions\ParserException;
use PhpMyAdmin\SqlParser\Parser;
use PhpMyAdmin\SqlParser\Statements\AlterStatement;
use PhpMyAdmin\SqlParser\Statements\CreateStatement;
use PhpMyAdmin\SqlParser\Statements\SelectStatement;
use PhpMyAdmin\SqlParser\Tests\TestCase;
use PHPUnit\Framework\Attributes\DataProvider;

class DataTypeTest extends TestCase
{
private const LENGTH_ERROR = 'VARCHAR length must be a single nonnegative integer.';

/**
* @dataProvider validLengthProvider
*/
#[DataProvider('validLengthProvider')]
public function testValidLength(string $declaration, string $length): void
{
$parser = new Parser();
$type = DataType::parse($parser, $this->getTokensList($declaration . ' COLLATE utf8mb4_bin NULL'));
self::assertSame([], $parser->errors);
self::assertNotNull($type);
self::assertSame('VARCHAR', $type->name);
self::assertSame([$length], $type->parameters);
self::assertSame('utf8mb4_bin', $type->options->has('COLLATE', true));

$parser = new Parser('CREATE TABLE temp_users (phone ' . $declaration . ' COLLATE utf8mb4_bin NULL);');
self::assertSame([], $parser->errors);
self::assertInstanceOf(CreateStatement::class, $parser->statements[0]);
self::assertIsArray($parser->statements[0]->fields);
$field = $parser->statements[0]->fields[0];
self::assertNotNull($field->type);
self::assertSame([$length], $field->type->parameters);
self::assertNotNull($field->options);
self::assertTrue($field->options->has('NULL'));

$parser = new Parser('ALTER TABLE temp_users ADD COLUMN phone ' . $declaration . ' NULL;');
self::assertSame([], $parser->errors);
self::assertInstanceOf(AlterStatement::class, $parser->statements[0]);
self::assertStringContainsString($length, $parser->statements[0]->build());
self::assertStringContainsString('NULL', $parser->statements[0]->build());
}

/** @return array<string, array{string, string}> */
public static function validLengthProvider(): array
{
return [
'ordinary' => ['VARCHAR(20)', '20'],
'zero' => ['VARCHAR(0)', '0'],
'leading zeros' => ['VARCHAR(020)', '20'],
'mixed case and whitespace' => ["vArChAr \n ( 20\t )", '20'],
'comments' => ['varchar /* type */ ( /* before */ 20 /* after */ )', '20'],
'line comments' => ["varchar( -- before\n 20 # after\n )", '20'],
];
}

/**
* @dataProvider invalidLengthProvider
*/
#[DataProvider('invalidLengthProvider')]
public function testInvalidLength(string $length, string $invalidToken): void
{
$parser = new Parser();
DataType::parse($parser, $this->getTokensList('VARCHAR(' . $length . ') NULL'));
self::assertCount(1, $parser->errors);
self::assertSame(self::LENGTH_ERROR, $parser->errors[0]->getMessage());
self::assertInstanceOf(ParserException::class, $parser->errors[0]);
self::assertNotNull($parser->errors[0]->token);
self::assertSame($invalidToken, $parser->errors[0]->token->token);

foreach (self::columnQueries('VARCHAR(' . $length . ')') as $query) {
$parser = new Parser($query . ' SELECT 1;');
self::assertCount(1, $parser->errors);
self::assertSame(self::LENGTH_ERROR, $parser->errors[0]->getMessage());
self::assertInstanceOf(ParserException::class, $parser->errors[0]);
self::assertNotNull($parser->errors[0]->token);
self::assertSame($invalidToken, $parser->errors[0]->token->token);
self::assertCount(2, $parser->statements);
self::assertInstanceOf(SelectStatement::class, $parser->statements[1]);

try {
new Parser($query, true);
self::fail('Strict mode must reject an invalid VARCHAR length.');
} catch (ParserException $exception) {
self::assertSame(self::LENGTH_ERROR, $exception->getMessage());
}
}
}

/** @return array<string, array{string, string}> */
public static function invalidLengthProvider(): array
{
return [
'identifier' => ['X', 'X'],
'quoted string' => ["'20'", "'20'"],
'double quoted string' => ['"20"', '"20"'],
'quoted identifier' => ['`20`', '`20`'],
'fraction' => ['20.0', '20.0'],
'negative' => ['-20', '-20'],
'positive sign' => ['+20', '+20'],
'scientific' => ['2e1', '2e1'],
'hexadecimal' => ['0x14', '0x14'],
'binary' => ['0b10100', '0b10100'],
'expression' => ['1+19', '+19'],
'nested parentheses' => ['(20)', '('],
'empty' => ['', ')'],
'only comments' => [' /* empty */ ', ')'],
'extra parameter' => ['20,30', ','],
'trailing comma' => ['20,', ','],
'separate numbers' => ['20 /* gap */ 30', '30'],
];
}

/** @return list<string> */
private static function columnQueries(string $declaration): array
{
return [
'CREATE TABLE temp_users (phone ' . $declaration . ' NULL);',
'ALTER TABLE temp_users ADD COLUMN phone ' . $declaration . ' NULL;',
'ALTER TABLE temp_users ADD phone ' . $declaration . ' NULL;',
];
}

/**
* @param list<string> $parameters
*
* @dataProvider otherTypeProvider
*/
#[DataProvider('otherTypeProvider')]
public function testOtherTypes(string $declaration, array $parameters): void
{
$parser = new Parser();
$type = DataType::parse($parser, $this->getTokensList($declaration));
self::assertSame([], $parser->errors);
self::assertNotNull($type);
self::assertSame($parameters, $type->parameters);
}

/** @return array<string, array{string, list<string>}> */
public static function otherTypeProvider(): array
{
return [
'enum' => ["ENUM('a','b')", ["'a'", "'b'"]],
'set' => ["SET('a','b')", ["'a'", "'b'"]],
'decimal' => ['DECIMAL(10,2)', ['10', '2']],
'lengthless varchar' => ['VARCHAR', []],
];
}

public function testLengthlessRoutine(): void
{
$parser = new Parser('CREATE PROCEDURE p(IN phone VARCHAR) SELECT phone;');
self::assertSame([], $parser->errors);
}

public function testAlterContinuation(): void
{
$parser = new Parser(
'ALTER TABLE temp_users ADD COLUMN phone VARCHAR(X) NULL, ADD COLUMN name VARCHAR(20) NOT NULL;'
);
self::assertCount(1, $parser->errors);
self::assertSame(self::LENGTH_ERROR, $parser->errors[0]->getMessage());
self::assertInstanceOf(AlterStatement::class, $parser->statements[0]);
self::assertNotNull($parser->statements[0]->altered);
self::assertCount(2, $parser->statements[0]->altered);
self::assertStringContainsString('VARCHAR(20) NOT NULL', $parser->statements[0]->build());
}

public function testAlterStringParameters(): void
{
$parser = new Parser("ALTER TABLE temp_users ADD COLUMN e ENUM('a','b'), ADD COLUMN s SET('a','b');");
self::assertSame([], $parser->errors);
self::assertInstanceOf(AlterStatement::class, $parser->statements[0]);
self::assertNotNull($parser->statements[0]->altered);
self::assertCount(2, $parser->statements[0]->altered);
self::assertStringContainsString("ENUM('a','b')", $parser->statements[0]->build());
}
}
1 change: 1 addition & 0 deletions tests/Parser/AlterStatementTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public static function alterProvider(): array
['parser/parseAlterTablePartitionByRange2'],
['parser/parseAlterTableCoalescePartition'],
['parser/parseAlterTableAddColumnWithCheck'],
['parser/parseAlterTableAddColumnInvalidVarcharLength'],
['parser/parseAlterTableAddSpatialIndex1'],
['parser/parseAlterTableAddUniqueKey1'],
['parser/parseAlterTableAddUniqueKey2'],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE temp_users ADD COLUMN phone VARCHAR(X) NULL;
Loading
Loading