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

Fix ambiguous column error when joining some relations #180

Merged
merged 3 commits into from
May 2, 2016

Conversation

natw
Copy link
Contributor

@natw natw commented Dec 31, 2015

Joining the relation returned by #higher_items or #lower_items to a table also having position_column would result in an ambiguous column error.

The new test file might be a bit heavy handed, but test_list.rb is somewhat...opaque. :)

The test cases are pretty close to my actual use case, and I admit that it's a slightly awkward one.
I have a visible scope on Item that joins on Section, but in the case of the query set returned by lower_items, it's already scoped to a particular Section.
So I could just say .where(visible: true) instead of .visible, which would avoid the unnecessary join, and I'll probably just do that in the meantime.

Still, I think this is probably a change for the better.

@@ -0,0 +1,64 @@
require 'helper'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@brendon
Copy link
Owner

brendon commented Apr 17, 2016

@natw, could you use quoted_table_name as per: #174 and rebase. Looks good otherwise.

@natw natw force-pushed the ambiguous_column branch from 030fdcb to 9bd36bf Compare April 21, 2016 16:12
@natw
Copy link
Contributor Author

natw commented Apr 22, 2016

done

@brendon
Copy link
Owner

brendon commented Apr 27, 2016

Thanks for that. I think there are some inconsistencies now regarding the use of quoted_position_column and position_column in queries.

Can you use quoted_position_column instead of position_column?

Nathaniel Williams added 3 commits May 2, 2016 09:40
Joining the relation returned by `#higher_items` or `#lower_items`
to a table also having `position_column` would result in an
ambiguous column error
@natw natw force-pushed the ambiguous_column branch from 9bd36bf to c01f1c7 Compare May 2, 2016 14:46
@natw
Copy link
Contributor Author

natw commented May 2, 2016

there you go. Sorry for the slow response.

@brendon brendon merged commit 7753978 into brendon:master May 2, 2016
@brendon
Copy link
Owner

brendon commented May 2, 2016

Thanks for that :) I've merged that now for you. Expect a new gem in the next week or two once a few other issues get closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants