On Oct 12, 12:06 pm, Nate Wiger <[email protected]> wrote:
> I'm not sure I'd call 30% a modest improvement. :-) That's a huge
> difference when you're talking lots of transactions.
I would refer to anything less than a binary order of magnitude as a
huge improvement, but I'll agree that it's more than modest. :)
> Regarding the API, I just want to point out that 'id = :id' is not my
> syntax - it's pervasive in DB adapters and ORM's alike. Hence the
> reason I naively tried it in Sequel and stumbled on it raising an OCI
> error ("not all variables bound").
It's not used in PostgreSQL to my knowledge (which uses $1), nor on
MySQL (which uses ?). I think SQLite allows it, and it looks like
Oracle does as well. So I wouldn't say that it's pervasive in DB
adapters.
I'm not sure when ActiveRecord started supporting it, but it think it
was after 2.0.
> I think there are a couple things that are getting intertwined:
>
> 1) My proposal to extend the Sequel syntax - bind vars or not - to
> support :named placeholders like other DB toolsets
True. Using a placeholder literal string with a hash of conditions
would be a feature by itself.
> 2) Your concern that bind variables can be enabled/disabled per-query
> since some adapters (or queries) may not want to use them
I'll go out on a limb here and state that due to the necessary
overhead, my theory is that using bind variables with Sequel will
decrease performance in the general case on most adapters.
> 3) Backwards compatibility/spec/adapter concerns (I'm a huge proponent
> of backwards compat, so let's assume that one will be solved)
I'd say that's a huge assumption given some of what you are
suggesting, but OK. :)
> Regarding #1, let me try to show why this is really powerful. Here's
> a real snippet from our codebase:
>
> curs = conn.parse("SELECT lb.* FROM
> (SELECT rownum as rn, a.* FROM
> (SELECT * FROM #{table_name}
> WHERE game_type_id = :game_type_id
> AND platform_id = :platform_id
> AND rank >= :rank-:num_above_below
> AND rank <= :rank+:num_above_below
> ORDER BY rank) a) lb,
> (SELECT rn from
> (SELECT rownum as rn, b.player_id FROM
> (SELECT player_id FROM #{table_name}
> WHERE game_type_id = :game_type_id
> AND platform_id = :platform_id
> AND rank >= :rank-:num_above_below
> AND rank <= :rank+:num_above_below
> ORDER BY rank) b)
> WHERE player_id = :player_id) p_rownum
> WHERE lb.rn >=
> p_rownum.rn-:num_above_below AND lb.rn <= p_rownum.rn+:num_above_below
> ORDER BY lb.rank", sql_args])
>
> sql_args = {:game_type_id => game_type_id,
> :platform_id => platform_id,
> :player_id => player_id,
> :rank => player.rank,
> :num_above_below => num_above_below}
>
> You'll notice that the bind params are reused multiple times in the
> statement - :player_id, :num_above_below, etc. (That query is highly-
> tuned c/o my team's DBA)
The Sequel way to write that would be to use Sequel's DSL. However, I
can see your point in regards to having existing SQL with named
placeholders and wanting to easily port your code to Sequel. Allowing
placeholder literal strings to take a hash of arguments would make
that case much easier, so I'll support including this in Sequel.
> When that query was first written, the programmer used ?'s and
> positional binding, and it was a mess. You couldn't tell which ? went
> with which parameter, and it actually had a bug since the wrong one
> was bound to the wrong position, and it took a good long while to
> figure out.
That's completely understandable. Like I mentioned, the Sequel way to
write that would be to use the DSL, not interpolated strings, which
avoids the issue in a different way.
> So in this case, your suggestion of wrapping the existing API:
>
> dataset.bindfilt('id = :id', :id=>1) == dataset.filter('id = ?', :
> $id).bind(:id=>1)
>
> Doesn't work, since it's the *:named placeholder itself* that's so
> important. Replacing it with ?'s results in the problems I mentioned
> above.
That's because you don't understand how Sequel works. Sequel would
literalize this:
dataset.filter('id = ?', :$id).bind(:id=>1)
as:
id = :id
Example using SQLite, which uses named placeholders:
$ ruby -I lib bin/sequel -E sqlite:/
Your database is stored in DB...
irb(main):001:0> DB.create_table(:a){Integer :a}
I, [2009-10-12T12:55:01.252667 #8359] INFO -- : CREATE TABLE `a` (`a`
integer)
=> []
irb(main):002:0> DB[:a].filter('a = ?', :$blah).call
(:select, :blah=>1)
I, [2009-10-12T12:55:28.105581 #8359] INFO -- : SELECT * FROM `a`
WHERE (a = :blah); {"blah"=>1}
=> []
irb(main):003:0>
With the named placeholder support, you could always translate it this
way instead this instead:
dataset.bindfilt('id = :id', :id=>1) =>
dataset.filter('id = :id', :id=>$id).bind(:id=>1)
However, there would be no reason to do it that way, as that would
just be slower.
> 2) Regarding enabling/disabling bind vars, as a variant of your idea,
> how about this:
>
> # No bind variables (default behavior remains unchanged)
> User.filter('id = ?', 14).all
> User.filter('id = :id', :id => 14).all
> User.filter('lower(city) like lower(:search) or lower(state) like
> lower(:search)', :search => string).all
That wouldn't have any problems.
> # Enable bind variables by calling bind()
> User.filter('id = ?', 14).bind.all
> User.filter('id = :id', :id => 14).bind.all
I think that would be problematic to implement correctly. What's
wrong with adding a new method that does exactly what you want?
> # bind() accepts delayed bind args too
> User.filter('lower(city) like lower(:search) or lower(state) like
> lower(:search)').bind(:search => string).all
I've already explained that a single string passed to filter becomes a
literal string (not a placeholder literal string), and Sequel never
parses literal strings, so this can't work.
> Additionally, how about an adapter setting (off by default) to
> globally always use bind()?
>
> DB = Sequel.connect(..., :logger => ..., :always_bind_variables =>
> true) # or whatever
>
> Then a person could still disable it per-query with bind(false):
>
> User.filter('id = ?', 14).bind(false).all
>
> This way we get the "best of all worlds."
Assuming that you could get 2) working correctly (a huge assumption),
that seems OK.
> Finally, there's no reason why the existing API couldn't remain around
> for compat, and this stuff could all be shortcuts to it. But
> the :named placeholders would have to go in core since they're in the
> sql string like ? is.
>
> Thoughts?
I still think the suggestions I made in my previous post stand, with
some small additions:
1) Add support to Sequel's Oracle adapter to support bound variables
and prepared statements, using Sequel's existing API.
2) Add support for placeholder literal strings to accept a single hash
of arguments and use named placeholders in that case.
3) Add support for Dataset#bind, for setting bound variables before
calling Dataset#call.
4) Create a Sequel extension that adds a method allowing nicer syntax
for bound variables/prepared statements. If this extension meets the
5 points I mentioned earlier, I wouldn't have a problem shipping it
with Sequel.
I wouldn't have a problem completing 2) and 3) myself, and I could
probably help you with 4) if not complete it myself. Since I don't
have an Oracle installation, you'll have to complete 1) yourself,
though I'd be happy to answer any Sequel-related questions.
Jeremy
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"sequel-talk" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/sequel-talk?hl=en
-~----------~----~----~----~------~----~------~--~---
