perf: push limit/offset to DB in PlayerService.get_players (#37)
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m26s
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m26s
Apply .offset() and .limit() on the Peewee query before materializing results, instead of fetching all rows into memory and slicing in Python. Total count is obtained via query.count() before pagination is applied. In-memory (mock) queries continue to use Python-level slicing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
ddf5f77da4
commit
99828893c2
@ -39,7 +39,7 @@ class PlayerService(BaseService):
|
||||
cache_patterns = ["players*", "players-search*", "player*", "team-roster*"]
|
||||
|
||||
# Deprecated fields to exclude from player responses
|
||||
EXCLUDED_FIELDS = ['pitcher_injury']
|
||||
EXCLUDED_FIELDS = ["pitcher_injury"]
|
||||
|
||||
# Class-level repository for dependency injection
|
||||
_injected_repo: Optional[AbstractPlayerRepository] = None
|
||||
@ -135,17 +135,21 @@ class PlayerService(BaseService):
|
||||
# Apply sorting
|
||||
query = cls._apply_player_sort(query, sort)
|
||||
|
||||
# Convert to list of dicts
|
||||
players_data = cls._query_to_player_dicts(query, short_output)
|
||||
|
||||
# Store total count before pagination
|
||||
total_count = len(players_data)
|
||||
|
||||
# Apply pagination (offset and limit)
|
||||
if offset is not None:
|
||||
players_data = players_data[offset:]
|
||||
if limit is not None:
|
||||
players_data = players_data[:limit]
|
||||
# Apply pagination at DB level for real queries, Python level for mocks
|
||||
if isinstance(query, InMemoryQueryResult):
|
||||
total_count = len(query)
|
||||
players_data = cls._query_to_player_dicts(query, short_output)
|
||||
if offset is not None:
|
||||
players_data = players_data[offset:]
|
||||
if limit is not None:
|
||||
players_data = players_data[:limit]
|
||||
else:
|
||||
total_count = query.count()
|
||||
if offset is not None:
|
||||
query = query.offset(offset)
|
||||
if limit is not None:
|
||||
query = query.limit(limit)
|
||||
players_data = cls._query_to_player_dicts(query, short_output)
|
||||
|
||||
# Return format
|
||||
if as_csv:
|
||||
@ -154,7 +158,7 @@ class PlayerService(BaseService):
|
||||
return {
|
||||
"count": len(players_data),
|
||||
"total": total_count,
|
||||
"players": players_data
|
||||
"players": players_data,
|
||||
}
|
||||
|
||||
except Exception as e:
|
||||
@ -204,9 +208,9 @@ class PlayerService(BaseService):
|
||||
p_list = [x.upper() for x in pos]
|
||||
|
||||
# Expand generic "P" to match all pitcher positions
|
||||
pitcher_positions = ['SP', 'RP', 'CP']
|
||||
if 'P' in p_list:
|
||||
p_list.remove('P')
|
||||
pitcher_positions = ["SP", "RP", "CP"]
|
||||
if "P" in p_list:
|
||||
p_list.remove("P")
|
||||
p_list.extend(pitcher_positions)
|
||||
|
||||
pos_conditions = (
|
||||
@ -245,9 +249,9 @@ class PlayerService(BaseService):
|
||||
p_list = [p.upper() for p in pos]
|
||||
|
||||
# Expand generic "P" to match all pitcher positions
|
||||
pitcher_positions = ['SP', 'RP', 'CP']
|
||||
if 'P' in p_list:
|
||||
p_list.remove('P')
|
||||
pitcher_positions = ["SP", "RP", "CP"]
|
||||
if "P" in p_list:
|
||||
p_list.remove("P")
|
||||
p_list.extend(pitcher_positions)
|
||||
|
||||
player_pos = [
|
||||
@ -385,19 +389,23 @@ class PlayerService(BaseService):
|
||||
# This filters at the database level instead of loading all players
|
||||
if search_all_seasons:
|
||||
# Search all seasons, order by season DESC (newest first)
|
||||
query = (Player.select()
|
||||
.where(fn.Lower(Player.name).contains(query_lower))
|
||||
.order_by(Player.season.desc(), Player.name)
|
||||
.limit(limit * 2)) # Get extra for exact match sorting
|
||||
query = (
|
||||
Player.select()
|
||||
.where(fn.Lower(Player.name).contains(query_lower))
|
||||
.order_by(Player.season.desc(), Player.name)
|
||||
.limit(limit * 2)
|
||||
) # Get extra for exact match sorting
|
||||
else:
|
||||
# Search specific season
|
||||
query = (Player.select()
|
||||
.where(
|
||||
(Player.season == season) &
|
||||
(fn.Lower(Player.name).contains(query_lower))
|
||||
)
|
||||
.order_by(Player.name)
|
||||
.limit(limit * 2)) # Get extra for exact match sorting
|
||||
query = (
|
||||
Player.select()
|
||||
.where(
|
||||
(Player.season == season)
|
||||
& (fn.Lower(Player.name).contains(query_lower))
|
||||
)
|
||||
.order_by(Player.name)
|
||||
.limit(limit * 2)
|
||||
) # Get extra for exact match sorting
|
||||
|
||||
# Execute query and convert limited results to dicts
|
||||
players = list(query)
|
||||
@ -468,19 +476,29 @@ class PlayerService(BaseService):
|
||||
# Use backrefs=False to avoid circular reference issues
|
||||
player_dict = model_to_dict(player, recurse=recurse, backrefs=False)
|
||||
# Filter out excluded fields
|
||||
return {k: v for k, v in player_dict.items() if k not in cls.EXCLUDED_FIELDS}
|
||||
return {
|
||||
k: v for k, v in player_dict.items() if k not in cls.EXCLUDED_FIELDS
|
||||
}
|
||||
except (ImportError, AttributeError, TypeError) as e:
|
||||
# Log the error and fall back to non-recursive serialization
|
||||
logger.warning(f"Error in recursive player serialization: {e}, falling back to non-recursive")
|
||||
logger.warning(
|
||||
f"Error in recursive player serialization: {e}, falling back to non-recursive"
|
||||
)
|
||||
try:
|
||||
# Fallback to non-recursive serialization
|
||||
player_dict = model_to_dict(player, recurse=False)
|
||||
return {k: v for k, v in player_dict.items() if k not in cls.EXCLUDED_FIELDS}
|
||||
return {
|
||||
k: v for k, v in player_dict.items() if k not in cls.EXCLUDED_FIELDS
|
||||
}
|
||||
except Exception as fallback_error:
|
||||
# Final fallback to basic dict conversion
|
||||
logger.error(f"Error in non-recursive serialization: {fallback_error}, using basic dict")
|
||||
logger.error(
|
||||
f"Error in non-recursive serialization: {fallback_error}, using basic dict"
|
||||
)
|
||||
player_dict = dict(player)
|
||||
return {k: v for k, v in player_dict.items() if k not in cls.EXCLUDED_FIELDS}
|
||||
return {
|
||||
k: v for k, v in player_dict.items() if k not in cls.EXCLUDED_FIELDS
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def update_player(
|
||||
@ -603,12 +621,12 @@ class PlayerService(BaseService):
|
||||
flat_player = player.copy()
|
||||
|
||||
# Flatten team object to just abbreviation
|
||||
if isinstance(flat_player.get('team'), dict):
|
||||
flat_player['team'] = flat_player['team'].get('abbrev', '')
|
||||
if isinstance(flat_player.get("team"), dict):
|
||||
flat_player["team"] = flat_player["team"].get("abbrev", "")
|
||||
|
||||
# Flatten sbaplayer object to just ID
|
||||
if isinstance(flat_player.get('sbaplayer'), dict):
|
||||
flat_player['sbaplayer'] = flat_player['sbaplayer'].get('id', '')
|
||||
if isinstance(flat_player.get("sbaplayer"), dict):
|
||||
flat_player["sbaplayer"] = flat_player["sbaplayer"].get("id", "")
|
||||
|
||||
flattened_players.append(flat_player)
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user