I am working a asp .net core web API with EF core. I wrote this query. But this take 20-30seconds to execute.
Anyone have idea to improve this query.
var hotels = await _context.Hotels
.Where(i => (i.DestinationCode == request.Destination))
.Select(i => new HotelListHotelVm
{
Item1 = i.Item1,
Item2 = i.Item2,
Item3 = i.Item3,
Item4Code = i.Item4Code,
Item4Description = i.Item4.TypeDescription,
Item5 = i.Item5.Select(x => new HotelListHotelVm.HotelListItem5Vm
{
Code = x.Item5Code,
Description = x.Item5.Description,
}).Where(x =>(incomingItem5s.Length > 0 ) ? (incomingItem5s.Contains(x.Code)) : (x.Code != "")),
Item6 = i.Item6.Select(x => new HotelListHotelVm.HotelListHotelItem6Vm
{
Id = x.Id,
Item6TypeCode = x.Item6TypeCode,
Order = x.Order,
Path = x.Path,
VisualOrder = x.VisualOrder,
}).Take(3),
HotelFacilities = i.Facilities.ToList().Distinct().Take(6).Select(x => new HotelListHotelVm.HotelListFacilityVm {
Id = x.Id,
FacilityGroupCode = x.FacilityGroupCode,
HotelFacilityGroupDescription = x.FacilityGroup.Description,
FacilityCode = x.FacilityCode
}),
})
.Where( i => ((incomingItem4.Length > 0 ) ? (incomingItem4.Contains(i.Item4Code)) : (i.Item4Code != "")) )
.OrderByDescending(i => i.Code)
.PaginatedListAsync(request.PageNumber, request.PageSize);
foreach( var item in hotels.Items){
foreach(var facility in item.HotelFacilities){
foreach( var fac in _context.Facilities){
if(facility.FacilityCode == fac.Code){
facility.HotelFacilityDescription = fac.Description;
}
}
}
}
I f I remove those foreach code, The query takes 8-10s to execute.
But I need those foreach codes. Because I need the HotelFacilityDescription
Any suggestion for optimize the query ?
CodePudding user response:
_context.Facilities
will be enumerated (i.e. database will be called) for every iteration of previous loops. The quick fix is to call it ones and store results in variable:
var facilities = _context.Facilities.ToList();
foreach( var item in hotels.Items){
foreach(var facility in item.HotelFacilities){
foreach(var fac in facilities){
if(facility.FacilityCode == fac.Code){
facility.HotelFacilityDescription = fac.Description;
}
}
}
}
Next improvement can be converting facilities
into Dictionary
for searching purposes.
Even better approach can be writing query joining with _context.Facilities
on database side (but here more info needed).
CodePudding user response:
I've read this a couple times, but it looks like the relationship for Hotel.Facilities is a Facility, so could you not just do:
HotelFacilities = i.Facilities.ToList().Distinct().Take(6).Select(x => new HotelListHotelVm.HotelListFacilityVm {
Id = x.Id,
FacilityGroupCode = x.FacilityGroupCode,
HotelFacilityGroupDescription = x.FacilityGroup.Description,
FacilityCode = x.FacilityCode,
HotelFacilityDescription = x.Description
}),
If for some reason Hotel.Facilities is not pointing at a Facility, but is a Many-to-Many HotelFacilityGroup entity to a FacilityGroup, that also contains a FacilityCode, if the associated FacilityGroup has access to a set of Facilities beneath it you could leverage that:
Edit: It sounds like multiple Facilities share the same Code where some may have a null description. Provided that the facilities matching the code would be within the same facility group and not consider the same Code within different facility groups. If you need to match the code across all facilities then there probably isn't much of an alternative to loading the entire set of facility codes & descriptions.
HotelFacilities = i.Facilities.ToList().Distinct().Take(6).Select(x => new HotelListHotelVm.HotelListFacilityVm {
Id = x.Id,
FacilityGroupCode = x.FacilityGroupCode,
HotelFacilityGroupDescription = x.FacilityGroup.Description,
FacilityCode = x.FacilityCode,
HotelFacilityDescription = x.FacilityGroup.Facilities.Where(f => f.Code == x.FacilityCode && f.Description != null).Select(f => f.Description).FirstOrDefault()
}),
That would avoid the need to go load all of the facilities to resolve that code. Otherwise, if you do need to fetch across all facilities, pre-loading them would be the way to go, but rather than fetching the entire Facility entity I would recommend just the values you need, the Code and the Description. This cuts down on the amount of memory needed and potentially be a faster query:
var facilities = _context.Facilities
.Select(f => new
{
f.Code,
f.Description
}).ToList();
Edit: From there, finding a match using:
foreach( var facility in hotels.Items.SelectMany(x => x.HotelFacilities)
{
facility.HotelFaciltyDescription = facilities
.Where(x => x.Code == facility.FacilityCode
&& !string.IsNullOrEmpty(x.Description)
.Select(x => x.Description)
.FirstOrDefault();
}
I would recommend an OrderBy clause to ensure the selection of the facility is predictable as it sounds like there could be multiple matches on a code with a non-null description.