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

Fixes #15053 - Select AR order values for uniq #3529

Closed
wants to merge 1 commit into from
Closed

Fixes #15053 - Select AR order values for uniq #3529

wants to merge 1 commit into from

Conversation

akofink
Copy link
Contributor

@akofink akofink commented May 16, 2016

Bug 15053 occurs when we call uniq on an ActiveRecord::Relation that
orders on some number of columns without explicitly selecting those
columns. This bug is found using a PostgreSQL database with the DB
error: "SELECT DISTINCT, ORDER BY expressions must appear in select
list"

@akofink akofink changed the title Refs #15053 - Add a test that replicates bug 15053 Fixes #15053 - Add a test that replicates bug 15053 May 16, 2016
Bug 15053 occurs when we call `uniq` on an ActiveRecord::Relation that
orders on some number of columns without explicitly selecting those
columns. This bug is found using a PostgreSQL database with the DB
error: "SELECT DISTINCT, ORDER BY expressions must appear in select
list"

- Add a test that replicates bug 15053
- Select the ActiveRecord::Relation `order_values` when using `uniq` to
satisfy the `select distinct` PostgreSQL requirement
@akofink akofink changed the title Fixes #15053 - Add a test that replicates bug 15053 Fixes #15053 - Select AR order values for uniq May 16, 2016
@dLobatog
Copy link
Member

@akofink Thanks for the patch! I've just tried to reproduce the same steps:

  1. Log in as admin user
  2. Create non-admin user and assign all roles to it
  3. Log out and login as the new user

I could not reproduce the issue, plus, the test in this PR passes w/o the fix included so I don't think it's testing any bug. Mind to dig a bit deeper into what's actually causing the StatementInvalid problem you saw?

Thanks!

@domcleal
Copy link
Contributor

The Redmine ticket also must be moved into the Foreman project if it is a bug in Foreman (subject to above comments).

@akofink
Copy link
Contributor Author

akofink commented May 17, 2016

@dLobatog Hm... I don't seem to be able to replicate it anymore either. Perhaps it was fixed since Friday? I'll go ahead and close this PR.

@akofink akofink closed this May 17, 2016
@akofink akofink deleted the 15053 branch July 27, 2016 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants