These are chat archives for ManageIQ/manageiq/performance

9th
Apr 2016
Keenan Brock
@kbrock
Apr 09 2016 01:14
@matthewd let me know what you think about 7813
also, there are no case or cast statements. so i'm wondering if we want to support strings.
i'll look into delegate functions
Keenan Brock
@kbrock
Apr 09 2016 03:15
@matthewd wow, this is totally working:
class VmOrTemplate 
  virtual_delegate :v_pct_free_disk_space, :v_pct_used_disk_space, :to => :hardware, :allow_nil => true
  # nothing more to do with v_pct_free_disk_space
end
class Hardware
  virtual_attribute :v_pct_free_disk_space, :float, :arel -> (t) { ... }
  def v_pct_free_disk_space
  end
end
VmOrTemplate
  .order(VmOrTemplate.arel_attribute(:v_pct_free_disk_space))
  .includes(Vm.virtual_includes(:v_pct_free_disk_space)||[])
  .references(Vm.virtual_includes(:v_pct_free_disk_space)||[])
  .take(10).map(&:name)
Keenan Brock
@kbrock
Apr 09 2016 03:23
This also works, thought using includes/references looses the "extra" parameter
VmOrTemplate \
  .select(VmOrTemplate.arel_attribute(:v_pct_free_disk_space).as("extra")) \
  .joins(Vm.virtual_includes(:v_pct_free_disk_space)||[]).first["extra"]
Matthew Draper
@matthewd
Apr 09 2016 04:46
Hmm.. I wanted the .order to work without needing the .includes
Keenan Brock
@kbrock
Apr 09 2016 04:47
no
I asked if I could botch the includes
aaah
right
you want inner queries
Im surprised that this all works so well. even putting that into the select
Matthew Draper
@matthewd
Apr 09 2016 04:49
Yeah. So the caller can treat it as Just Another Attribute, and not have to know about anything more special.
Keenan Brock
@kbrock
Apr 09 2016 04:49
@matthewd that is delegation to another model
Matthew Draper
@matthewd
Apr 09 2016 04:49
Right, but the caller doesn't know that
Keenan Brock
@kbrock
Apr 09 2016 04:49
heh
you ask for the moon
Matthew Draper
@matthewd
Apr 09 2016 04:50
Of course :)
Keenan Brock
@kbrock
Apr 09 2016 04:50
ok, let me know about the first PR
I'll look at adding arel inner selects to the second one
again - my gut is telling me that we want joins not inner selects
Matthew Draper
@matthewd
Apr 09 2016 04:50
Basically what you've got, only virtual_delegate builds a subquery in its artificial arel_attribute
Keenan Brock
@kbrock
Apr 09 2016 04:51
think it was with the internals of elastic search that they allowed you to pull out nodes from the inner document, but you couldn't get attributes from that particular entry.
I was bummed that casting was so difficult. and case is not there yet
I need to get back in there and start putting in more PRs
re compressing the return of get_order_info
there is one odd case where you can sort, but there is no sort
you're saying [] - and it will basically work
Matthew Draper
@matthewd
Apr 09 2016 04:54
Yeah.. even if the caller chooses to apply the same distinctions (and thus has to explicitly check for empty array), seems a simpler API
Keenan Brock
@kbrock
Apr 09 2016 04:57
well
if there is no sorting
and you apply a limit
it is kinda arbitrary what is displayed on the screen
@matthewd change this pr to loose the boolean, or another pr?
Matthew Draper
@matthewd
Apr 09 2016 04:58
def cast(node, sqltype); Arel::Nodes::NamedFunction.new('CAST', [Arel::Nodes::As.new(node, Arel::Nodes::SqlLiteral.new(sqltype)]); end
Keenan Brock
@kbrock
Apr 09 2016 04:58
I have to change the translation
Matthew Draper
@matthewd
Apr 09 2016 04:58
Another, I think
I realised it might chase too many new changes into this diff
Keenan Brock
@kbrock
Apr 09 2016 04:59
k
before or after "delegation"?
Matthew Draper
@matthewd
Apr 09 2016 04:59
We're currently stopping at that API boundary; no need to mess with it at the same time
Keenan Brock
@kbrock
Apr 09 2016 04:59
k
Matthew Draper
@matthewd
Apr 09 2016 04:59
(above cast is unproven, but sounds like it might work)
Keenan Brock
@kbrock
Apr 09 2016 04:59
:)
the code looks similar to what I ended up doing
Matthew Draper
@matthewd
Apr 09 2016 05:03
I have no clever suggestion to get around the lack of CASE, though :confused:
yes you do
say "I'll have them push arel 7.1 soon"
;)
anyway
Matthew Draper
@matthewd
Apr 09 2016 05:05
haha
Ah yep, nice
You can finish that particular one off with a NULLIF(expr, 0)
… assuming disk_capacity = 0 implies disk_free_space = 0
Oh, no you can't
Because the inverse isn't true
Keenan Brock
@kbrock
Apr 09 2016 05:06
it is odd
you can put attribute / 5
but not function(attribute) / 5
Matthew Draper
@matthewd
Apr 09 2016 05:07
See grouping
Keenan Brock
@kbrock
Apr 09 2016 05:07
k
looks very similar to the stuff we are doing
will be nice to delete all this miq_expression stuff and move more towards virtual attributes for a lot of this logic
am curious about a Arel::RubyVisitor ;)
Matthew Draper
@matthewd
Apr 09 2016 05:09
hahaha.. no. ;)
Keenan Brock
@kbrock
Apr 09 2016 05:09
come on!
Just because you can, doesn't mean you should
@matthewd
Matthew Draper
@matthewd
Apr 09 2016 05:09
Expression should indeed move to building Arel though, instead of SQL
Keenan Brock
@kbrock
Apr 09 2016 05:09
I dunno - did you see that refactor I passed along?
it basically made it all but disapear
(that is your work)
Matthew Draper
@matthewd
Apr 09 2016 05:10
… and this arel_attribute stuff, and its interaction with virtual columns, is key to same
Keenan Brock
@kbrock
Apr 09 2016 05:10
I'm very surprised the delegate stuff
omg
delegates worked w/ order, select
same with regular virtual columns
good call on moving the if it is a virtual column, and no arel, return nil
Matthew Draper
@matthewd
Apr 09 2016 05:11
You haven't even tried where yet :grinning:
Keenan Brock
@kbrock
Apr 09 2016 05:11
lol
I haven't tried a delegate to 2 tables away
but the delegate worked in both ruby and sql
if I change the delegates to do inner selects - you'll be more happy?
hmm. when I translate that to ruby, that will work pretty well ;)
yea - inner selects works for belongs_to and has_one, but has_many - that is going to give us problems
Matthew Draper
@matthewd
Apr 09 2016 05:13
Yeah; then the attribute will work truly stand-alone, and all callers can ignore its virtualness
Keenan Brock
@kbrock
Apr 09 2016 05:13
not from a technical point of view, but from an architectural decision
ok
Matthew Draper
@matthewd
Apr 09 2016 05:14
Yup… I think we need to flush those out, if they exist, and consider them individually
Keenan Brock
@kbrock
Apr 09 2016 05:14
so a delegate will not need a uses?
Matthew Draper
@matthewd
Apr 09 2016 05:14
c
Keenan Brock
@kbrock
Apr 09 2016 05:14
hmm
ok
Matthew Draper
@matthewd
Apr 09 2016 05:14
Err, no
It will
Keenan Brock
@kbrock
Apr 09 2016 05:14
why?
ooh - to load the class
Matthew Draper
@matthewd
Apr 09 2016 05:15
So you can includes it, and then call the ruby version
Keenan Brock
@kbrock
Apr 09 2016 05:15
so we are joining to the table, and have a sub select to the same table
granted - this is not very common, we're not sorting by every field
Matthew Draper
@matthewd
Apr 09 2016 05:15
Yup
.. and the join should eventually go away in favour of a non-references includes = second query
Keenan Brock
@kbrock
Apr 09 2016 05:16
+1
Matthew Draper
@matthewd
Apr 09 2016 05:16
Remember when I rationalized away the extra work yesterday? :)
Keenan Brock
@kbrock
Apr 09 2016 05:16
delegate can basically detect the uses. no?
Matthew Draper
@matthewd
Apr 09 2016 05:16
Or was that today? Timezones :(
Keenan Brock
@kbrock
Apr 09 2016 05:16
lol
Matthew Draper
@matthewd
Apr 09 2016 05:16
Yes
Keenan Brock
@kbrock
Apr 09 2016 05:16
it is already tomorrow for me
and it is already tomorrow's tomorrow for you
funny - I can't even remember the work you told me not to do
I remember you doing it
I guess I can search for conflate another time
Matthew Draper
@matthewd
Apr 09 2016 05:18
I rationalized the fact PG would be doing extra work because of the subquery, but that it was okay because I said so :trollface:
Keenan Brock
@kbrock
Apr 09 2016 05:19
lol
you also said to ignore the join vs include/require
and you just mentioned it again
Matthew Draper
@matthewd
Apr 09 2016 05:21
Right.. we're making choices with an awareness of what will come (so we can be short-term suboptimal). Doesn't mean we have to start implementing it now ;)
Keenan Brock
@kbrock
Apr 09 2016 05:21
fixing multi commit PRs is a pain
Matthew Draper
@matthewd
Apr 09 2016 05:22
You --fixup and autosquash?
Keenan Brock
@kbrock
Apr 09 2016 05:22
but sometimes a change is for 3 different commits
and you have to make a change in the first commit but the stuff you're changing is different in the second, so you have to make a temporary change in the first to then change in the second
maybe it is just me
Matthew Draper
@matthewd
Apr 09 2016 05:24
Ah, yeah
fixup means you do that as a conflict resolution, so it works for me as a mental gear shift (from "do a thing" to "work out how to re-apply this delta to this different source"), but definitely a pain either way
Keenan Brock
@kbrock
Apr 09 2016 05:27
it would be easier to just do stream of conscious commits (based upon time) rather than theme based
Matthew Draper
@matthewd
Apr 09 2016 05:28
tbh, yeah, that's when I'm more likely to do a new "subsequent and separate refactoring" commit, instead of threading it back into the past
Keenan Brock
@kbrock
Apr 09 2016 05:36
ugh - so attribute().as("name") works but multiplication(attribute(),5).as("name") does not
I think we're pushing arel further than it tends to go
Matthew Draper
@matthewd
Apr 09 2016 05:37
Yeah, those helper methods aren't always present
That's why I went for the "safe" option of just building the class directly
Keenan Brock
@kbrock
Apr 09 2016 05:46
Grouping adds parenthesis?
aah - changed the wrong arel
Keenan Brock
@kbrock
Apr 09 2016 06:24
for rbac - besides removing :count - any other suggestions? ManageIQ/manageiq#5844