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

[Improvement]: Prevent db errors on index updates on classes rebuild #13195

Closed
lukadschaak opened this issue Sep 19, 2022 · 3 comments · Fixed by #16871
Closed

[Improvement]: Prevent db errors on index updates on classes rebuild #13195

lukadschaak opened this issue Sep 19, 2022 · 3 comments · Fixed by #16871

Comments

@lukadschaak
Copy link
Contributor

Improvement description

When rebuilding classes via bin/console pimcore:deploy:classes-rebuild, the indexes of all class definitions are updated in this process. Pimcore uses the trait \Pimcore\Model\DataObject\ClassDefinition\Helper\Dao to do this.

The trait uses ALTER TABLE ... ADD INDEX ... and ALTER TABLE ... DROP INDEX ... for the actual operations. The SQL queries throws errors when adding a index, that already exists and dropping an index that doesn't exist. There is a Helper that ignores these errors.

MariaDB has nice additional options for these type of queries: ADD INDEX IF NOT EXISTS and DROP INDEX IF EXISTS, but because Pimcore is supporting MySQL as well, this is not an option in the first place.

My question is, if there is any interest in fixing that problem by using general implementations that ask for existence of indexes like
select count(*) from information_schema.statistics where table_name = ... and index_name = ... ?

@brusch
Copy link
Member

brusch commented Sep 19, 2022

I think it should be also possible to use Doctrine's schema manager for that, just as we do it e.g. in our migrations:

if (!$versionsTable->hasIndex('autoSave')) {

@lukadschaak
Copy link
Contributor Author

Nice mention!

With Doctrine as feature, but not so much experience, i tried some things and this is what i found out.

Create a schmema from inside the code is a heavy lifting for the database in term of number of requests. Following works, but it would be necessary to cache that schema over the whole classes rebuild process.

$schema = $this->db->getSchemaManager()->createSchema();
$table = $schema->getTable($table);
$indexExists = $table->hasIndex($indexName);

Another solution is to use the SchemaManager directly like this. In this approach, the $tableName must be lowercase.

$indexes = $this->db->getSchemaManager()->listTableIndexes($table);
$indexExists = \in_array(strtolower($tableName), \array_keys($indexes))

This results in something like the following, which seems okay for me. It would replace the ADD INDEX/DROP INDEX requests by this SELECT requests (when nothing changes).

SELECT NON_UNIQUE AS Non_Unique, INDEX_NAME AS Key_name, COLUMN_NAME AS Column_Name, SUB_PART AS Sub_Part, INDEX_TYPE AS Index_Type FROM information_schema.STATISTICS WHERE TABLE_NAME = '...' AND TABLE_SCHEMA = 'pimcore' ORDER BY SEQ_IN_INDEX ASC

Both work. The second approach is less invasive to the code. Any suggestions?

@github-actions
Copy link

Thanks a lot for reporting the issue. We did not consider the issue as "Priority" or "Backlog", so we're not going to work on that anytime soon. Please create a pull request to fix the issue if this is a bug report. We'll then review it as quickly as possible. If you're interested in contributing a feature, please contact us first here before creating a pull request. We'll then decide whether we'd accept it or not. Thanks for your understanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants