dustin10 / VichUploaderBundle

A simple Symfony bundle to ease file uploads with ORM entities and ODM documents.
MIT License
1.85k stars 519 forks source link

Lot of useless updates #1348

Closed alx-web closed 1 year ago

alx-web commented 1 year ago
Q A
Bundle version 1.21
Symfony version 5.4.16
PHP version 7.4

Support Question

If i do a simple findAll(); with a flush after on entity like this :

$projects = $this->em->getRepository(Projects::class)->findAll();
[...]
$this->em->flush();

I will have lot of :

doctrine.DEBUG: Executing statement: UPDATE projects SET project_file = ? WHERE id = ? ...

I have this :

        /**
     * If manually uploading a file (i.e. not using Symfony Form) ensure an instance
     * of 'UploadedFile' is injected into this setter to trigger the update. If this
     * bundle's configuration parameter 'inject_on_load' is set to 'true' this setter
     * must be able to accept an instance of 'File' as the bundle will inject one here
     * during Doctrine hydration.
     *
     * @param File|\Symfony\Component\HttpFoundation\File\File $ProjectFile
     */
    public function setProjectFile(?File $ProjectFile = null): void
    {

        $this->projectFile = $ProjectFile;

        if ($this->projectFile instanceof UploadedFile)
        {
            // It is required that at least one field changes if you are using doctrine
            // otherwise the event listeners won't be called and the file is lost
            $this->projectupdatedAt = new \DateTimeImmutable();
        }
    }

    public function getProjectFile(): ?File
    {
        return $this->projectFile;
    }

I can remove when i put inject_on_load to false but i need to have it to true.

How i can remove all this useless updates ?

Thanks

garak commented 1 year ago

Sorry but this is far from being clear... What is projects and what is project? What do you do before flush?

alx-web commented 1 year ago

Sorry for that, i will give you all informations:

1/ Projects is an entity with a list of projects (around 1000) 2/ $projectFile is the mapped field for a file by project 3/ I do nothing before flush, i eliminate all my code to see where come from the problem and if i just do a findAll(); and a flush i have around 1000 UPDATES

I have the same problem on other symfony projects 5.4

garak commented 1 year ago

Flushing makes sense only if do at least one update.

alx-web commented 1 year ago

You're right, but like i said it's just to show you that the problem is here with just a findAll and a flush. I remove all my original code to isolate the problem. I think the inject_on_load do the update and the flush execute it.

I can add the code i want for example this, but the problem still the same:

$projects = $this->em->getRepository(Projects::class)->findAll();
foreach ($projects as $project)
{
  $project->setName("test");
}
$this->em->flush();
garak commented 1 year ago

And so I guess that in this case, where you update the name, the UPDATE query will update the name and the project_file, which is not useless to me.

alx-web commented 1 year ago

On that example i update the name of Projects, but on my real code i have some scripts to updates others entities, and when i flush i have 1000 updates of Projects to update the same values.

I don't understand why there is an UPDATE for update unchanged datas on database ?

For my opinion we should update the database only if a data is updated, and not all the time on load.

Tomorrow if i have 100000 projects, you will do 100000 requests on the database to update the same unchanged value ?

I just try to understand why an UPDATE is done on field with unchanged value.

Thanks

garak commented 1 year ago

If you want help with some real code, please show the real code.

alx-web commented 1 year ago

Hi,

The problem is still here. When i set a value on an entity, the field file of vich uploader is

/** 
* @Route("/test", name="test")
*/
public function test(Request $request, EntityManagerInterface $entityManager)
{
    $projects = $entityManager->getRepository(Projects::class)->findAll();
    foreach ($projects as $project)
    {
        //Whatever
    }
    $entityManager->flush();
}

I will have this for each line, but i don't ask my script to update something. And even if i ask to update a field i don't ask for update the files fields from vich uploader. Moreover, an update to keep the same value.

UPDATE projects SET project_file = ? WHERE id = ?

I'm just trying to show you that your bundle do a lot of useless update with the existing values. If you think it's required, could you explain why ?

Thanks

garak commented 1 year ago

I'm just trying to show you that your bundle do a lot of useless update with the existing values. If you think it's required, could you explain why ?

I appreciate your effort. Could you provide a basic project in which your problem can be reproduced?

alx-web commented 1 year ago

Hi,

I did it : https://github.com/alx-web/test_vich

You can install the project and launch the fixture to have test data.

I have 5 times this update , the code doing 1 update per line on project table with the same data

UPDATE projects SET image_file = ? WHERE id = ?

But, i thing the problem come from "@ORM\Column(nullable=true)" on the field private $imageFile; on Projects.php Entity.

When i remove it i think the problem not here, but i need the field as nullable.

Could you launch the project and check this ?

Thanks

`

garak commented 1 year ago

I'm speechless... Schermata del 2023-03-13 17-29-33

alx-web commented 1 year ago

This is what i explained on my comment ! I'm happy to see that the code writted by you is clear for you ;) I recognize i didn't read this note in detail, this is why i was looking for other cause more complex. A small misleading is better than a big bug no ? ;) Anyway thanks for your help !