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

DibiTranslator: respect %if blocks for %lmt and %ofs as well #145

Closed
wants to merge 1 commit into from
Closed

DibiTranslator: respect %if blocks for %lmt and %ofs as well #145

wants to merge 1 commit into from

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Aug 26, 2014

Fixes issue #87.

In current dibi, something like

\dibi::query([
    'SELECT * FROM foo',
    '%if', false,
        '%lmt', 3,
        '%ofs', 5,
    '%end',
]);

yields SELECT * FROM foo /* */ LIMIT 3 OFFSET 5, which is probably not what you want.

This changes the translation of %lmt and %ofs to only set ->limit and ->offset when ->comment is falsy.

The new behaviour is to generate SELECT * FROM foo /* (limit 3) ... (offset 5) */.
I would have preferred to generate proper SQL, but it would have been possible only with the MySQL_, Postgres and SQLite_ drivers. The other drivers' applyLimit changes the whole SQL statement and thus would generate horrible stuff.

Thus, while I feel it's necessary to mention the limit/offset in the comment (because debugability), I think we should not pretend to generate valid SQL by making it /* LIMIT 3 OFFSET 5 */ because since we can't make it valid always, we should be explicit about it not being valid.

But feel free to change it, obviously :).

@dg dg force-pushed the master branch 8 times, most recently from 601dbd2 to 26b167f Compare January 12, 2015 10:01
@dg dg closed this in d6826d6 Jan 13, 2015
@dg
Copy link
Owner

dg commented Jan 13, 2015

Thank you!

jasir added a commit to jasir/dibi that referenced this pull request Mar 2, 2015
* master: (37 commits)
  Released version 2.3.1
  removed version.txt
  dibi: named connections are allowed [Closes dg#161]
  Dibi: Dump now recognize MsSql2012 offset as keyword
  DibiPdoDriver: added support for MsSql2012 Offset
  Tracy\Panel: added vector icon
  added contributing.md
  Released version 2.3.0
  Postgre: added test for matching by %like
  Postgre: fixed %like escaping [Closes dg#159]
  Dibi: $defaultDriver changed to mysqli [Closes dg#156]
  Released 2.3.0-RC1
  removed bridge for Nette 2.0 (BC break)
  dibi: named connections and activate() are deprecated (BC break)
  DibiFluent: add `leftJoin` and `on` to phpdoc.
  DibiFirePhpLogger: save some header operations for sites with hundreds of sql queries.
  DibiFirePhpLogger: Allow user defined size of json stream chunks [Closes dg#148]
  DibiTranslator: respect %if blocks for %lmt and %ofs as well [Closes dg#145][Closes dg#87]
  DibiResult: float detection locale fix [Closes dg#154]
  DibiMySqliDriver.php: fixes for HHVM
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants