Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue when using joins with distinct #590

Closed
samu opened this issue Oct 7, 2014 · 7 comments
Closed

Issue when using joins with distinct #590

samu opened this issue Oct 7, 2014 · 7 comments
Labels

Comments

@samu
Copy link

samu commented Oct 7, 2014

We have a basic user to roles mapping with a roles_users table in between.

User.joins(:roles_users).distinct.where(id: 1).first

would cause the error

ActiveRecord::JDBCError: The multi-part identifier "users.id" could not be bound

while

User.joins(:roles_users).where(id: 1).first

works fine.

We are using an MSSQL Database, therefore we use the activerecord-jdbcmssql-adapter (newest version). Here are the two sql statements that are being generated:

cruby:

EXEC sp_executesql N'SELECT DISTINCT TOP (1) [users].* 

FROM [users] INNER JOIN [roles_users] ON [roles_users].[user_id] = [users].[id] AND [roles_users].[deleted_at] IS NULL WHERE [users].[first_name] = N''abc'

' ORDER BY [users].[id] ASC'

jruby:

SELECT t.* FROM ( SELECT ROW_NUMBER() OVER(ORDER BY [users].[id] ASC) AS _row_num, t.* FROM (SELECT  DISTINCT [users].* 

FROM [users] INNER JOIN [roles_users] ON [roles_users].[user_id] = [users].[id] AND [roles_users].[deleted_at] IS NULL WHERE [users].[first_name] = N'abc'

) AS t ) AS t WHERE t._row_num BETWEEN 1 AND 1

We have tried it with different jruby versions: jruby-1.7.9 and jruby-1.7.13 on Rails 4.1.1 and Rails 4.1.6.

you can support MS-SQL fixes at BountySource

@samu samu changed the title Issue with when using default_scope Issue when using default_scope Oct 7, 2014
@samu samu changed the title Issue when using default_scope Issue when using joins with distinct Oct 7, 2014
@kares kares added the mssql label Oct 16, 2014
@kares
Copy link
Member

kares commented Oct 16, 2014

Thanks Sam, I unfortunately do not have the time to look into MS-SQL but I'm happy to help out when someone does ... please consider looking into it on your own. We're low on (SQLServer) maintainers, there are several issues open.

@samu
Copy link
Author

samu commented Oct 16, 2014

Thanks @kares for the reply. I'd like to look into this myself, however i don't really know where to start. Could you give me some hints maybe? I need a starting point in the code, as i'm very unfamiliar with it as of now.

@kares
Copy link
Member

kares commented Oct 16, 2014

sure, here you go:

@samu
Copy link
Author

samu commented Oct 21, 2014

@kares I investigated on this and i think the cause of the problem is on this line:

order = order.gsub(/([a-z0-9_])+\./, 't.')

The problem is that the order by sql looks like this

ORDER BY [users].[id] ASC

and the line of code i referenced is supposed to change it to this

ORDER BY t.[id] ASC

However, the REGEX used does not contain [ and ]. Changing the original line

order = order.gsub(/([a-z0-9_])+\./, 't.')

to this

order = order.gsub(/([\[\]a-z0-9_])+\./, 't.')

solves the problem.

I'm not sure if this is a sufficient solution as i'm having a few troubles running the test suite (several tests are failing due to problems that are unrelated to this change). Do you want me to create a pull request anyways?

@kares
Copy link
Member

kares commented Oct 21, 2014

@samu sure, if there are no new failures introduced and you can cover your case with a test I'll merge it.

@samu
Copy link
Author

samu commented Oct 24, 2014

@kares do i understand this correctly that the tests for mssql are only being run locally and not in CI?

@kares
Copy link
Member

kares commented Oct 25, 2014

@samu yes, you do not worry about travis-ci failures unless you touch some shared (non-specific) parts ...
I'll take your word for not seeing any new failures before and after, until someone else says smt else :) !

falti added a commit to falti/activerecord-jdbc-adapter that referenced this issue Dec 8, 2014
kares pushed a commit that referenced this issue Sep 9, 2015
@falti falti mentioned this issue Sep 9, 2015
@kares kares closed this as completed Sep 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants