perf(list): optimize bd list --json to fetch only needed dependencies (#1316)
Add GetDependencyRecordsForIssues method to storage interface that fetches dependencies only for specified issue IDs instead of all dependencies in the database. This optimizes bd list --json which previously called GetAllDependencyRecords() even when displaying only a few issues (e.g., bd list --limit 10). - Add GetDependencyRecordsForIssues to Storage interface - Implement in SQLite, Dolt, and Memory backends - Update list.go JSON output to use targeted method - Update mock storage in tests Origin: Mayor's review of PR #1296 Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1194,7 +1194,7 @@ var listCmd = &cobra.Command{
|
|||||||
}
|
}
|
||||||
labelsMap, _ := store.GetLabelsForIssues(ctx, issueIDs)
|
labelsMap, _ := store.GetLabelsForIssues(ctx, issueIDs)
|
||||||
depCounts, _ := store.GetDependencyCounts(ctx, issueIDs)
|
depCounts, _ := store.GetDependencyCounts(ctx, issueIDs)
|
||||||
allDeps, _ := store.GetAllDependencyRecords(ctx)
|
allDeps, _ := store.GetDependencyRecordsForIssues(ctx, issueIDs)
|
||||||
|
|
||||||
// Populate labels and dependencies for JSON output
|
// Populate labels and dependencies for JSON output
|
||||||
for _, issue := range issues {
|
for _, issue := range issues {
|
||||||
|
|||||||
@@ -187,6 +187,45 @@ func (s *DoltStore) GetAllDependencyRecords(ctx context.Context) (map[string][]*
|
|||||||
return result, rows.Err()
|
return result, rows.Err()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// GetDependencyRecordsForIssues returns dependency records for specific issues
|
||||||
|
func (s *DoltStore) GetDependencyRecordsForIssues(ctx context.Context, issueIDs []string) (map[string][]*types.Dependency, error) {
|
||||||
|
if len(issueIDs) == 0 {
|
||||||
|
return make(map[string][]*types.Dependency), nil
|
||||||
|
}
|
||||||
|
|
||||||
|
placeholders := make([]string, len(issueIDs))
|
||||||
|
args := make([]interface{}, len(issueIDs))
|
||||||
|
for i, id := range issueIDs {
|
||||||
|
placeholders[i] = "?"
|
||||||
|
args[i] = id
|
||||||
|
}
|
||||||
|
inClause := strings.Join(placeholders, ",")
|
||||||
|
|
||||||
|
// nolint:gosec // G201: inClause contains only ? placeholders, actual values passed via args
|
||||||
|
query := fmt.Sprintf(`
|
||||||
|
SELECT issue_id, depends_on_id, type, created_at, created_by, metadata, thread_id
|
||||||
|
FROM dependencies
|
||||||
|
WHERE issue_id IN (%s)
|
||||||
|
ORDER BY issue_id
|
||||||
|
`, inClause)
|
||||||
|
|
||||||
|
rows, err := s.db.QueryContext(ctx, query, args...)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("failed to get dependency records for issues: %w", err)
|
||||||
|
}
|
||||||
|
defer rows.Close()
|
||||||
|
|
||||||
|
result := make(map[string][]*types.Dependency)
|
||||||
|
for rows.Next() {
|
||||||
|
dep, err := scanDependencyRow(rows)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
result[dep.IssueID] = append(result[dep.IssueID], dep)
|
||||||
|
}
|
||||||
|
return result, rows.Err()
|
||||||
|
}
|
||||||
|
|
||||||
// GetDependencyCounts returns dependency counts for multiple issues
|
// GetDependencyCounts returns dependency counts for multiple issues
|
||||||
func (s *DoltStore) GetDependencyCounts(ctx context.Context, issueIDs []string) (map[string]*types.DependencyCounts, error) {
|
func (s *DoltStore) GetDependencyCounts(ctx context.Context, issueIDs []string) (map[string]*types.DependencyCounts, error) {
|
||||||
if len(issueIDs) == 0 {
|
if len(issueIDs) == 0 {
|
||||||
|
|||||||
@@ -875,6 +875,20 @@ func (m *MemoryStorage) GetAllDependencyRecords(ctx context.Context) (map[string
|
|||||||
return result, nil
|
return result, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// GetDependencyRecordsForIssues returns dependency records for specific issues
|
||||||
|
func (m *MemoryStorage) GetDependencyRecordsForIssues(ctx context.Context, issueIDs []string) (map[string][]*types.Dependency, error) {
|
||||||
|
m.mu.RLock()
|
||||||
|
defer m.mu.RUnlock()
|
||||||
|
|
||||||
|
result := make(map[string][]*types.Dependency)
|
||||||
|
for _, id := range issueIDs {
|
||||||
|
if deps, ok := m.dependencies[id]; ok {
|
||||||
|
result[id] = deps
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return result, nil
|
||||||
|
}
|
||||||
|
|
||||||
// GetDirtyIssueHash returns the hash for dirty issue tracking
|
// GetDirtyIssueHash returns the hash for dirty issue tracking
|
||||||
func (m *MemoryStorage) GetDirtyIssueHash(ctx context.Context, issueID string) (string, error) {
|
func (m *MemoryStorage) GetDirtyIssueHash(ctx context.Context, issueID string) (string, error) {
|
||||||
// Memory storage doesn't track dirty hashes, return empty string
|
// Memory storage doesn't track dirty hashes, return empty string
|
||||||
|
|||||||
@@ -478,6 +478,64 @@ func (s *SQLiteStorage) GetAllDependencyRecords(ctx context.Context) (map[string
|
|||||||
return depsMap, nil
|
return depsMap, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// GetDependencyRecordsForIssues returns dependency records for specific issues
|
||||||
|
// This is optimized for operations that only need deps for a subset of issues
|
||||||
|
func (s *SQLiteStorage) GetDependencyRecordsForIssues(ctx context.Context, issueIDs []string) (map[string][]*types.Dependency, error) {
|
||||||
|
if len(issueIDs) == 0 {
|
||||||
|
return make(map[string][]*types.Dependency), nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Hold read lock during database operations to prevent reconnect() from
|
||||||
|
// closing the connection mid-query (GH#607 race condition fix)
|
||||||
|
s.reconnectMu.RLock()
|
||||||
|
defer s.reconnectMu.RUnlock()
|
||||||
|
|
||||||
|
// Build parameterized IN clause
|
||||||
|
placeholders := make([]string, len(issueIDs))
|
||||||
|
args := make([]interface{}, len(issueIDs))
|
||||||
|
for i, id := range issueIDs {
|
||||||
|
placeholders[i] = "?"
|
||||||
|
args[i] = id
|
||||||
|
}
|
||||||
|
inClause := strings.Join(placeholders, ",")
|
||||||
|
|
||||||
|
// nolint:gosec // G201: inClause contains only ? placeholders, actual values passed via args
|
||||||
|
query := fmt.Sprintf(`
|
||||||
|
SELECT issue_id, depends_on_id, type, created_at, created_by,
|
||||||
|
COALESCE(metadata, '{}') as metadata, COALESCE(thread_id, '') as thread_id
|
||||||
|
FROM dependencies
|
||||||
|
WHERE issue_id IN (%s)
|
||||||
|
ORDER BY issue_id, created_at ASC
|
||||||
|
`, inClause)
|
||||||
|
|
||||||
|
rows, err := s.db.QueryContext(ctx, query, args...)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("failed to get dependency records for issues: %w", err)
|
||||||
|
}
|
||||||
|
defer func() { _ = rows.Close() }()
|
||||||
|
|
||||||
|
// Group dependencies by issue ID
|
||||||
|
depsMap := make(map[string][]*types.Dependency)
|
||||||
|
for rows.Next() {
|
||||||
|
var dep types.Dependency
|
||||||
|
err := rows.Scan(
|
||||||
|
&dep.IssueID,
|
||||||
|
&dep.DependsOnID,
|
||||||
|
&dep.Type,
|
||||||
|
&dep.CreatedAt,
|
||||||
|
&dep.CreatedBy,
|
||||||
|
&dep.Metadata,
|
||||||
|
&dep.ThreadID,
|
||||||
|
)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("failed to scan dependency: %w", err)
|
||||||
|
}
|
||||||
|
depsMap[dep.IssueID] = append(depsMap[dep.IssueID], &dep)
|
||||||
|
}
|
||||||
|
|
||||||
|
return depsMap, nil
|
||||||
|
}
|
||||||
|
|
||||||
// GetDependencyTree returns the full dependency tree with optional deduplication
|
// GetDependencyTree returns the full dependency tree with optional deduplication
|
||||||
// When showAllPaths is false (default), nodes appearing via multiple paths (diamond dependencies)
|
// When showAllPaths is false (default), nodes appearing via multiple paths (diamond dependencies)
|
||||||
// appear only once at their shallowest depth in the tree.
|
// appear only once at their shallowest depth in the tree.
|
||||||
|
|||||||
@@ -101,6 +101,7 @@ type Storage interface {
|
|||||||
GetDependentsWithMetadata(ctx context.Context, issueID string) ([]*types.IssueWithDependencyMetadata, error)
|
GetDependentsWithMetadata(ctx context.Context, issueID string) ([]*types.IssueWithDependencyMetadata, error)
|
||||||
GetDependencyRecords(ctx context.Context, issueID string) ([]*types.Dependency, error)
|
GetDependencyRecords(ctx context.Context, issueID string) ([]*types.Dependency, error)
|
||||||
GetAllDependencyRecords(ctx context.Context) (map[string][]*types.Dependency, error)
|
GetAllDependencyRecords(ctx context.Context) (map[string][]*types.Dependency, error)
|
||||||
|
GetDependencyRecordsForIssues(ctx context.Context, issueIDs []string) (map[string][]*types.Dependency, error)
|
||||||
GetDependencyCounts(ctx context.Context, issueIDs []string) (map[string]*types.DependencyCounts, error)
|
GetDependencyCounts(ctx context.Context, issueIDs []string) (map[string]*types.DependencyCounts, error)
|
||||||
GetDependencyTree(ctx context.Context, issueID string, maxDepth int, showAllPaths bool, reverse bool) ([]*types.TreeNode, error)
|
GetDependencyTree(ctx context.Context, issueID string, maxDepth int, showAllPaths bool, reverse bool) ([]*types.TreeNode, error)
|
||||||
DetectCycles(ctx context.Context) ([][]*types.Issue, error)
|
DetectCycles(ctx context.Context) ([][]*types.Issue, error)
|
||||||
|
|||||||
@@ -69,6 +69,9 @@ func (m *mockStorage) GetDependencyRecords(ctx context.Context, issueID string)
|
|||||||
func (m *mockStorage) GetAllDependencyRecords(ctx context.Context) (map[string][]*types.Dependency, error) {
|
func (m *mockStorage) GetAllDependencyRecords(ctx context.Context) (map[string][]*types.Dependency, error) {
|
||||||
return nil, nil
|
return nil, nil
|
||||||
}
|
}
|
||||||
|
func (m *mockStorage) GetDependencyRecordsForIssues(ctx context.Context, issueIDs []string) (map[string][]*types.Dependency, error) {
|
||||||
|
return nil, nil
|
||||||
|
}
|
||||||
func (m *mockStorage) GetDependencyCounts(ctx context.Context, issueIDs []string) (map[string]*types.DependencyCounts, error) {
|
func (m *mockStorage) GetDependencyCounts(ctx context.Context, issueIDs []string) (map[string]*types.DependencyCounts, error) {
|
||||||
return nil, nil
|
return nil, nil
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user