Skip to content

Commit

Permalink
Validating 'limit' in Oracle
Browse files Browse the repository at this point in the history
  • Loading branch information
@andresgutierrez
andresgutierrez committed Dec 13, 2015
1 parent 00378ef commit 1fe6089
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 27 deletions.
54 changes: 30 additions & 24 deletions ext/phalcon/db/dialect/oracle.zep.c
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 9 additions & 3 deletions phalcon/db/dialect/oracle.zep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,18 @@ class Oracle extends Dialect
let offset = (int) trim(number[1], "'");
}

let limit = (int)trim(number[0], "'") + offset;
let limit = (int) trim(number[0], "'") + offset;
} else {
let limit = (int)trim(number, "'");
let limit = (int) trim(number, "'");
}

let sqlQuery = "SELECT * FROM (SELECT Z1.*, ROWNUM PHALCON_RN FROM (" . sqlQuery . ") Z1 WHERE ROWNUM <= " . limit . ")";
let sqlQuery = "SELECT * FROM (SELECT Z1.*, ROWNUM PHALCON_RN FROM (" . sqlQuery . ") Z1";

if limit != 0 {
let sqlQuery .= " WHERE ROWNUM <= " . limit;
}

let sqlQuery .= ")";

if offset != 0 {
let sqlQuery .= " WHERE PHALCON_RN >= " . offset;
Expand Down

4 comments on commit 1fe6089

@sergeyklay
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@rlaffers
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what this commit was intended to resolve but it breaks the oracle db support for me. When I do findFirst, the constructed query does not contain any limits (which is weird), and the result correctly contains the first record. However, when I do find(array('limit' => 1)) the query again contains no limits but the result contains no records, which is a bummer. The limit method needs to account for the possibility when the limit is passed as a "APL0" bound parameter as mentioned in #11217

@rlaffers
Copy link
Contributor

Choose a reason for hiding this comment

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

@andresgutierrez I fixed the Oracle dialect in my private branch at rlaffers@91985cf

This fixes #11033 (comment) and #11217

Unfortunately I had to branch off from the last known working commit from just before your commit #1fe6089a2773fe0926af34c6202470ee32c199be . After that, it seems Phalcon does not step into the limit method in the Oracle dialect when using findFirst() or find with limit specified. Can you confirm this behavior? I would like to bring my fix into the 2.0.x branch but first we have to figure out what went wrong later on.

@sergeyklay
Copy link
Contributor

Choose a reason for hiding this comment

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

@rlaffers Hello. Create please an issue for this discussion

Please sign in to comment.