Aura.SqlQuery
Aura.SqlQuery copied to clipboard
Go back to pseudo-sequential where() and having() placeholders?
Hi all, esp. @pavarnos --
Earlier, we removed ?-placeholder replacements from where(), having(), etc., and replaced it with binding of named parameters as part of the method call. So, in 2.x we had this:
$select->where('foo IN(?)', [1, 2, 3]);
// WHERE foo IN (:_1_)
And then changed it to this in 3.x:
$select->where('foo IN(:foo)', ['foo' => [1, 2, 3]]);
// WHERE foo IN (:foo)
After having used this for a while, I'm unsatisfied with it. It feels clunky and verbose.
This PR kind-of goes back to pseudo-sequential placeholders. However, instead of ?-placeholders, the conditions are alternately interpreted as literals and bind-values:
$select->where('foo IN(', [1, 2, 3], ')');
// WHERE foo IN(:_1_)
This feels a little bit better to me overall. It also makes it so that the SqlQuery code does not need to scan through for ?-marks, which means the various Postgres JSON query syntaxes are left untouched.
Any thoughts here?
Thanks for asking... Interesting idea. Looks like your code would handle complicated stuff like the below OK
$select->join('left', 'person_travel', 'person_travel.person_id = person.id and person_travel.finishes_on >= :startDate and person_travel.finishes_on <= :endDate')
or
$select->where('(finish_date >= :finishDate or finish_date = 0)');
(note the brackets to create the OR condition: all the other where() clauses are ANDed).
I kinda like the verbose: it makes it clear what goes where. It also looks like your code is not a BC break (?) so it won't matter too much if people don't want to use it that way?
It's a BC break against both 2.x and the current 3.x, in that it uses alternating "SQL" and "value" params via ...$cond
, rather than $cond, $bind = array()
. The following works now in 3.x, but will not work if the PR merges:
$select->where('foo = :foo OR bar = :bar', ['foo' => 'fooval', 'bar' => 'barval']);
However, this will work with the PR:
$select->where('foo = :foo OR bar = :bar');
$select->bindValues(['foo' => 'fooval', 'bar' => 'barval']);
And so will this:
$select->where('foo = ', 'fooval', ' OR bar = ', 'barval');
You see where I'm coming from?
yep. thanks for the extra explanation. on further thought i quite like the new syntax: it is clean, lightweight, flows well, and makes good use of new language features. Can some assertions be added to help manage the BC breaks? eg if count($cond) == 2 && is_string($cond[0]) && is_array($cond[1]) then this probably won't work like you expect....
Let me see about 2.x breaks; 3.x has not had a release, so technically it's not a BC break -- but still, there might be a reasonable way to make an allowance.
thanks. it will make migration from 2.x to 3.x easier i guess. fwiw i'm living dangerously and using 3.x in production. its working well.
What's the purpose of tracking instance counts? Just curious as I don't think it's that obvious.
@djmattyg007 i think tracking instance counts makes it in the end easier to find out what values where bound to which parameter because they are named
$select->where('foo IN(', [1, 2, 3], ')');
$select->where('bla IN(:bla)');
$select->where('bar IN(', [7, 8, 9], ')');
$select->bindValue("bla", [4,5,6]);
// WHERE foo IN(:_1_) AND bar IN(:bar) AND bla IN(:_2_)
// with bind values [ "_1_" => [1,2,3], "_2_" => [7,8,9], "bla" =>[4,5,6] ]
What I would like even more is if you could optionally do the naming using back-scanning (the placeholder must be at the end of the partial string)
$select->where('foo IN(:bars', [1, 2, 3], ')');
// WHERE foo IN(:bars)
// bindValues: [ "bars" => [1,2,3] ]
or without back-scanning
$select->where('foo IN(', [ "bars" => [1, 2, 3] ], ')');
// WHERE foo IN(:bars)
// bindValues: [ "bars" => [1,2,3] ]
If you restrict yourself to simpler statements per where calls you end up with triples of statement-snippet, placeholder and value
$select->where("ID_KEY = ", ":ID_KEY", $val);
$select->where("fpath LIKE ", ":USERDIR", "%/user/%");
$select->where("name IN ", "(:NAMES)", ["pmjones","pavarnos"]);
Similar one https://github.com/auraphp/Aura.SqlQuery/pull/162 ?
@pmjones Any reason you didn't merge this PR?
@harikt I don't recall. :-/