On Dec 14, 6:06 am, Olek Poplavsky <[email protected]> wrote: > On Dec 13, 4:44 pm, Jeremy Evans <[email protected]> wrote: > > > > 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. > > That would be more or less correct if Sequel was part of the "in-house- > app". > It is not, it is public API, and some people will start to rely on > those queries returned, and then when this changes in some newer > version of the Sequel, their code will stop working. I understand that > this is arguable either way, so lets agree to disagree :)
There are many places in Sequel where the return value is not specifically chosen. For a method called for side-effects, unless the return value is specifically documented, you should not assume that it will remain the same. However, if it is changed, it will be documented in the release notes. Still, like I said, I'm happy to consider patches with specs and docs that will make the return value part of the API. > > A simpler rhetorical question is this: Why should Sequel throw an > > exception for a case that it currently handles correctly? > > It depends on the definition of 'correctness' in the first place :) > I do not think it is correct to insert records one-by-one if it is > part of the 'contract' of method multi-insert to insert all data in > one step. Except that that's not part of the contract of multi_insert/import. From the import method's RDoc: # Inserts multiple records into the associated table. This method can be # used to efficiently insert a large number of records into a table in a # single query if the database supports it. The if clause at the end is important. The method never guarantees a single query, merely that it will attempt to a single query if it can do so. In the MySQL case, it can do so only if you don't request the primary key be returned. > > 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. > > I guess you just wrote the main reason of my dissatisfaction with this > api: both methods insert_multiple and multi_insert do pretty much same > thing, and their names are just artificially different. The only > distinction for them is that one of them will generate list of > inserts, and the other one will ATTEMPT to perform one SQL "to do it > all". There are just too many cases where they both will do EXACT same > thing (generate multiple insert statements), and it makes things > confusing. Ruby is full of method aliases, giving you more than one way to do things. In this case, both multi_insert and insert_multiple have been around since before I took over maintenance of Sequel. They do have slightly different APIs, multi_insert is not just a faster version of insert_multiple (insert_multiple accepts a block). > Maybe it would be better (as is in 'less confusing') if there was just > one method for insertion of multiple records at a time, and that > method would take options that would try to "hint" on how that should > be done, without any guarantees? Well, I certainly could combine multi_insert and insert_multiple into a single method, making multi_insert support the block as insert_multiple does. But I'd still have to keep insert_multiple around for backwards compatibility. Also, there may be a reason why a user wants to insert each row with a separate insert query on MySQL, PostgreSQL, and MSSQL. Combining the methods would no longer allow that. So I don't see a benefit to your proposal. > Again, current API is functional, it is just a bit vague because of a > 'mushy' contract of multi_insert method. Welcome to the world where not all databases support exactly the same set of features. :) 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.
