Generated/Virtual Columns: Insertable / Updateable

[mehldau]

Defines whether a column is included in an SQL INSERT and/or UPDATE statement.

• insertable: false - do not include this column as part of INSERT statement for entities of the type.
• updatable: false - do not include this column as part of UPDATE statement
• generated: "always" - after inserting or updating this entity, always fetch an updated value from the database.
• generated: "insert" - after inserting this entity, fetch an updated value from the database.

Use Cases

Map a column several times.

#[Entity] 
class User
{
     #[ManyToOne(targetEntity: Country:class), JoinColumn(name: "country_code", referencedColumnName: "country_code")]
     public $country;
     
     #[Column(type: "string", name: "country_code", insertable: false, updatable: false)]
     public $countryCode;
}

Columns with database updates

#[Entity]
class Article
{
    #[Column(type: "datetime", 
        columnDefinition: "TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP", 
        insertable: false,
        updatable: false,
        generated: "ALWAYS")]
    public $created;
}

#5728

[greg0ire]

Please rebase on 2.11.x and force push, new features should go on that branch now. Also, please add documentation about this (in docs)

[beberlei]

This is already amazing work and i am happy this looks like it be implemented with just a few lines in BasicEntityPersister.

I made a few suggestions about optimizations and towards more ORM idomatic style.

One thing I checked is weather we should even allow non updateable / insertable columns to show up in a changeset. But I believe this would cause more harm to performance since there is no access to $fieldMapping in changeset computation.

One thing this PR does not address here is auto updated columns in the database. In my opinion if there is a column that is not insertable / updateable, that means we need to fetch them after an insert / update and set them on the entity. The updatable = false should not be used to help with immutability, it means the database updates this value itself (via trigger for example).

[mehldau]

This is already amazing work and i am happy this looks like it be implemented with just a few lines in BasicEntityPersister.

Thank you @beberlei

I made a few suggestions about optimizations and towards more ORM idomatic style.

I addressed this and your proposal of inverting 'insertable' and 'updateable' reg. optimization of the ClassMetadata cache in the current PR.

One thing this PR does not address here is auto updated columns in the database. In my opinion if there is a column that is not insertable / updateable, that means we need to fetch them after an insert / update and set them on the entity. The updatable = false should not be used to help with immutability, it means the database updates this value itself (via trigger for example).

You're absolutely right. Maybe you could lent me your expertise to find the most suitable hook point to implement this. Maybe remember the necessity of reloading the Entity within BasicEntityPersister::prepareUpdateData() and use this information somewhere within the update / insert - processing? Or could I use the event system either way?

[beberlei]

@mehldau the event system should only be used by outside integrations. We hardcode our logic into the persisters somewhere.

There is already precedent for this in the isVersioned property and its code in assignDefaultVersionValue to fetch the database assigned version id. We can generalize this code.

a.) Introduce a new property requiresFetchAfterUpdate that defaults to false. if a versioned field is set or any notUpdateable / notInsertable property exists set this property true.
b.) Refactor assignDefaultVersionValue to work over all not insertable / updateable / versioned columns.

[mehldau]

There is already precedent for this in the isVersioned property and its code in assignDefaultVersionValue to fetch the database assigned version id. We can generalize this code.

a.) Introduce a new property requiresFetchAfterUpdate that defaults to false. if a versioned field is set or any notUpdateable / notInsertable property exists set this property true. b.) Refactor assignDefaultVersionValue to work over all not insertable / updateable / versioned columns.

@beberlei All right. I implemented the changes as suggested and updated the pull request.

Nevertheless, I'm wondering whether we have to reload 'non insertable' columns in the edge case of 'updating' an entity which only has 'non insertable' columns defined.

[beberlei]

Getting very close here :)

A few questions:

• Did we miss the notUpdateable now? Not sure what condition/code prevents a field to be updated. Can you point me to it?
• Can you add a test for a joined table updateable as well?

[beberlei]

@mehldau I took the day to improve this PR, catching a nasty edge case with Second Level Cache that we haven't thought of yet.

I removed the exception when updating a field that is not updatable, this value just gets overwritten with the correct one from the DB now. There are still these todos I want to tackle before merging this:

• Add a test with joined table inheritance
• Add tests for AbstractMappingDriver using Insertable and Updatable entities, will also test xml, attribute and other drivers.
• Fetch only the right columns on insert or update, not all of them

[beberlei]

One point that i am still struggling with, there are cases where you want to disable insertable or updateable but don't want data to be fetched from the database after each write. For example if you want to use updateable: false for immutable properties, then why fetch them from the database?

Hibernate (but not JPA) differentiates with another option @Generated(time) where time is an enum with the values always or insert. We should copy this behavior instead of magically adding this when just using insertable/updateable.

[derrabus]

What is our BC policy regarding BasicEntityPersister? The class is not final and full of protected members. This PR renames some protected methods and changes the signature of others. Do we expect userland code to override that class and specific methods in it? If so, shall we document this PR as a minor BC break?

[beberlei]

@derrabus its not possible to override the persisters, their factory method is guarded by final things in EM and UoW.

[derrabus]

So we should probably finalize them as well, I guess. For 3.0 at least.🤔

[derrabus]

Anyway, this would be out if scope here. Thanks for the explanation.

[beberlei]

I believe I want to simplify this to use virtual: true instead of insertable/updatable, because to me the use-cases for disabling only one of insert/update for a column seems to be a more than edgy edge case that we can just choose not to support.

[beberlei]

Hm ok, virtual makes no sense in naming either, because that would mean the column does not exist (??) in the schema? Maybe the naming is fine the way it is here.

[beberlei]

@mehldau Thank you very much for your work on this important feature!