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 #118 against 4-0-stable #239

Closed
wants to merge 3 commits into from

Conversation

jsanders
Copy link

Fix for #118 against 4-0-stable.

This is really mostly @ofavre's work from awhile ago, I just updated it for 4-0-stable.

ofavre and others added 3 commits January 15, 2014 10:22
Fixes rails#118
Fixes rails#98

Conflicts:
	lib/arel/nodes/and.rb
	lib/arel/nodes/select_statement.rb
	lib/arel/visitors/mssql.rb
	lib/arel/visitors/to_sql.rb
@ofavre
Copy link

ofavre commented Jan 19, 2014

Once merged, don't forget to update the code at yakaz/rails@29b8ebd (relation-unions-v3.2.2 branch), in order to resolve rails/rails#939!
EXCEPT and INTERSECT can be exposed too.
Thanks for taking this forward!

@ghost ghost assigned rafaelfranca Jan 19, 2014
@bithive
Copy link

bithive commented Mar 18, 2014

Thanks for your work on this @ofavre and @jsanders. I would love to see this get merged, as it is very helpful in one of my projects.

This may be unrelated but in using this new functionality I've noticed parameters don't seem to get bound when using associations in the method chain. I am working around this by writing e.g. Item.where(catalog: self) where I would normally just write items otherwise I get ActiveRecord::StatementInvalid: PG::UndefinedParameter: ERROR: there is no parameter $1

@jsanders
Copy link
Author

@bithive: Hmm, haven't seen that... How do you have your association set up?

@bithive
Copy link

bithive commented Mar 18, 2014

Hopefully this will make sense. It's not a big deal, and perhaps just a gap in my understanding of how to use Arel properly. I know what I am doing is unusual, but there is a reason.

create_table :values do |t|
  t.string :type,  null: false
  t.text   :value, null: true

  t.references :field, null: false
  t.references :item
  t.timestamps
end

class Value < ActiveRecord::Base
  belongs_to :field
  belongs_to :item
end

class Item < ActiveRecord::Base
  belongs_to :catalog
  has_many :values
end

class Catalog < ActiveRecord::Base
  has_many :items
  has_many :values, through: :items

  # This works for me
  def where attributes
    values_table = Value.arel_table

    queries = attributes.map do |key, value|
      Item.where(catalog: self).joins(:values).where(
        values_table[:field_id].eq(field_ids_from_tokens[key.to_s])
        .and(values_table[:value].eq(value))
      )
    end

    Item.find_by_sql queries.inject(&:intersect).to_sql
  end

  # This doesn't work
  def terse_where attributes
    queries = attributes.map do |key, value|
      values.where(field_id: field_ids_from_tokens[key.to_s], value: value)
    end

    Item.find_by_sql queries.inject(&:intersect).to_sql
  end
end

In the console first method generates SQL containing "items"."catalog_id" = 1 and succeeds while the latter generates "items"."catalog_id" = $1 and then complains about the undefined parameter.

@tamird
Copy link

tamird commented Sep 16, 2014

did this ever actually get fixed on master?

@jsanders
Copy link
Author

@tamird: No. I'm working on updating this patch for 5.0.1 (so that it can be used with rails 4.1) as we speak.

@tamird
Copy link

tamird commented Sep 26, 2014

@jsanders I suggest writing the patch against master first

@jsanders
Copy link
Author

@tamird: Took your advice :)

Closing this in favor of #320.

@jsanders jsanders closed this Sep 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants