Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBM_DB2 platform doesn't handle offset without limit #2463

Merged
merged 1 commit into from
Jan 16, 2017

Conversation

morozov
Copy link
Member

@morozov morozov commented Aug 3, 2016

There's a non-empty table users. The following script should return all existing record IDs.

$builder = $conn->createQueryBuilder();
$builder->select('id')
    ->from('users')
    ->setFirstResult(0);
$data = $builder->execute()->fetchAll();
var_dump($data);

It does so on Oracle, SQL Server and MySQL, but returns empty result on DB2. The reason is that resulting SQL has a condition like WHERE db22.DC_ROWNUM BETWEEN 1 AND 0.

Copy link
Member

@deeky666 deeky666 left a comment

Choose a reason for hiding this comment

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

Patch is good, just some minor CS adjustments required.

$where[] = sprintf('db22.DC_ROWNUM <= %d', $offset + $limit);
}

if (count($where) === 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you use empty() here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. Just trying to understand the requirements, is empty($array) somehow better than count($array)? IMO, empty() is prone to typos like empty($were) which one may not notice w/o running tests.

Copy link
Member

Choose a reason for hiding this comment

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

@morozov this is just about minor optimizations. empty() is a language construct and much more efficient than using the function count(). The point about typos is a good point, but in DBAL we try to code as much performance efficient as possible even if it seems neglible. Thanks for changing.

Copy link
Member Author

Choose a reason for hiding this comment

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

@deeky666 gotcha, thanks, will keep that in mind in the future.


// Todo OVER() needs ORDER BY data!
$sql = 'SELECT db22.* FROM (SELECT db21.*, ROW_NUMBER() OVER() AS DC_ROWNUM '.
'FROM (' . $query . ') db21) db22 WHERE db22.DC_ROWNUM BETWEEN ' . ($offset+1) .' AND ' . ($offset+$limit);
$sql = sprintf(
Copy link
Member

Choose a reason for hiding this comment

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

Can you return early here like return sprintf()?

@deeky666 deeky666 merged commit 3004473 into doctrine:master Jan 16, 2017
@deeky666
Copy link
Member

@morozov thanks for your effort!

@deeky666
Copy link
Member

Backported to 2.5 via 0f7bf0c

@morozov morozov deleted the db2-offset-no-limit branch January 16, 2017 23:35
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants