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

Deprecate EntityManager::flush with $entity argument #8459

Closed
beberlei opened this issue Feb 6, 2021 · 21 comments
Closed

Deprecate EntityManager::flush with $entity argument #8459

beberlei opened this issue Feb 6, 2021 · 21 comments
Milestone

Comments

@beberlei
Copy link
Member

beberlei commented Feb 6, 2021

The method EntityManager::flush has an optional argument $entity (either accepting a single entity or array of entities) that when passed are the only set of entities considered in compute changeset operation. The idea originally was to allow more performant flush operations when a lot of entities are in the identity map.

However due to restrictions in how the UnitOfWork operates, it had to also "flush" other entities sometimes that were not passed to flush, making the operation inconsistent.

As such we decided to deprecate this approach to flushing and only allow a full flush for now.

Alternative: If you want to keep the set of entities small that flush operates on, you could decide to use the explicit change tracking policy for an entity instead of the implicit one. https://www.doctrine-project.org/projects/doctrine-orm/en/2.8/reference/change-tracking-policies.html

@miedzikd
Copy link

miedzikd commented Sep 6, 2021

Hello @beberlei :)

Can you give me another tip, how can I solve my issue when flushing objects as parameters, will it be deprecated?

I have one huge table with Orders. It's used in a lot of places in the system.

Normally we use ->flush() without passing Order object as a parameter, but I have one command job that regenerates all Orders. Im doing that in batch for 1000 orders per execution.

If I pass Order object to flush - whole execution take more or less 60s. Without that 1200s, because size of unitOfWork is too huge.

Explicit change tracking policy can be apply on whole entity level only? I can't change this because it is used in too many places. Can I apply the policy only in one action? or one command?

Greetings from Poland :)

@wadjeroudi
Copy link

@beberlei @Ocramius any news on this deprecation ? We should be able to do a specific data change at least in a new transaction without flushing the whole changes.
Maybe some sort of temporary clear() on entityManager and restore the unit of work after :

$em->saveStateAndClear() // temporary reset
$em->persist($entity)
$em->flush() // do not auto restore state in case multiple flushes are needed
$em->restorePreviousState()

I'm probably wrong but adding a property on EntityManager $savedUnitOfWork to store the current uof, instantiate a new empty one seems pretty simple imho :

$this->unitOfWork        = new UnitOfWork($this);

@poolerMF
Copy link

@beberlei @Ocramius any news on this deprecation ? We should be able to do a specific data change at least in a new transaction without flushing the whole changes. Maybe some sort of temporary clear() on entityManager and restore the unit of work after :

$em->saveStateAndClear() // temporary reset
$em->persist($entity)
$em->flush() // do not auto restore state in case multiple flushes are needed
$em->restorePreviousState()

I'm probably wrong but adding a property on EntityManager $savedUnitOfWork to store the current uof, instantiate a new empty one seems pretty simple imho :

$this->unitOfWork        = new UnitOfWork($this);

+1

@Fedik
Copy link
Contributor

Fedik commented Dec 23, 2021

We should be able to do a specific data change at least in a new transaction without flushing the whole changes.

It already possible, if I right understood the linked doc.
We have to define DEFERRED_EXPLICIT for ChangeTrackingPolicy.
Then flush() operation will take in count only an entities that was explicitly marked to "persist" with $em->persist($entity).

Old:

$em->persist($entity);
$em->flush($entity);

New:
Set mapping to DEFERRED_EXPLICIT, and:

$em->persist($entity);
$em->flush();

@GDXbsv
Copy link

GDXbsv commented Dec 23, 2021

@Fedik I see a problem with it:
If you not set DEFERRED_EXPLICIT as default and only one possible value for all entities, then if some other entity was changed in a different place without DEFERRED_EXPLICIT then this entity will be saved as well. So we still can not prevent flushing unexpected entities.

@Fedik
Copy link
Contributor

Fedik commented Dec 23, 2021

That correct, and that why you have to be sure all your Entities is set to DEFERRED_EXPLICIT(or NOTIFY) and that you call $em->persist($entity) when changes need to be written.
Unless your App requires different. I think.

@aniolekx
Copy link

Is it possible to set ChangeTrackingPolicy per script execution? I am happy enough that whole app is set to Deferred Implicit, but for data upload from excel files I need Deferred Explicit, removing of that option will add me a lot of work (I will have to check whole app where we have missing persist)

@derrabus
Copy link
Member

Fixed via #9485.

@kgasienica
Copy link

kgasienica commented Aug 9, 2022

There were any discussion about that? This change makes many scripts in batches last way much longer. And what is even worse, since this change you can no longer be sure which things are flushed.

If I have to be honest, I don't really like this change.

@beberlei
Copy link
Member Author

beberlei commented Aug 9, 2022

@kgasienica its deprecated yes, but we also plan to introduce an alternative that is better.

Just keep using the deprecated way until then

@kgasienica
Copy link

kgasienica commented Aug 9, 2022

Okay, thank you. It is really breaking-change thing that is changing the way entities have to be managed in our applications.

@wadjeroudi
Copy link

@beberlei any information on the new alternative please ?

@wadjeroudi
Copy link

@beberlei @greg0ire it seems that the alternative was this one but @Ocramius won't work on it anymore by lack of time I guess.
#5550 (comment)

Still same approach ? or an other is considered ?

@phpws
Copy link

phpws commented Sep 26, 2023

there are probably cases that I can't think of where this wouldn't work, but thought to share it incase it could be useful.

If the existing flush method was renamed to something like flushAll and made private and passing in an entity was removed from the method, then the flush method was replaced with this, then it seems like this could work, at least for my case where I am using it does, but maybe you will know reasons as to why it can't be done like this, maybe it would still be inconsistant in some cases.

    public function flush(?object $object = null): void
    {
        if ($object === null) {
            $this->flushAll();
            return;
        }

        $class = \get_class($object);

        $unitOfWork = $this->getUnitOfWork();
        $scheduledEntities = $unitOfWork->getScheduledEntityInsertions();

        foreach ($scheduledEntities as $scheduledEntity) {
            if (\get_class($scheduledEntity) != $class) {
                $unitOfWork->remove($scheduledEntity);
            }
        }

        $this->flushAll();

        foreach ($scheduledEntities as $scheduledEntity) {
            if (\get_class($scheduledEntity) != $class) {
                $unitOfWork->persist($scheduledEntity);
            }
        }
    }

@beberlei
Copy link
Member Author

@phpws Please don't do that :-)

@phpws
Copy link

phpws commented Sep 29, 2023

@beberlei ok, I didn't in the end, don't worry :-D, I take it that would have adverse affects on other things, performance for one would probably be a problem on large projects with huge sets of scheduledEntites collections, also it doesn't account for updates.

@wadjeroudi
Copy link

@beberlei @greg0ire you released the doctrine 3.0 RC1, congrats, what about this BC BREAK
https://github.com/doctrine/orm/blob/3.0.x/UPGRADE.md#bc-break-removed-ability-to-partially-flushcommit-entity-manager-and-unit-of-work

there's still no alternative ?

@greg0ire
Copy link
Member

@wadjeroudi I'm told that using detach() (it's been undeprecated) would be an alternative for you, as partial flushing should be equivalent to detaching things you don't want to flush and then calling flush()

@wokenlex
Copy link

wokenlex commented Apr 5, 2024

So, the real way to work with it normally, is to make handwritten queries, like in old times?

$stmt = $this->em->getConnection()->prepare('');
$stmt->executeStatement();

I mean, sometimes you need to do something under the hood - write do database log, cache some items, but you don't wan't to ruin the main business logic.

@CsabaNa
Copy link

CsabaNa commented Jun 12, 2024

So how can we insert/flush only one Entity e.g. I need a record ID in multiple places but based on this record some other entities could change so I don't want to inc the server load to insert one and update the others Then again update several records so updating at least twice i would insert one do the stuff and update once....

@thereisnobugs
Copy link

@kgasienica its deprecated yes, but we also plan to introduce an alternative that is better.

And... where is an alternative?

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

No branches or pull requests