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.

Sign up for GitHub

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

Jump to bottom

Exclude Service::AGGREGATE_ALL_VM_ATTRS from MiqExp.to_sql #16915

Merged
merged 2 commits into from
Jan 31, 2018
Merged

Exclude Service::AGGREGATE_ALL_VM_ATTRS from MiqExp.to_sql #16915

merged 2 commits into from
Jan 31, 2018

Commits on Jan 31, 2018

  1. Use Field in MiqExpression.excluded_col_for_expression 

    Instead of passing just the column name into
.excluded_col_for_expression, pass the entire field object into the
method so that the model can also be referenced if needed (will be used
in a future commit).

This method is only used where it has been edited here in our entire
codebase, so this should be a relatively safe change.  I also don't see
this as something that would called in an automate script.
    @NickLaMuro
    NickLaMuro committed Jan 31, 2018
    Configuration menu
    Copy the full SHA
    e7955bf View commit details
    Browse the repository at this point in the history

  2. Exclude Service::AGGREGATE_ALL_VM_ATTRS from MiqExp.to_sql 

    These calls, while they are Arel and could be converted, would most
likely be used in "filter" type scenarios, and would not be good
candidates for that since they would perform horribly when used on more
than one record at a time (which filtering would do that).

Instead of preventing these records from showing up as a filter option,
just make sure they can't be used with SQL when they are.  That said,
they will cause N+1 hell, so they really shouldn't be filtered on...
but...

¯\(°_o)/¯
    @NickLaMuro
    NickLaMuro committed Jan 31, 2018
    Configuration menu
    Copy the full SHA
    0d32b87 View commit details
    Browse the repository at this point in the history