Propel2 icon indicating copy to clipboard operation
Propel2 copied to clipboard

fix for broken alter-table statements when default column values cont…

Open hevengo opened this issue 5 years ago • 5 comments

The problem

When a column defaultValue contains a semicolon (;) the generated alter-table statement is broken.

<column name="attachement_allowed_extensions" phpName="AttachementAllowedExtensions" 
type="VARCHAR" size="250" required="true" defaultValue="pdf;jpg;png;doc;docx;xls;xlsx;txt"/>

the reason for that can be found in file /src/Propel/Generator/Platform/DefaultPlatform.php line 818ff

if ($columnChanges) {
    //merge column changes into one command. This is more compatible especially with PK constraints.

    $changes = explode(';', $columnChanges);

the already generated sql string consists of multiple alter table statements and shall be split into single alter table statements to merge. If the generated statement

ALTER TABLE `newsletter_sets`  CHANGE `attachement_allowed_extensions` `attachement_allowed_extensions` VARCHAR(250) DEFAULT 'pdf;jpg;png;doc;docx;xls;xlsx;txt' NOT NULL;

contains multiple semicolons so the splitting goes wrong.

The solution

propel already contains a sql statement parser that can be used as a drop-in replacement here.

$sqlParser = new SqlParser();
$sqlParser->setSQL($columnChanges);
$changes = $sqlParser->explodeIntoStatements();

hevengo avatar Jun 22 '20 13:06 hevengo

We now have master almost green ( https://travis-ci.org/github/propelorm/Propel2 ) Please try to rebase yours against that one, that will make it easier to verify your changes. Do you happen to have a test case along with your change request?

dereuromark avatar Jun 22 '20 15:06 dereuromark

i rebased it against the current master - i will try to build a testcase

hevengo avatar Jun 22 '20 16:06 hevengo

i see that some testCases are failing now due to some different newlines. What would you recommend me to do?

1) Propel\Tests\Generator\Platform\OraclePlatformMigrationTest::testGetModifyDatabaseDDL with data set #0 (Propel\Generator\Model\Diff\DatabaseDiff Object (...))
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 \n
 CREATE SEQUENCE foo5_SEQ\n
     INCREMENT BY 1 START WITH 1 NOMAXVALUE NOCYCLE NOCACHE ORDER;\n
-\n
 ALTER TABLE foo2 RENAME COLUMN bar TO bar1;\n
 \n
 ALTER TABLE foo2\n
/home/travis/build/propelorm/Propel2/tests/Propel/Tests/Generator/Platform/OraclePlatformMigrationTest.php:73

https://travis-ci.org/github/propelorm/Propel2/jobs/700959913

hevengo avatar Jun 22 '20 17:06 hevengo

Before, if (!trim($change)) continue; took care of empty rows You might want to check what your changed method returns here instead.

dereuromark avatar Jun 22 '20 17:06 dereuromark

ping @hevengo

dereuromark avatar Apr 06 '22 15:04 dereuromark

Merged in https://github.com/propelorm/Propel2/pull/1997

gechetspr avatar Apr 01 '24 08:04 gechetspr