From 6d1b0ac74703976b7e15f25c1282601f569ea32e Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Thu, 5 Mar 2026 10:33:54 -0600 Subject: [PATCH] perf: push limit/offset to DB in PlayerService.get_players (#37) 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 --- app/services/player_service.py | 96 ++++++++++++++++++++-------------- 1 file changed, 57 insertions(+), 39 deletions(-) diff --git a/app/services/player_service.py b/app/services/player_service.py index 23ab8d8..dcd3631 100644 --- a/app/services/player_service.py +++ b/app/services/player_service.py @@ -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)