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 DB-specific, query-related exceptions #3156

Merged
merged 2 commits into from
Apr 3, 2019

Conversation

aitbw
Copy link
Contributor

@aitbw aitbw commented Mar 29, 2019

Description

I was being greeted with the following errors when running specs specifying PostgreSQL or MySQL as database engine:

  • On PG: PG::InvalidColumnReference

This was happening because PG is extremely picky when using the SELECT and ORDER BY methods; it requires that the developer must explicitly select the attribute(s) that are going to be used to sort the query results.

  • On MySQL: Mysql2::Error: Duplicate column name

This exception was being raised because the affected code was selecting the id attribute for Spree::Product twice. (It was being already selected by the spree_products.* statement)

See the following failed builds as reference for the errors:

The issue is present from Rails v5.2.3 onwards, seems like rails/rails#35361 introduced this regression. (shoutout to @kennyadsl and @spaghetticode for pointing this out to me)

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)

@aitbw aitbw force-pushed the aitbw/fix-pg-query-error branch from 36b3e97 to cf69397 Compare March 30, 2019 19:51
@aitbw aitbw changed the title Remove redundant order clause for taxons' classifications Fix DB-specific, query-related exceptions Mar 30, 2019
@aitbw aitbw force-pushed the aitbw/fix-pg-query-error branch from cf69397 to a45d47f Compare April 1, 2019 14:11
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Left a couple of comments/questions. Thanks for working on this!

core/app/models/spree/product/scopes.rb Show resolved Hide resolved
core/app/models/spree/product/scopes.rb Outdated Show resolved Hide resolved
@aitbw aitbw force-pushed the aitbw/fix-pg-query-error branch from a45d47f to 49a3dbe Compare April 1, 2019 16:04
Copy link
Member

@spaghetticode spaghetticode left a comment

Choose a reason for hiding this comment

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

👍 thank you @aitbw for this much needed fix!

I've left a minor comment, hope it makes sense

core/app/models/spree/product/scopes.rb Outdated Show resolved Hide resolved
aitbw added 2 commits April 3, 2019 10:21
PostgreSQL is very picky when building a SQL query that requires
SELECT and ORDER BY methods; the attribute you want to use to sort
the results must be explicitly selected; otherwise, PG raises an
exception
The SQL queries specified on said helper on lines 10 and 15 were
redundantly selecting the `id` attribute for `spree_products` table,
which caused MySQL to raise an exception about a duplicate column
name since said attribute is already being selected through the
`spree_products.*` statement
@aitbw aitbw force-pushed the aitbw/fix-pg-query-error branch from 49a3dbe to 3db4c93 Compare April 3, 2019 14:22
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks @aitbw !

@kennyadsl
Copy link
Member

@solidusio/core-team can we have another review here, please? This issue is making all other PRs fail.

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.

5 participants