Skip to content

Commit

Permalink
Fix confusion between CHARACTER data type and CHARACTER SET issue (#376)
Browse files Browse the repository at this point in the history
* Confusion between CHARACTER type and CHARACTER SET

In an attempt to fix the character set issue introduced by me in
#355

This change separates out the CHARACTER check to do a look ahead
to see if the next keyword is SET and splits the logic appropriately.

An alternative would be to look back at the constructed expression
array ($expr) to see if a ExpressionType::DATA_TYPE has already been
defined.

This is an alternative fix to the fix proposed by
#366

* Updated removed PHPUnit expect function.

* Added additional tests for explicit conditions.
  • Loading branch information
xsist10 authored Dec 18, 2023
1 parent 924ee80 commit dec667f
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 3 deletions.
30 changes: 28 additions & 2 deletions src/PHPSQLParser/processors/ColumnDefinitionProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,19 @@ protected function buildColDef($expr, $base_expr, $options, $refs, $key) {
return $expr;
}

protected function peekAtNextToken($tokens, $index)
{
$offset = $index + 1;
while (isset($tokens[$offset])) {
$token = trim($tokens[$offset]);
if ($token !== '') {
return strtoupper($token);
}
$offset++;
}
return '';
}

public function process($tokens) {

$trim = '';
Expand Down Expand Up @@ -187,7 +200,6 @@ public function process($tokens) {
continue 2;

case 'CHAR':
case 'CHARACTER': // Alias for CHAR
$expr[] = array('expr_type' => ExpressionType::DATA_TYPE, 'base_expr' => $trim, 'length' => false);
$currCategory = 'SINGLE_PARAM_PARENTHESIS';
$prevCategory = 'TEXT';
Expand Down Expand Up @@ -259,11 +271,25 @@ public function process($tokens) {
// spatial types
continue 2;

case 'CHARACTER':
case 'CHARSET':
$currCategory = 'CHARSET';
$options['sub_tree'][] = array('expr_type' => ExpressionType::RESERVED, 'base_expr' => $trim);
continue 2;

case 'CHARACTER':
// Alias of CHAR as well as pre-running for CHARACTER SET
// To determine which we peek at the next token to see if it's a SET or not.
if ($this->peekAtNextToken($tokens, $key) == 'SET') {
$currCategory = 'CHARSET';
$options['sub_tree'][] = array('expr_type' => ExpressionType::RESERVED, 'base_expr' => $trim);
// If it's not a SET we assume that it is a CHARACTER type definition
} else {
$expr[] = array('expr_type' => ExpressionType::DATA_TYPE, 'base_expr' => $trim, 'length' => false);
$currCategory = 'SINGLE_PARAM_PARENTHESIS';
$prevCategory = 'TEXT';
}
continue 2;

case 'SET':
if ($currCategory == 'CHARSET') {
$options['sub_tree'][] = array('expr_type' => ExpressionType::RESERVED, 'base_expr' => $trim);
Expand Down
2 changes: 1 addition & 1 deletion tests/cases/parser/issue320Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public function test_no_warning_is_issued_when_help_table_is_used()
// there currently is an exception because `HELP` is a keyword
// but this query seems valid at least in mysql and mssql
// so ideally PHPSQLParser would be able to parse it
$this->setExpectedException(
$this->expectException(
'\PHPSQLParser\exceptions\UnableToCalculatePositionException'
);

Expand Down
48 changes: 48 additions & 0 deletions tests/cases/parser/issue365Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php
/**
* issue265.php
*
* Test case for PHPSQLCreator.
*/

namespace PHPSQLParser\Test\Creator;

use PHPSQLParser\PHPSQLParser;
use PHPSQLParser\PHPSQLCreator;

class Issue365Test extends \PHPUnit\Framework\TestCase
{
/*
* https://github.com/greenlion/PHP-SQL-Parser/issues/365
* Data type alias of character broken the CHARACTER SET parsing
*/
public function testIssue365()
{
$sql = "CREATE TABLE IF NOT EXISTS example (`type` CHARACTER (255) CHARACTER SET utf8)";

$parser = new PHPSQLParser($sql);
$parsed = $parser->parsed;
$create_def = $parsed['TABLE']['create-def'];
$sub_tree = $create_def['sub_tree'][0]['sub_tree'][1];

$this->assertEquals('utf8', $sub_tree['charset'], 'CHARACTER SET utf8');
$expected_type = [
'expr_type' => 'data-type',
'base_expr' => 'CHARACTER',
'length' => 255
];
$this->assertEquals($expected_type, $sub_tree['sub_tree'][0], 'CHARACTER data type definition');
}

public function testIssue365BonusCharset()
{
$sql = "CREATE TABLE IF NOT EXISTS example (`type` CHARACTER (255) CHARSET utf8)";

$parser = new PHPSQLParser($sql);
$parsed = $parser->parsed;
$create_def = $parsed['TABLE']['create-def'];
$sub_tree = $create_def['sub_tree'][0]['sub_tree'][1];

$this->assertEquals('utf8', $sub_tree['charset'], 'CHARACTER SET utf8');
}
}

0 comments on commit dec667f

Please sign in to comment.