Skip to content

Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0#3248

Merged
morozov merged 1 commit intodoctrine:developfrom
morozov:non-nullable-offset
Aug 12, 2018
Merged

Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0#3248
morozov merged 1 commit intodoctrine:developfrom
morozov:non-nullable-offset

Conversation

@morozov
Copy link
Copy Markdown
Member

@morozov morozov commented Aug 11, 2018

Q A
Type improvement
BC Break yes
Fixed issues none

Semantically, the absence of the offset in a LIMITed query ($offset = null) means "start from the beginning", i.e. with the zero offset. Therefore, $offset = null and $offset = 0 are semantically equivalent.

From the practical standpoint, allowing $offset = null means that we have to cast it to an (int) or consider its nullability during strict comparison.

This patch makes the offset non-nullable defaulting to 0. The usage of the NULL offset in DBAL 2.x will be deprecated in favor of using 0.

@morozov morozov added this to the 3.0.0 milestone Aug 11, 2018
@morozov morozov self-assigned this Aug 11, 2018
@morozov morozov requested review from Majkl578 and Ocramius August 11, 2018 04:12
morozov
morozov commented Aug 11, 2018
View reviewed changes
lib/Doctrine/DBAL/Platforms/SqlitePlatform.php
{
if ($limit === null && $offset > 0) {
return $query . ' LIMIT -1 OFFSET ' . $offset;
$limit = -1;
Copy link
Copy Markdown
Member Author

@morozov morozov Aug 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not strictly related to the issue, just a code duplication cleanup.

@morozov morozov force-pushed the non-nullable-offset branch from cd4e8b1 to 1ffceaa Compare August 11, 2018 05:56
@morozov morozov force-pushed the non-nullable-offset branch from 1ffceaa to 980a01f Compare August 11, 2018 05:59
Majkl578
Majkl578 approved these changes Aug 11, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@Majkl578 Majkl578 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, makes sense.

@morozov morozov merged commit 8578aac into doctrine:develop Aug 12, 2018
@morozov morozov deleted the non-nullable-offset branch August 12, 2018 02:33
@morozov morozov mentioned this pull request Aug 16, 2018
Merged
morozov added a commit that referenced this pull request Sep 29, 2018
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
635a86b 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Oct 7, 2018
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
500c4a8 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Oct 7, 2018
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
021d6e0 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Nov 14, 2018
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
1201311 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Nov 14, 2018
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
110d8af 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Nov 25, 2018
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
ff92ad6 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Dec 6, 2018
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
c405841 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Jan 20, 2019
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
df49b20 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit to morozov/dbal that referenced this pull request Feb 6, 2019
@morozov
Merge pull request doctrine#3248 from morozov/non-nullable-offset
bd21b59 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit to morozov/dbal that referenced this pull request Feb 6, 2019
@morozov
Merge pull request doctrine#3248 from morozov/non-nullable-offset
843aaaf 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Feb 7, 2019
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
eca47dc 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Feb 20, 2019
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
68cc0ec 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Feb 22, 2019
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
0df0447 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Mar 15, 2019
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
6fca9e6 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Mar 15, 2019
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
fb0f5ca 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Mar 18, 2019
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
7b7bfc3 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Apr 16, 2019
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
809fe31 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request May 6, 2019
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
0982356 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request May 23, 2019
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
506a04b 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit to morozov/dbal that referenced this pull request May 31, 2019
@morozov
Merge pull request doctrine#3248 from morozov/non-nullable-offset
608f91c 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Jun 13, 2019
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
192df5e 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Jun 27, 2019
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
4456015 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit to morozov/dbal that referenced this pull request Aug 26, 2019
@morozov
Merge pull request doctrine#3248 from morozov/non-nullable-offset
107d2f4 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
morozov added a commit that referenced this pull request Nov 2, 2019
@morozov
Merge pull request #3248 from morozov/non-nullable-offset
5b6b13e 
Made the OFFSET in LIMIT queries non-nullable integer defaulting to 0
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Ocramius Ocramius Awaiting requested review from Ocramius

1 more reviewer

@Majkl578 Majkl578 Majkl578 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@morozov morozov

Labels

BC Break Improvement Platforms Query Limit/Offset Modification

Projects

None yet

Milestone

4.0.0-beta1

Development

Successfully merging this pull request may close these issues.

2 participants

@morozov @Majkl578