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.

Sign up for GitHub

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

Bugfix/issue1489 mysql types #1986

Closed
wants to merge 5 commits into from
Closed

Bugfix/issue1489 mysql types #1986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d47c67f
Fix MySQL column type retrieval
mringler Nov 25, 2023
d2bc7b8
lint fixes
mringler Nov 27, 2023
8cd7c43
fix test coverage
mringler Nov 28, 2023
4a2032b
Merge branch 'master' of github.com:propelorm/Propel2
mringler Apr 18, 2024
a510dea
Merge branch 'master' into bugfix/issue1489-mysql_types
mringler Apr 18, 2024
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
175 changes: 120 additions & 55 deletions src/Propel/Generator/Reverse/MysqlSchemaParser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Propel\Generator\Model\Column;
use Propel\Generator\Model\ColumnDefaultValue;
use Propel\Generator\Model\Database;
use Propel\Generator\Model\Domain;
use Propel\Generator\Model\ForeignKey;
use Propel\Generator\Model\Index;
use Propel\Generator\Model\PropelTypes;
Expand Down Expand Up @@ -62,10 +63,10 @@ class MysqlSchemaParser extends AbstractSchemaParser
'blob' => PropelTypes::BLOB,
'mediumblob' => PropelTypes::VARBINARY,
'longblob' => PropelTypes::LONGVARBINARY,
'longtext' => PropelTypes::CLOB,
'tinytext' => PropelTypes::VARCHAR,
'mediumtext' => PropelTypes::LONGVARCHAR,
'text' => PropelTypes::LONGVARCHAR,
'mediumtext' => PropelTypes::LONGVARCHAR,
'longtext' => PropelTypes::CLOB,
'enum' => PropelTypes::CHAR,
'set' => PropelTypes::CHAR,
'binary' => PropelTypes::BINARY,
Expand Down Expand Up @@ -220,9 +221,84 @@ protected function addColumns(Table $table): void
*/
public function getColumnFromRow(array $row, Table $table): Column
{
$name = $row['Field'];
$isNullable = ($row['Null'] === 'YES');
$autoincrement = (strpos($row['Extra'], 'auto_increment') !== false);
[
'Field' => $columnName,
'Type' => $type,
'Null' => $null,
// 'Key' => $key,
'Default' => $default,
'Extra' => $extra,
] = $row;

$column = new Column($columnName);
$column->setTable($table);

$domain = $this->extractTypeDomain($type, $default, $column->getFullyQualifiedName(), $extra);
$column->setDomain($domain);

$autoincrement = (strpos($extra, 'auto_increment') !== false);
$column->setAutoIncrement($autoincrement);

$column->setNotNull($null === 'NO');

if ($this->addVendorInfo) {
$vi = $this->getNewVendorInfoObject($row);
$column->addVendorInfo($vi);
}

return $column;
}

/**
* @param string $typeDeclaration MySQL type declaration string (like "VARCHAR(16) CHARACTER SET utf8mb4", "INT UNSIGNED", etc)
* @param string|null $defaultValueLiteral Default value declaration
* @param string $columnName Used when printing warninga
* @param string $extra Additional type specification (i.e. UNSIGNED)
*
* @return \Propel\Generator\Model\Domain
*/
protected function extractTypeDomain(string $typeDeclaration, ?string $defaultValueLiteral, string $columnName, string $extra): Domain
{
[$nativeType, $sqlType, $size, $scale] = $this->parseType($typeDeclaration);

$propelType = $this->getMappedPropelType($nativeType);
if (!$propelType) {
$propelType = Column::DEFAULT_TYPE;
$sqlType = $typeDeclaration;
$this->warn("Column [{$columnName}] has a column type ({$nativeType}) that Propel does not support.");
}

// Special case for TINYINT(1) which is a BOOLEAN
if ($propelType === PropelTypes::TINYINT && $size === 1) {
$propelType = PropelTypes::BOOLEAN;
}

$domain = clone $this->getPlatform()->getDomainForType($propelType);
if ($sqlType) {
$domain->replaceSqlType($sqlType);
} elseif (in_array(strtoupper($nativeType), ['TINYTEXT', 'MEDIUMTEXT', 'TINYBLOB'], true)) {
$domain->replaceSqlType(strtoupper($nativeType));
}
$domain->replaceSize($size);
$domain->replaceScale($scale);

$defaultValue = $this->extractDefaultValue($defaultValueLiteral, $propelType, $nativeType, $extra);
if ($defaultValue) {
$domain->setDefaultValue($defaultValue);
}

return $domain;
}

/**
* Parse values from a MySQL type declaration string.
*
* @param string $typeDeclaration MySQL type declaration string (like "VARCHAR(16) CHARACTER SET utf8mb4", "INT UNSIGNED", etc)
*
* @return list{string, string|false, ?int, ?int} Array with the extracted values (type name, type declaration with parameters or false, size, precision)
*/
protected function parseType(string $typeDeclaration): array
{
$size = null;
$scale = null;
$sqlType = false;
Expand All @@ -233,9 +309,9 @@ public function getColumnFromRow(array $row, Table $table): Column
?([\d,]*) # size or size, precision [2]
[\)] # )
?\s* # whitespace
(\w*) # extra description (UNSIGNED, CHARACTER SET, ...) [3]
([\w ]*) # extra description (UNSIGNED, CHARACTER SET, ...) [3]
$/x';
if (preg_match($regexp, $row['Type'], $matches)) {
if (preg_match($regexp, $typeDeclaration, $matches)) {
$nativeType = $matches[1];
if ($matches[2]) {
$cpos = strpos($matches[2], ',');
Expand All @@ -247,72 +323,61 @@ public function getColumnFromRow(array $row, Table $table): Column
}
}
if ($matches[3]) {
$sqlType = $row['Type'];
$sqlType = $typeDeclaration;
}
if (isset(static::$defaultTypeSizes[$nativeType]) && $scale == null && $size === static::$defaultTypeSizes[$nativeType]) {
$size = null;
}
} elseif (preg_match('/^(\w+)\(/', $row['Type'], $matches)) {
} elseif (preg_match('/^(\w+)\(/', $typeDeclaration, $matches)) {
$nativeType = $matches[1];
if ($nativeType === 'enum' || $nativeType === 'set') {
$sqlType = $row['Type'];
$sqlType = $typeDeclaration;
}
} else {
$nativeType = $row['Type'];
$nativeType = $typeDeclaration;
}

// BLOBs can't have any default values in MySQL
$default = preg_match('~blob|text~', $nativeType) ? null : $row['Default'];
return [$nativeType, $sqlType, $size, $scale];
}

$propelType = $this->getMappedPropelType($nativeType);
if (!$propelType) {
$propelType = Column::DEFAULT_TYPE;
$sqlType = $row['Type'];
$this->warn('Column [' . $table->getName() . '.' . $name . '] has a column type (' . $nativeType . ') that Propel does not support.');
}
/**
* @param string|null $parsedValue Default value declaration
* @param string $propelType Column type indicator from \Propel\Generator\Model\PropelTypes
* @param string $nativeType MySQL type name
* @param string $extra Additional type specification (i.e. UNSIGNED)
*
* @return \Propel\Generator\Model\ColumnDefaultValue|null
*/
protected function extractDefaultValue(?string $parsedValue, string $propelType, string $nativeType, string $extra): ?ColumnDefaultValue
{
// BLOBs can't have any default values in MySQL
$isBlob = preg_match('~blob|text~', $nativeType);

// Special case for TINYINT(1) which is a BOOLEAN
if ($propelType === PropelTypes::TINYINT && $size === 1) {
$propelType = PropelTypes::BOOLEAN;
if ($parsedValue === null || $isBlob) {
return null;
}
$default = $parsedValue;

$column = new Column($name);
$column->setTable($table);
$column->setDomainForType($propelType);
if ($sqlType) {
$column->getDomain()->replaceSqlType($sqlType);
}
$column->getDomain()->replaceSize($size);
$column->getDomain()->replaceScale($scale);
if ($default !== null) {
if ($propelType == PropelTypes::BOOLEAN) {
if ($default == '1') {
$default = 'true';
}
if ($default == '0') {
$default = 'false';
}
if ($propelType == PropelTypes::BOOLEAN) {
if ($parsedValue == '1') {
$default = 'true';
}
if (in_array($default, ['CURRENT_TIMESTAMP', 'current_timestamp()'], true)) {
$default = 'CURRENT_TIMESTAMP';
if (strpos(strtolower($row['Extra']), 'on update current_timestamp') !== false) {
$default = 'CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP';
}
$type = ColumnDefaultValue::TYPE_EXPR;
} else {
$type = ColumnDefaultValue::TYPE_VALUE;
if ($parsedValue == '0') {
$default = 'false';
}
$column->getDomain()->setDefaultValue(new ColumnDefaultValue($default, $type));
}
$column->setAutoIncrement($autoincrement);
$column->setNotNull(!$isNullable);

if ($this->addVendorInfo) {
$vi = $this->getNewVendorInfoObject($row);
$column->addVendorInfo($vi);
if (in_array($default, ['CURRENT_TIMESTAMP', 'current_timestamp()'], true)) {
$default = 'CURRENT_TIMESTAMP';
if (strpos(strtolower($extra), 'on update current_timestamp') !== false) {
$default = 'CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP';
}
$type = ColumnDefaultValue::TYPE_EXPR;
} else {
$type = ColumnDefaultValue::TYPE_VALUE;
}

return $column;
return new ColumnDefaultValue($default, $type);
}

/**
Expand Down Expand Up @@ -359,7 +424,7 @@ protected function addColumnDescriptionsToTable(Table $table): void
protected function loadTableDescription(Table $table): ?string
{
$tableName = $this->getPlatform()->quote($table->getName());
$query = <<< EOT
$query = <<<EOT
SELECT table_comment
FROM INFORMATION_SCHEMA.TABLES
WHERE table_schema=DATABASE()
Expand Down Expand Up @@ -388,7 +453,7 @@ protected function loadColumnDescription(Column $column): ?string
{
$tableName = $this->getPlatform()->quote($column->getTableName());
$columnName = $this->getPlatform()->quote($column->getName());
$query = <<< EOT
$query = <<<EOT
SELECT column_comment
FROM INFORMATION_SCHEMA.COLUMNS
WHERE table_schema=DATABASE()
Expand Down
30 changes: 22 additions & 8 deletions tests/Propel/Tests/Generator/Reverse/AbstractSchemaParserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@
*/
abstract class AbstractSchemaParserTest extends BookstoreTestBase
{
/*
* HACK: tests were written using instance properties for parser and
* parsedDatabase, leading to re-initialization on every test.
* Using static properties instead fixes the issue, but I don't want
* to update every test, so the static objects will be copied.
*/


/**
* @var \Propel\Generator\Model\Database
*/
protected static $parserDatabaseInstance;

/**
* @var \Propel\Generator\Reverse\SchemaParserInterface
*/
Expand Down Expand Up @@ -54,17 +67,12 @@ abstract protected function getDriverName(): string;
*/
protected function init(): void
{
$parserClass = $this->getSchemaParserClass();
$parser = new $parserClass($this->con);
$parser->setGeneratorConfig(new QuickGeneratorConfig());

$database = new Database();
$database->setPlatform(new DefaultPlatform());

$parser->parse($database);
$this->parser->parse($database);

$this->parser = $parser;
$this->parsedDatabase = $database;
static::$parserDatabaseInstance = $database;
}

/**
Expand All @@ -80,8 +88,14 @@ protected function setUp(): void
$this->markTestSkipped("This test is designed for $expectedDriverName and cannot be run with $currentDriverName");
}

if ($this->parser === null) {
$parserClass = $this->getSchemaParserClass();
$this->parser = new $parserClass($this->con);
$this->parser->setGeneratorConfig(new QuickGeneratorConfig());

if (!static::$parserDatabaseInstance) {
$this->init();
}

$this->parsedDatabase = static::$parserDatabaseInstance;
}
}
59 changes: 58 additions & 1 deletion tests/Propel/Tests/Generator/Reverse/MysqlSchemaParserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
namespace Propel\Tests\Generator\Reverse;

use PDO;
use Propel\Generator\Model\Column;
use Propel\Generator\Model\Table;
use Propel\Generator\Model\ColumnDefaultValue;
use Propel\Generator\Reverse\MysqlSchemaParser;
use Propel\Tests\Bookstore\Map\BookTableMap;
Expand Down Expand Up @@ -44,7 +46,7 @@ protected function getSchemaParserClass(): string
*/
public function testParseImportsAllTables(): void
{
$query = <<< EOT
$query = <<<EOT
SELECT table_name
FROM INFORMATION_SCHEMA.TABLES
WHERE table_schema=DATABASE()
Expand Down Expand Up @@ -83,6 +85,61 @@ public function testDescriptionsAreImported(): void
$this->assertEquals('Book Title', $bookTable->getColumn('title')->getDescription());
}

public function typeLiterals()
{
return [
// input type literal, out native type, out sql, out size, out precision
['int', 'int', false, null, null],
["set('foo', 'bar')", 'set', "set('foo', 'bar')", null, null],
["enum('foo', 'bar')", 'enum', "enum('foo', 'bar')", null, null],
["unknown('foo', 'bar')", 'unknown', false, null, null],
['varchar(16)', 'varchar', false, 16, null],
['varchar(16) CHARACTER SET utf8mb4', 'varchar', 'varchar(16) CHARACTER SET utf8mb4', 16, null],
['decimal(6,4)', 'decimal', false, 6, 4],
['char(1)', 'char', false, null, null], // default type size
['(nonsense)', '(nonsense)', false, null, null],
];
}

/**
* @dataProvider typeLiterals
*/
public function testParseType(
string $inputType,
...$expectedOutput)
{
$output = $this->callMethod($this->parser, 'parseType', [$inputType]);
$this->assertEquals($expectedOutput, $output);
}

public function testInvalidTypeBehavior(){
$args = ['(nonsense)', null, 'leTable.leColumn', ''];
/** @var \Propel\Generator\Model\Domain */
$domain = $this->callMethod($this->parser, 'extractTypeDomain', $args);
$this->assertSame(Column::DEFAULT_TYPE, $domain->getType());
$this->assertSame($args[0], $domain->getSqlType());
}

public function testAddVendorInfo(){
$row = [
'Field' => 'le_col',
'Type' => 'int',
'Null' => 'NO',
'Key' => null,
'Default' => null,
'Extra' => '',
];
$table = new Table('le_table');
$args = [$row, $table];

$parserClass = $this->getSchemaParserClass();
$parser = new $parserClass($this->con);
$this->setProperty($parser, 'addVendorInfo', true);
/** @var \Propel\Generator\Model\Column */
$column = $this->callMethod($parser, 'getColumnFromRow', $args);
$this->assertNotEmpty($column->getVendorInformation());
}

/**
* @return void
*/
Expand Down
Loading
Loading