On Dec 13, 1:12 pm, Olek Poplavsky <[email protected]> wrote: > Hi Jeremy > > Thank you for implementing that functionality so quickly! > > I gave it a try and it worked perfect with Postgres 8.4 and extra > option ":return => :primary_key". Great!
Good. :) > Now, without this option, where I used to get back nil, I get now an > array containing sql queries that were just executed. Not sure if this > is intentional, and it is certainly confusing. It's not intentional. If you don't specify you want the primary key returned, the return value is arbitrary. In that case, you are using the method purely for side effects, so the return value shouldn't matter. Note that I'm not opposed to returning nil in that case, so if you care enough, send in a patch with specs. > Also, I personally do not quite agree with decision to degrade > multi_insert to insert_multiple if user asked to get back primary keys > and database does not support it. It does not satisfy the 'principle > of least surprise' and most likely would create a problem that can go > unnoticed for quite some time. I would rather have Sequel throw an > exception if code asked for a feature that can not be provided by a > database. I think what you are proposing would violate the principle of least surprise. With multi_insert/import, the user is simply asking to get all primary keys back. How that happens should not be important. The only database that uses 1 query without :return=>:primary_key and multiple queries with :return=>:primary_key is MySQL, and that's because it's not possible to get reliable primary key values from MySQL using a single query. All of the databases supported by Sequel except PostgreSQL and MSSQL use a query per row for import/ multi_insert with :return=>:primary_key, so it's not like MySQL behavior. Basically, you shouldn't think of the MySQL import/ multi_insert :return=>:primary_key as degraded behavior, simply that PostgreSQL/MSSQL have optimized behavior. A simpler rhetorical question is this: Why should Sequel throw an exception for a case that it currently handles correctly? > If you do not want to throw an exception here (there are some reasons > not to like it), then maybe always returning list of ids for databases > where this is possible, and nil for databases where it is not > possible, may be a better, more reliable approach. No. Especially on a method called mostly for side effects, not doing what is requested and not raising an exception is the worst of the three options. I fail to see why you think this is a problem. multi_insert/import/ insert_multiple all do basically the same type of thing (insert all related records). What do you think is wrong with the current implementation? Please provide specific examples with code. 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.
