[Sql Server] Add support for different integer sizes #1250
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stickler-ci
reviewed
Dec 4, 2017
| @@ -44,6 +44,12 @@ class SqlServerAdapter extends PdoAdapter implements AdapterInterface | |||
|
|
|||
| protected $signedColumnTypes = ['integer' => true, 'biginteger' => true, 'float' => true, 'decimal' => true]; | |||
|
|
|||
| const INT_TINY = 255; | |||
| @@ -44,6 +44,12 @@ class SqlServerAdapter extends PdoAdapter implements AdapterInterface | |||
|
|
|||
| protected $signedColumnTypes = ['integer' => true, 'biginteger' => true, 'float' => true, 'decimal' => true]; | |||
|
|
|||
| const INT_TINY = 255; | |||
| const INT_SMALL = 65535; | |||
| @@ -44,6 +44,12 @@ class SqlServerAdapter extends PdoAdapter implements AdapterInterface | |||
|
|
|||
| protected $signedColumnTypes = ['integer' => true, 'biginteger' => true, 'float' => true, 'decimal' => true]; | |||
|
|
|||
| const INT_TINY = 255; | |||
| const INT_SMALL = 65535; | |||
| const INT_MEDIUM = 16777215; | |||
| const INT_SMALL = 65535; | ||
| const INT_MEDIUM = 16777215; | ||
| const INT_REGULAR = 4294967295; | ||
| const INT_BIG = 18446744073709551615; |
| if ($limit && $limit >= static::INT_TINY) { | ||
| $sizes = [ | ||
| // Order matters! Size must always be tested from longest to shortest! | ||
| 'bigint' => static::INT_BIG, |
| $sizes = [ | ||
| // Order matters! Size must always be tested from longest to shortest! | ||
| 'bigint' => static::INT_BIG, | ||
| 'int' => static::INT_REGULAR, |
| 'bigint' => static::INT_BIG, | ||
| 'int' => static::INT_REGULAR, | ||
| 'mediumint' => static::INT_MEDIUM, | ||
| 'smallint' => static::INT_SMALL, |
| 'int' => static::INT_REGULAR, | ||
| 'mediumint' => static::INT_MEDIUM, | ||
| 'smallint' => static::INT_SMALL, | ||
| 'tinyint' => static::INT_TINY, |
iluuu1994
force-pushed
the
feature/sqlsrv-int-types
branch
from
7258b3a
to
4f18644
Compare
stickler-ci
reviewed
Dec 4, 2017
| @@ -44,6 +44,12 @@ class SqlServerAdapter extends PdoAdapter implements AdapterInterface | |||
|
|
|||
| protected $signedColumnTypes = ['integer' => true, 'biginteger' => true, 'float' => true, 'decimal' => true]; | |||
|
|
|||
| const INT_TINY = 255; | |||
| const INT_SMALL = 65535; | |||
| const INT_MEDIUM = 16777215; | |||
iluuu1994
force-pushed
the
feature/sqlsrv-int-types
branch
from
4f18644
to
1668ce1
Compare
Codecov Report
@@ Coverage Diff @@
## master #1250 +/- ##
==========================================
+ Coverage 72.32% 74.38% +2.05%
==========================================
Files 35 35
Lines 5551 4744 -807
==========================================
- Hits 4015 3529 -486
+ Misses 1536 1215 -321
Continue to review full report at Codecov.
|
|
Why does this happen? Is the |
|
Thank you for your contribution. That aside, I tested your patch on my local computer with sqllocaldb, but it didn't pass. Would it pass on your environment? |
|
@chinpei215 Thank you for testing! Yeah I was hoping that I could use the CI process and wouldn't have to set it up myself locally. I'll look at the issues and report back. |
chinpei215
reviewed
Dec 6, 2017
| const INT_SMALL = 65535; | ||
| const INT_MEDIUM = 16777215; | ||
| const INT_REGULAR = 4294967295; | ||
| const INT_BIG = 18446744073709551615; |
There was a problem hiding this comment.
Is is possible to declare those constants as strings? Sorry if I am wrong, but I guess INT_BIG would overflow in 32bit PHP, and if you want to use unsigned bigint, it would overflow even in 64bit PHP.
|
@chinpei215 I took this from the MysqlAdapter which does it in the same way. |
|
Oh I didn't know that. Sorry. Then I don't request changes in this PR. |
|
@chinpei215 Oh that's fine 😄 We had several other issues and decided to write native queries in the migrations which is suboptimal but easier in our situation. Thus I'll have to fix these issues in my free time. I'll look at them as soon as possible. |
|
Thanks. |
|
@iluuu1994 Do you still want to add those? |
|
Nope, I'm not using this library anymore. You're free to take over. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.