rails-sqlserver / activerecord-sqlserver-adapter

SQL Server Adapter For Rails
MIT License
975 stars 563 forks source link

ActiveRecord::StatementInvalid (TinyTds::Error: Incorrect syntax near ')' #218

Closed yosadchuk closed 12 years ago

yosadchuk commented 12 years ago

rails 3.2.3 tiny_tds 0.5.1 freetds 0.91 Microsoft SQL Server 2008 R2

Hi! I've encountered this error, while trying to add new record(s) (along with updating already existed) to database using possibilities of gem 'awesome_nested_fields' https://github.com/lailsonbm/awesome_nested_fields. Full error message including invalid SQL looks like:

As You can see, there's extra comma(s) at the end of query, that makes it invalid. I've forked this repository and made small in hotfix branch here https://github.com/yosadchuk/activerecord-sqlserver-adapter.

Fix: I've added sql = sql.gsub(/\'(, )+)$/, "')") before query is executed, as can be seen on this commit https://github.com/yosadchuk/activerecord-sqlserver-adapter/commit/6d9e7a711f3db7455e33a5be5f5a86abad74a0d5 but it's rather quick.

Maybe there's another way to solve this problem?

metaskills commented 12 years ago

There has to be a better way because that technique will not be added. This issue needs to be distilled down to an understanding of weather ActiveRecord itself endorses what that gem is doing. At a quick glance, it appears that it is passing down an empty string as a primary key, is this correct? If indeed it relies on that behavior, then I need to see how ActiveRecord endorses that at a test (which we are passing all of them). It is likely that this gem is relying on unexpected behavior that is not documented or guaranteed, for instance... calling #to_i on an empty string to produce 0.

I'll take a look at this sometime soon, but if you have some time, please investigate more.

metaskills commented 12 years ago

This issue seems to relate back to this old ticket #164. Will close this and work on that one.