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

Support providing arbitrary SQL query generators #35

Merged
merged 1 commit into from
Apr 22, 2017

Conversation

jakecraige
Copy link
Contributor

@jakecraige jakecraige commented Mar 24, 2017

This commit provides an extension point in the search_scope definition that
allows you to define a named generator that can be used with the hash
structure to perform arbitrary SQL queries.

This allows people to query for any DB types or operators that aren't directly
supported in SearchCop, and SearchCop doesn't have to support them.

Related to #14, #28 and #31.

@mrkamel
Copy link
Owner

mrkamel commented Mar 27, 2017

Thx for this high quality PR!

Unfortunately, I can't really imagine a use case which is worth the complexity, yet - can you give a concrete example? For me, it seems, you can accomplish similar things easily via basic ActiveRecord chaining, at least because we can't use the feature in a query string directly.

MyModel.search("hello world").where("my_attribute = ?", value)

I would prefer this one instead of fighting with visitors and nodes.
Regarding #14, #28 and #31 i'm pretty sure they asking for query string support.

If we could instead "mount" this into query string queries, it would be really great.

@jakecraige
Copy link
Contributor Author

Yep, you're totally right that adding a .where to the query is one way to almost get the same feature.

The reason behind this PR is that in the app I'm working on, I'm building out a generic searching functionality using what search cop provides. I have a class that returns a hash to be parsed by the hash parser which contains the entire search. The reason I went this route vs. the where is that I wanted to have one thing, search cop, handling all the searching so that if something changes in the future, I don't have to make sure to catch ever extra .where I may have added.

I could definitely work around this problem and wrap search cop in something that would handle this, but it seemed beneficial due to the references issues wanting some custom behavior as well.

Another benefit of search cop providing this behavior is that it can be combined with other search cop hash params like :and and :or still work.

I agree that getting into visitors and notes is pretty complex and could likely be confusing for people. I'm not totally familiar with how it all works but I figured it needed to be to make sure that it works properly when composed with other things. A simpler API could look like:

Book.search({:title => {:sql => ->(attribute_name) { "#{attribute} LIKE '%Rowl%'" }}})

Would that be preferred or do you have something totally different in mind for what the API would look like?

@mrkamel
Copy link
Owner

mrkamel commented Mar 27, 2017

Yep, skipping visitors and nodes looks better to me!

Two more things to add:

  1. What about parameterizing the value?
  2. Maybe it's better to put the generator definition into the search_scope. Pros: having a re-usable name, query hash is still serializable, one place to rule it all.
search_scope :search do
  # ...

  generator :custom_match do |column_name, raw_value|
    "#{column_name} = #{quote raw_value}"
  end
end

Book.search(title: { custom_match: "value" })

What do you think?

@jakecraige
Copy link
Contributor Author

Yeah I dig that. That was similar to one of my initial ideas, except using the options method to generate it.

Is there a particular reason you're thinking we need a new method and can't reuse that one?

@mrkamel
Copy link
Owner

mrkamel commented Mar 28, 2017

Well, options specifies options for a particular attribute and i thought about this now being more or less attribute independent - due to the parameterized column_name and value.

search_scope :search do
  attribute :title, :description

  generator :custom_match do |column_name, raw_value|
    "#{column_name} = #{quote raw_value}"
  end
end

Book.search(title: { custom_match: "value1"} , description: { custom_match: "value2" })

Otherwise, this one would look like:

search_scope :search do
  attribute :title, :description

  options :title, generator: -> (column_name, raw_value) { "#{column_name} = #{quote raw_value}" }
  options :description, generator: -> (column_name, raw_value) { "#{column_name} = #{quote raw_value}" }
end

Different opinion?

@jakecraige
Copy link
Contributor Author

Nope, I hadn't considered having it apply to any attribute you wanted. I like it!

I'll probably be able to take a stab at this sometime in the next day or two.

@mrkamel
Copy link
Owner

mrkamel commented Mar 28, 2017

sounds great!

This commit provides an extension point in the `search_scope` definition that
allows you to define a named `generator` that can be used with the hash
structure to perform arbitrary SQL queries.

This allows people to query for any DB types or operators that aren't directly
supported in SearchCop, and SearchCop doesn't have to support them.

Related to mrkamel#14, mrkamel#28 and mrkamel#31.
@jakecraige jakecraige changed the title Support providing arbitrary SQL strings Support providing arbitrary SQL query generators Apr 21, 2017
@jakecraige
Copy link
Contributor Author

@mrkamel PR updated with the generator API 😄

@mrkamel mrkamel merged commit 6c01617 into mrkamel:master Apr 22, 2017
@mrkamel
Copy link
Owner

mrkamel commented Jul 12, 2017

search_cop 1.0.9 is out, including your patches.
Sorry for the delay and thanks for your great work!

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.

None yet

2 participants