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 #12194 - join fact tables multiple times for each search term #2906

Closed
wants to merge 1 commit into from

Conversation

domcleal
Copy link
Contributor

scoped_search usually generates a new inner join for each search term
when searching through a key/value table layout to correctly search for
hosts via multiple facts. Since the change to ext_method in 3f8e6c3, a
fixed table name was used. This is changed to multiple joins to match
how scoped_search works with multiple search terms.


private

cattr_accessor :fact_values_table_counter
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm unsure if this is better or worse than scoped_search's randomness: https://github.com/wvanbergen/scoped_search/blob/v3.2.2/lib/scoped_search/query_builder.rb#L298

@ohadlevy
Copy link
Member

👍 while I assume this will work, but wont it better to solve it in scoped search?

@domcleal
Copy link
Contributor Author

👍 while I assume this will work, but wont it better to solve it in scoped search?

I don't think there's a directly applicable fix for scoped_search. Using ext_method replaces all of the "to_sql" mechanism in its query builder, which includes the magic that constructs queries across key/value tables. If we use ext_method at all, we're entirely responsible for passing back conditions, joins, keyparameters, keyconditions etc - there's no compromise that I can see.

If I were to change anything in scoped_search, it'd be to somehow make some of those helpers easily reusable, but it'd probably be quite invasive.

@ohadlevy
Copy link
Member

👍 then, waiting for tests.

scoped_search usually generates a new inner join for each search term
when searching through a key/value table layout to correctly search for
hosts via multiple facts.  Since the change to ext_method in 3f8e6c3, a
fixed table name was used.  This is changed to multiple joins to match
how scoped_search works with multiple search terms.
@ohadlevy
Copy link
Member

merged as 57e9d8a thanks @domcleal

@ohadlevy ohadlevy closed this Nov 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants