bglusman
So I have a gnarly query in my open source Phoenix app I was trying to make dramatically more efficient.. in its current form it brings in a lot of unnecessary data over the wire, so I managed to get a raw SQL form that only gets the data I need, and tried translating it into Ecto, and got stuck with some errors related to group_by vs select I think… the purpose is to get the data we need, which is stocks that have a “path” to any of the credit_types in the database, grouped by which credit type(s) they have a path to (most only have one path, but some have 2, and in that case we want the stock duplicated and to appear in a column for both credit types)… in it’s current form (in Ecto, before attempted refactor) it works but it pulls every food in the database also, because they all relate to one or more credit types, but we only care about “stocked” foods. Current form goes from credit_type “toward” stocks, the refactor works by going from stocks “toward” credit type, and grouping stocks by credit type…
Here is the raw SQL version:
select stock.id, food.long_desc, food.manufacturer_name, food_group.foodgroup_desc,
credit_type.name
from facilities facility
inner join stocks stock on stock.facility_id = facility.id
inner join foods food on food.id = stock.food_id
inner join credit_type_memberships ctm on ctm.food_group_id = food.food_group_id
inner join credit_types credit_type on credit_type.id = ctm.credit_type_id
where facility.id = 1
group by stock.id, food.long_desc, food.manufacturer_name, food_group.foodgroup_desc,
credit_type.name
And rather than paste the non-working ecto-translation of it here, I’ll link in context to my best/closest attempt I think I got to, in the branch I was playing with it on… the play function here was accidentally committed ages ago while trying to solve the same problem, before I had a working SQL version of it, just to make it easy to test the query in iex… the stock_by_type function above it is the actual query from the app as currently used. You can probably answer any question from the code in the link there, (and original form of that query before the WIP/play commit above is actually changed here for reference) but also happy to provide context if anything is unclear/harder to find an answer to than you’d like, but you’re otherwise interested in helping with the refactor… Thanks all!
Trending in Questions
Other Trending Topics
Categories:
Sub Categories:
Forums
Popular Tags
- #ecto
- #liveview
- #troubleshooting
- #learning-elixir
- #library
- #deployment
- #erlang
- #testing
- #genserver
- #mix
- #absinthe
- #remote-other
- #otp
- #plug
- #how-to-question
- #macros
- #postgres
- #elixirconf
- #channels
- #exunit
- #discussion
- #code-sync
- #podcasts
- #javascript
- #onsite
- #dialyzer
- #docker
- #authentication
- #umbrella
- #full-time-contract
- #podcasts-by-brainlid
- #ecto-query
- #elixirconf-us
- #ai
- #blog-post
- #elixir-ls
- #phoenix_html
- #iex
- #graphql
- #genstage
- #websockets
- #supervisor
- #advent-of-code
- #distillery
- #processes
- #api
- #forms
- #hex
- #security
- #metaprogramming










Showing Posts 29 to 20- Show Best Posts
- Show All (oldest first)
- Show All (newest first)
bglusman
I put this to the side after we came full circle back to probably using group_by, but I opened a github issue for it, and a contributor appears to have solved the problem differently than I was trying to, but it seems to be OK, and certainly better than it was. PR here:
https://github.com/openpantry/open_pantry/pull/152
kip
Agree @cmkarlsson, not complex at all - bad characterisation on my part.
bglusman
Hah, full circle
and yes, they do, not a huge number, but enough…
OvermindDL1
This might be a good use for group_by then. It is likely happening because multiple stocks join to the same/multiple credits.
bglusman
Oh, cool, I was trying to select on that before but was getting an error about the binding not matching, but now I have that working, though now it seems to give me duplicate credit types somehow instead of duplicate stocks within a single credit type… this is the current query, which is maybe promising but it seems slower than it was before:
Not totally sure at a glance if it’s pulling all foods form DB now or not… my guess is that it is, but, harder to tell than it was in the shape it was in before
As far as duplicates thing, if I pipe above query to map name:
|> Enum.map(&(&1.name))then with only 3 credit types I get 6 names,
["Veggienoms", "Carbnoms", "Proteinoms", "Proteinoms", "Proteinoms", "Proteinoms"]which must be, like, the 3 or 4 foods in food groups that are both proteins and veggies or something I think, some of them repeating? Hard to reason about at a glance, but sort of seems like progress… merging those together into one struct per name/id wouldn’t be so bad though if it is avoiding pulling all the food, but like I said, at a glance I think the query is now slower by like 100% or so than the old versionOvermindDL1
At the very least to start with if you want to return a list of CreditTypes then you should add a
select:statement doing so from yourfood_credit_typesjoin.bglusman
Also this isn’t urgent @OvermindDL1 so please, appreciate all your help but feel free to check back in in a day or 3 if busy now but interested enough to help later! I appreciate the input and thoughts!
bglusman
There won’t be a TON of overlap, but where there is we want it… mostly its about things like beans/legumes which are both a veggie and a protein, in our specific use case, and a few other foods that cross over some boundaries so we allow both credit types to be used for them… I got the Ecto query working as I expected, and like I said, now it’s a list of stocks
Here’s a workign query I’d like to turn “inside out”:
OvermindDL1
Once we talk in a thread then by default we get notified on updates on it.
I’m still a bit short on time, but quick question, since you want the CreditType structs, do you expect a lot of the same stocks and foods structs too, or will they all be pretty much distinct? Which answer determines which is the best way to do it.
bglusman
that wasn’t done as a reply to any of you so you may not see it.. @OvermindDL1 / @cmkarlsson / @kip