Behaviour change in executeInserts() in 2.15.1

[sips-richard]

BC Break Report

Behaviour change in Basic Entity Persister to resolve BackedEnum to value prior to call to bindValue() means that the original value no longer makes it to configured types for handling.

09b4a75#diff-cde081d92bca42e1b0df117b840a39066470673f9457a8c5b8725e5e43a8e211R275

Q	A
BC Break	yes
Version	2.15.1+

Summary

The following code was introduced into executeInserts() in 2.15.1:

if ($value instanceof BackedEnum) {
    $value = $value->value;
}

... prior to the existing logic to bind values after processing them via their configured types:

$stmt->bindValue($paramIndex++, $value, $this->columnTypes[$column]);

This means that our custom enum types are no longer receiving the enum case as it is being pre-resolved.

This is a bit of a heavy-handed change for a patch version, no?

Previous behavior

Enum cases were simply passed to bindValue() and were processed by type objects.

$stmt->bindValue($paramIndex++, $value, $this->columnTypes[$column]);

Current behavior

BackedEnums are converted to backed value so that only backed value reaches the configured type; in our cases our custom enum types are expecting enum cases, not their value.

We can amend our enum types to handle the value as an alternative to the enum case if necessary, but the change seems rather excessive for a patch change.

How to reproduce

See above.

[mpdude]

@Gwemox can you help here? You authored the change that caused the regression.

[Gwemox]

I no longer remember the reason for these lines.
However, without these lines the tests continue to pass.

I think @greg0ire we can revert those 3 lines to fix this BC ?

[greg0ire]

Yeah sure, let's revert them. If they turn out to be important, let's restore them, but this time, with a meaningful commit message, code comments, tests showing their purpose.