Supply default values for optional values in sequence mapping if omitted by driver.

[alextech]

Fix #6682
Cannot figure out how to get XML to run properly from Ticket directory as in contributor guide. If this is a strict requirement will try again. Figured out.

[Ocramius]

Tbh, sequences are not necessarily int-based

[alextech]

They can be floats? I do not think I can do CREATE SEQUENCE INCREMENT with anything but integer. Neither PostgreSQL, nor oracle https://docs.oracle.com/database/121/SQLRF/statements_6017.htm#SQLRF01314

[Majkl578]

Postgres only supports bigint:

Sequences are based on bigint arithmetic, so the range cannot exceed the range of an eight-byte integer (-9223372036854775808 to 9223372036854775807).
https://www.postgresql.org/docs/9.5/static/sql-createsequence.html

But on 32-bit platform, bigint would be string/float...

[alextech]

Interesting. That would be still on PostgreSQL side though. The bug at highlighted line in referenced issue is in PHP side, where SequenceGenerator trying to predict maximum value. It is expecting PHP integer, as enforced by the cast, to do the addition for the prediction. But if value comes from XML parser, prediction calculation fails having second addend being string.

[alextech]

Or are you saying because of this 32-bit corner case, the schema mapping should not be touched, but the cast should be fixed instead wherever the parameters are used? In this case, are you saying xml parser is not root cause, that SequenceGenerator should be fixed?

[alextech]

Also turns out migrations wants the value to be string. If driver casts to integer itself, migration diff generates INCREMENT BY 0. Unit tests here do not notice that potential regression. so I guess the crash is better fixed at the SequenceGenerator level.

[alextech]

sorry, discovered actual root cause. Thanks @Majkl578 for making me think further about it. Turns out xml driver is not supplying default values for optional fields when they are omitted in mapping file. This results in empty $definition keys, which causes "non numeric value" error during addition, not because it is a casting problem as I originally thought.

Will redo the fix by either validating input in ClassMetadataInfo::setSequenceGeneratorDefinition where it already does a little bit of parsing validation for sequence_name, but not the other parameters, or maybe I can find why XSD schema is not applied by XML driver.

[alextech]

Kept default as string too as @Ocramius suggested, however I will still point out that

1. If plan to later convert to PHP 7 type syntax, SequenceGenerator constructor https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/Id/SequenceGenerator.php#L63 wants an integer and will not be happy with string.
2. xsd schema claims it to be integer so IMO it should throw exception if non_numeric value is given by user.

[lcobucci]

I've rebased the branch and applied some minor changes. It LGTM now!

[lcobucci]

If plan to later convert to PHP 7 type syntax, SequenceGenerator constructor https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/Id/SequenceGenerator.php#L63 wants an integer and will not be happy with string.

@alextech we only plan to add type declaration for v3 so that's fine for now, I'd say that's important to mimic the current behaviour in order to avoid BC breaks.

[alextech]

Thank you! I can now get off my custom branch.
@lcobucci sounds good. I am also seeing a very useful #6728 which is a great step, but I do not see it managing default values which is how these bugs occur. I do not know if it is validator's job to inject default values into driver, or driver should have defaults hardcoded in. ATM there is a disconnect between configuration passing schema validation, but the driver still expecting something else. So for v3 should the suggestion be made to have validator or parser pass in missing defaults, or do an "audit" of all driver components to ensure they do not crash without missing values?

[lcobucci]

So for v3 should the suggestion be made to have validator or parser pass in missing defaults, or do an "audit" of all driver components to ensure they do not crash without missing values?

@alextech the best person to answer that would be @guilhermeblanco 😄