Table name parsing fix for MS-SQL

[Altonymous]

This addresses the issue in determining the table name when nested select statements are used as FROM statements.

I highly suggest we revisit the statement that is generated so that nested select statements are used when limiting rows.

[kares]

great, would it be possible to get a test case for this ? we do not have windows machines and thus need to rely on people that open PRs testing things out ... try running rake test_mssql (and report your db server version) even if there were previous failures - just make sure your case passes and no additional failures were introduced with the commit. otherwise it might "rote" here open just like the other MS-SQL open pulls. would be really great to get some SQLServer related issues fixed with the next release. thanks.

[Altonymous]

I'll see what I can do! Might be later this week as we have a release tomorrow which takes priority.

[Altonymous]

I tried setting up the connection information in test/db/mssql.rb Seems unable to connection. It could be the database isn't setup correctly. However, just in case while I chase this down, is there somewhere else I need to make a change for tests to run?

[gregors]

What version of sql server are you running? Also, create the database manually. Also try connecting to the database using management studio to be sure login in working. I also added a :host => 'localhost' setting to my MSSQL_CONFIG

[Altonymous]

Needed the host.. thanks!

[Altonymous]

I reverted my changes and ran the tests and then made the change and ran the tests again.

The results can be found here in before/after files: https://gist.github.com/Altonymous/ca9006e930f95a387c5e

As you can see from the results, the very first test fails from the bug I discovered. The change resolves this issue. However, it does not fix all the other unrelated failures.

[Altonymous]

I've now added dbo. as the owner of the sysobjects and syscolumns tables. This will address the remainder of the failing tests.

In an ideal world we'd allow the user to defined owners of the different tables, but for now this will address the majority of use cases and no longer fail 100% of the time.

$ rake test_mssql
Specify AR version with `rake appraisal:{version} test_mssql' where version=(rails23|rails30|rails31|rails32|rails40)
Using ActiveRecord::VERSION = 3.2.11
Loaded suite /Users/chrisaltman/.rvm/gems/jruby-1.7.1@global/gems/rake-10.0.3/lib/rake/rake_test_loader
Started
...............................................................................
.................................

Finished in 119.676 seconds.

112 tests, 175 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

[kares]

@Altonymous this is really looking great - I'll merge this in for sure but would like to ask a few more tweaks please :

• actually the Model.first is covered by the tests thus it's irrelevant to add another tests
• but for Table sysobjects does not exist on change_column #320 I do not see a generic change_column test would be great to add one to make sure it's not reverted

please report your MS-SQL version in-case someone else lands here (not sure how compatible SQLServer versions are), thank you

[kares]

actually my second test (change_column) concern is irrelevant as well since there are MS-SQL specific tests for it ...

rebasing (and force pushing) to get rid of 1183de6 would be fine but if you're not into it I can cherry-pick the relevant (two) commits by hand ... thanks again for the fix and testing, much appreciated !

[Altonymous]

I'm not sure how to do the rebase properly. Not a git guru yet. If you care to share how, I'll do it.

[kares]

no worries than, it might be a little more work since the merge commit that does not look like a merge is there ... will be easier if I do cherry-picks. remember that you will need to reset your (master) history once it's on master here ...

for the record what's your SQL-Server version/windows version you tested this on than ? thx

[Altonymous]

I am using:
WIndows Web Server 2008 R2
SQL Server 2008 R2 - Web Edition

[Altonymous]

Going to remove this pull request and create a new Pull request after forking again.