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.

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

DB update loads schema of none Contao tables #78

Closed
backbone87 opened this issue Dec 19, 2017 · 13 comments
Closed

DB update loads schema of none Contao tables #78

backbone87 opened this issue Dec 19, 2017 · 13 comments
Assignees
Labels
Milestone

Comments

@backbone87
Copy link

$fromSchema = $this->dropNonContaoTables($this->connection->getSchemaManager()->createSchema());

Here the full database schema is loaded first and non-Contao parts are dropped after.
This causes errors, when database specific column types (like MySQL's POINT) are used in non-Contao database assets, that Doctrine doesnt know about:

[2017-12-19 03:09:26] app.CRITICAL: An exception occurred. {"exception":"[object] (Doctrine\\DBAL\\DBALException(code: 0): Unknown database type point requested, Doctrine\\DBAL\\Platforms\\MySqlPlatform may not support it. at /vendor/doctrine/dbal/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php:429)"} []

Doctrine's Schema asset filter option should be used, to prevent loading irrelevant Schema parts:

$config = $this->connection->getConfiguration();
$previousSchemaFilter = $config->getFilterSchemaAssetsExpression();
$config->setFilterSchemaAssetsExpression('@^tl_@');
$fromSchema = $this->connection->getSchemaManager()->createSchema();
$config->setFilterSchemaAssetsExpression($previousSchemaFilter);

Though it would be probably better to use a separate connection configured via doctrine bundle config for the install tool, insteadof "hot"-changing doctrine connection configuration.

@leofeyer leofeyer added this to the 4.5.0 milestone Dec 19, 2017
@leofeyer
Copy link
Member

@contao/developers Any objections?

@aschempp
Copy link
Member

I honestly don't think the problem is in Contao, but in the fact that MySQL specific attributes are used that are not supported by Doctrine. @backbone87 you might want to exclude these tables in the application configuration (via schema filters), which should still work in the install tool

@leofeyer
Copy link
Member

Why are we not using the schema filter ourselves?

@aschempp
Copy link
Member

because it's an application-wide setting that a bundle cannot configure 😂

@leofeyer
Copy link
Member

Managed edition == application, isn't it?

@leofeyer leofeyer modified the milestones: 4.5.0, 4.4.10 Dec 21, 2017
@leofeyer
Copy link
Member

As discussed in Mumble on December 21st, we want to overwrite the schema filter in the install tool.

@leofeyer
Copy link
Member

leofeyer commented Dec 27, 2017

Fixed in bc4067a and contao/core-bundle@4967d3a.

@m-vo
Copy link
Member

m-vo commented Jan 6, 2018

This change leads to the following side effect:

The $toSchema now also contains regular Doctrine Entities (no tl_ prefix) that the install offers to create as these aren't filtered anymore. However these entries are filtered from the $fromSchema which leads to them still beeing in the list (and of course crashing on update because the table already exists).

Imho the installation tool should either handle all entities or we need to implement a solution like #79 and ignore others. So ironically in this use case it would be better to have no filter set at all. :-)

@leofeyer
Copy link
Member

leofeyer commented Jan 6, 2018

I cannot reproduce this. The schema filter is applied to the $toSchema as well:

https://github.com/contao/core-bundle/blob/e4f0d29c314d22ca2ac235ff938027f48aaf985b/src/Doctrine/Schema/DcaSchemaProvider.php#L356-L363

@m-vo
Copy link
Member

m-vo commented Jan 6, 2018

This piece of code only filters the entries from $installer->getFromDca(); which do not include the entities. Later those filtered definitions get appended to the schema that contains the doctrine entities.

@m-vo
Copy link
Member

m-vo commented Jan 6, 2018

To reproduce create a simple entity in a bundle and enable the mapping.

use Doctrine\ORM\Mapping as ORM;

class SomeEntity
{
    /**
     * @ORM\Column(name="id", type="integer")
     * @ORM\Id
     *
     * @var integer $id
     */
    protected $id;
}
doctrine:
    orm:
        entity_managers:
            default:
                mappings:
                    SomeBundle: ~

Clear cache or open the install tool in app_dev.

@leofeyer
Copy link
Member

Fixed in contao/core-bundle@d5dcabd. Needs to be backported to Contao 4.4, too.

@leofeyer leofeyer reopened this Jan 10, 2018
@leofeyer leofeyer modified the milestones: 4.4.10, 4.4.13 Jan 10, 2018
@leofeyer
Copy link
Member

Backported in contao/core-bundle@8109786.

@leofeyer leofeyer modified the milestones: 4.4.13, 4.4 May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants