On Oct 12, 5:27 pm, Nate Wiger <[email protected]> wrote: > Overall that sounds good - just checked the commits, thanks for the > fast patches. > > Regarding the bind_filter approach, though, I'm still not too hot on > it to be honest. I guess I don't see a case where I'd not want bind > vars when dealing with Oracle. And I also don't see how it would > handle inserts/updates that are deep within the Sequel::Model API > > User.create(props) > user = User[id] > user.name = 'Yo' > user.update etc etc
The Model case is actually easier than the dataset case. You'd just need to create a model plugin that overrides Model#_insert and Model#_update to use bind variables. I can show you a fairly simple example if you are interested. I wouldn't have a problem supporting the plugin and shipping it with Sequel. Associations wouldn't use bound variables, but unless you using large object fields as keys (which, considering your emphasis on performance, I doubt), the performance differences are probably not significant. Were you getting 30% performance differences for binding vs. literalizing for simple integers on Oracle? You still haven't provided any benchmarks, so I'm not sure. > I like the Sequel DSL and I'm sure the programmers on my team will > want to do > > User.filter(stuff).group_by(stuff).order(stuff).etc.etc > > I don't want to have to wrap basically the entire Sequel DSL > > User.bind_filter(stuff).bind_group_by(stuff).bind_order > (stuff).etc.etc How much else are you going to need to wrap? You could probably get away wrapping fewer than 10 methods for your entire application. Even if you had to wrap 50 methods, wrapping isn't exactly an arduous process. > How about my other suggestion of just a global flag? > > Sequel.connect(..., :use_bind_variables => true) Well, you could have an extension that overrode filter to recognize when it is being used with named placeholders (and check that the types can be bound correctly by the database adapter), and do basically the same thing as bind_filter if so, and call super otherwise. That wouldn't work for group_by or order, since they don't deal implicitly with placeholder literal strings. Basically, your suggestion can't work without a significant amount of redesigning, which I'm unwilling to do considering: 1) My recommended approach is robust and simple to follow. 2) Redesigning will be time consuming and significantly increase the internal complexity. 3) Only a very small percentage of users would benefit from it. I'm sorry, but Sequel can't be redesigned around your needs. I understand that my approach isn't your optimal syntax, but I think the tradeoff I'm recommending is best in the long run, both for you and for Sequel in general. 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 -~----------~----~----~----~------~----~------~--~---
