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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize saving relational fields #16210

Conversation

BlackbitDevs
Copy link
Contributor

@BlackbitDevs BlackbitDevs commented Nov 7, 2023

Changes in this pull request

Every time when relational field is changed Pimcore always delete all relations related to the field

$dataExists = $this->db->fetchOne('SELECT `src_id` FROM `object_relations_'. $this->model->getClassId().'`
WHERE '.$condition .' LIMIT 1');
if ($dataExists) {
$this->db->executeStatement('DELETE FROM object_relations_' . $this->model->getClassId() . ' WHERE ' . $condition);
}

If relational field have a lot of relations this behavior may take time to firstly delete all relations and then insert it again.

This PR changed standard behavior and relations will be deleted only if it's really been deleted but not all relations. And only new relations will be inserted.

Additional info

WHAT

馃 Generated by Copilot at 76a0928

This pull request makes several formatting, readability, and performance changes to various PHP files in the pimcore/pimcore repository. It adds empty lines to the end of some files and removes redundant comment lines from others to comply with the PSR-12 coding standard and improve code quality. It also improves the logic for deleting relational objects from the database when updating a data object to avoid unnecessary queries and errors.

馃 Generated by Copilot at 76a0928

Sing, O Muse, of the code review that brought many changes
To the files of Pimcore, the framework of web development
How the diligent coder, following the PSR-12 standard
Added empty lines to the end of each PHP file

HOW

馃 Generated by Copilot at 76a0928

  • Add empty lines to the end of several PHP files to follow the PSR-12 coding standard (link, link, link, link, link, link, link, link, link, link, link, link)
  • Remove unnecessary comment lines from generateCodes in Pattern and writeSaveQueue in CoreCacheHandler to improve code readability and quality (link, link)
  • Remove misplaced comment line from Service to avoid confusion (link)
  • Add condition to save in AbstractRelations to avoid unnecessary database queries and errors when saving empty relations (link)
  • Add logic to update in Dao to detect and delete relational objects that are no longer associated with the object (link, link, link)

Copy link

github-actions bot commented Nov 7, 2023

Review Checklist

  • Target branch (11.0 for bug fixes, others 11.x)
  • Tests (if it's testable code, there should be a test for it - get help)
  • Docs (every functionality needs to be documented, see here)
  • Migration incl. install.sql (e.g. if the database schema changes, ...)
  • Upgrade notes (deprecations, important information, migration hints, ...)
  • Label
  • Milestone

Copy link

sonarcloud bot commented Nov 7, 2023

Kudos, SonarCloud Quality Gate passed!聽 聽 Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Comment on lines +113 to +115
$db = Db::get();
$condition = 'src_id = ' . $db->quote($object->getId()) . ' AND ownertype = "object"';
$dataExists = $db->fetchCol('SELECT `dest_id` FROM `object_relations_' . $object->getClassName() . '` WHERE ' . $condition);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why fetch the data before the condition if it's only used inside it?

@BlackbitDevs BlackbitDevs changed the base branch from 10.6 to 11.x November 9, 2023 15:35
@BlackbitDevs BlackbitDevs deleted the optimize-saving-relational-fields branch November 12, 2023 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants