auraphp / Aura.SqlQuery

Independent query builders for MySQL, PostgreSQL, SQLite, and Microsoft SQL Server.
MIT License
452 stars 86 forks source link

Go back to pseudo-sequential where() and having() placeholders? #142

Open pmjones opened 7 years ago

pmjones commented 7 years ago

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?

pavarnos commented 7 years ago

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?

pmjones commented 7 years ago

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?

pavarnos commented 7 years ago

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....

pmjones commented 7 years ago

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.

pavarnos commented 7 years ago

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.

djmattyg007 commented 7 years ago

What's the purpose of tracking instance counts? Just curious as I don't think it's that obvious.

agrrr3 commented 6 years ago

@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"]);
harikt commented 6 years ago

Similar one https://github.com/auraphp/Aura.SqlQuery/pull/162 ?

harikt commented 2 years ago

@pmjones Any reason you didn't merge this PR?

pmjones commented 2 years ago

@harikt I don't recall. :-/