Fix bigint PHP_INT_MIN/PHP_INT_MAX string to int convert

[mvorisek]

Q	A
Type	bug
Fixed issues	n/a

Summary

Resolve #6177 (comment) discussion and related original #6177.

Whole native php int range is guaranteed to be supported per https://www.doctrine-project.org/projects/doctrine-dbal/en/4.0/reference/types.html#bigint docs.

[mvorisek]

@derrabus can you please review?

[derrabus]

Please don't ping me on PRs please.

[mvorisek]

I pinged you because of you authored the original PR, sorry.

[derrabus]

Please keep in mind that this piece of code will be executed on a hot path. If I hydrate thousands of entities with bigint fields, I don't want to execute preg_match() for each of them. Your implementation is too expensive.

[mvorisek]

Please keep in mind that this piece of code will be executed on a hot path. If I hydrate thousands of entities with bigint fields, I don't want to execute preg_match() for each of them. Your implementation is too expensive.

PCRE JIT is very fast, but yes, the regex replace is possible to be coded /wo regex and I alredy considered that option because only limited number (4096) are cached. I will rework the code.

[mvorisek]

Please keep in mind that this piece of code will be executed on a hot path. If I hydrate thousands of entities with bigint fields, I don't want to execute preg_match() for each of them. Your implementation is too expensive.

done

[derrabus]

Sorry, but that implementation is way too complicated. I don't want to maintain this.

[mvorisek]

The implementation is minimal, we strip leading plus sign, zeros and trailing zeros after decimal point. If the number is then castable into int without precision loss, we cast it.

The leading/trailing zeros should be stripped because the input number can come from sources with explicit digits/decimal configured. This behaviour is tested and I do not think it can be implemented simpler. If you have an idea how to implemenet this simpler/better, I am of course ready for your ideas.

[mvorisek]

If leading/trailing zeros should be supported, the impl. is minimal IMO. I would be happy if this can either be merged as is or please let me know how to fix the min/max issue differently.

[derrabus]

Which DBMS formats 2^31-1 with leading zeros?

[mvorisek]

I tested all DBs using https://dbfiddle.uk/6OSky-ka and none DB vendor prepend leading zeros even for DECIMAL type by default.

So you want me to remove the "leading zeros accepting" code in order to save a few lines of code?

[derrabus]

🤷‍♂️

[mvorisek]

@greg0ire with Alexander I got to a point when this PR is lighweight and passing the tests. Can you please clarify "I don't think this is going anywhere" into a feedback I can act on? Can you please reopen this PR?

[greg0ire]

Clarification: I don't think there is anything you can do.

[mvorisek]

Sorry, do you understand the problem, are you aware that valid max. int 2^64-1 (and min.) value is currently not properly casted to int type?

[greg0ire]

I don't understand the problem, and I do not see an explanation about it anywhere.

[derrabus]

I don't understand the problem

The problem ist that if the DB would return PHP_INT_MAX as string, we would not convert it to int although one might expect that.

[greg0ire]

I won't block this anymore but I don't get the fix, feel free to deal with this if you understand it @derrabus

[derrabus]

As soon as the unnecessary changes to the test have been reverted, I'm fine with the PR.