Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Issue #301 Fix MySQL quote issue for table options #302

Closed
wants to merge 1 commit into from

2 participants

@rncain

Fixed quoting MySQL vendor params on tables which caused syntax error problems

Issue #301

@willdurand willdurand commented on the diff
generator/lib/platform/MysqlPlatform.php
@@ -235,17 +235,26 @@ protected function getTableOptions(Table $table)
'Union' => 'UNION',
);
foreach ($supportedOptions as $name => $sqlName) {
+
+ $parameterValue = NULL;
@willdurand Owner

it seems the indentation is wrong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@willdurand willdurand commented on the diff
generator/lib/platform/MysqlPlatform.php
((19 lines not shown))
}
+
+ //if we have a param value, then parse it out
@willdurand Owner

useless comment sir

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@willdurand willdurand commented on the diff
generator/lib/platform/MysqlPlatform.php
((19 lines not shown))
}
+
+ //if we have a param value, then parse it out
+ if( !is_null( $parameterValue ) ){
@willdurand Owner

bad CS, should be:

if (!is_null($parameterValue)) {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@willdurand willdurand commented on the diff
generator/lib/platform/MysqlPlatform.php
((19 lines not shown))
}
+
+ //if we have a param value, then parse it out
+ if( !is_null( $parameterValue ) ){
+
+ //if the value is numeric, then there is no need for quotes
+ $parameterValue = is_numeric( $parameterValue ) ? $parameterValue : $this->quote( $parameterValue );
@willdurand Owner

Gosh.. avoid spaces after parenthesis.

is_numeric($parameterValue) ? $parameterValue : $this->quote($parameterValue);
@rncain
rncain added a note

Do you have a coding standards doc for your project?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@willdurand willdurand commented on the diff
generator/lib/platform/MysqlPlatform.php
((19 lines not shown))
}
+
+ //if we have a param value, then parse it out
+ if( !is_null( $parameterValue ) ){
+
+ //if the value is numeric, then there is no need for quotes
+ $parameterValue = is_numeric( $parameterValue ) ? $parameterValue : $this->quote( $parameterValue );
+
+ $tableOptions []= sprintf( '%s=%s', $sqlName, $parameterValue );
@willdurand Owner

same here

and:

$tableOptions[] =
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@willdurand
Owner

Nice fix, please fix your code, and I'll be glad to merge your PR :)

@willdurand
Owner

Ping @rncain

@willdurand
Owner

Merged, thanks. Can you port this patch on Propel2?

@willdurand willdurand closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 25, 2012
  1. @rncain
This page is out of date. Refresh to see the latest.
View
25 generator/lib/platform/MysqlPlatform.php
@@ -235,17 +235,26 @@ protected function getTableOptions(Table $table)
'Union' => 'UNION',
);
foreach ($supportedOptions as $name => $sqlName) {
+
+ $parameterValue = NULL;
@willdurand Owner

it seems the indentation is wrong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
if ($vi->hasParameter($name)) {
- $tableOptions []= sprintf('%s=%s',
- $sqlName,
- $this->quote($vi->getParameter($name))
- );
+ $parameterValue = $vi->getParameter( $name );
} elseif ($vi->hasParameter($sqlName)) {
- $tableOptions []= sprintf('%s=%s',
- $sqlName,
- $this->quote($vi->getParameter($sqlName))
- );
+ $parameterValue = $vi->getParameter( $sqlName );
}
+
+ //if we have a param value, then parse it out
@willdurand Owner

useless comment sir

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if( !is_null( $parameterValue ) ){
@willdurand Owner

bad CS, should be:

if (!is_null($parameterValue)) {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ //if the value is numeric, then there is no need for quotes
+ $parameterValue = is_numeric( $parameterValue ) ? $parameterValue : $this->quote( $parameterValue );
@willdurand Owner

Gosh.. avoid spaces after parenthesis.

is_numeric($parameterValue) ? $parameterValue : $this->quote($parameterValue);
@rncain
rncain added a note

Do you have a coding standards doc for your project?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ $tableOptions []= sprintf( '%s=%s', $sqlName, $parameterValue );
@willdurand Owner

same here

and:

$tableOptions[] =
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ }
+
+
}
return $tableOptions;
}
View
3  test/testsuite/generator/platform/MysqlPlatformTest.php
@@ -363,6 +363,7 @@ public function testGetAddTableDDLVendor()
<vendor type="mysql">
<parameter name="Engine" value="InnoDB"/>
<parameter name="Charset" value="utf8"/>
+ <parameter name="AutoIncrement" value="1000"/>
</vendor>
</table>
</database>
@@ -373,7 +374,7 @@ public function testGetAddTableDDLVendor()
(
`id` INTEGER NOT NULL AUTO_INCREMENT,
PRIMARY KEY (`id`)
-) ENGINE=InnoDB CHARACTER SET='utf8';
+) ENGINE=InnoDB AUTO_INCREMENT=1000 CHARACTER SET='utf8';
";
$this->assertEquals($expected, $this->getPlatform()->getAddTableDDL($table));
}
Something went wrong with that request. Please try again.