Skip to content

Conversation

@yfukai
Copy link
Contributor

@yfukai yfukai commented Dec 10, 2025

This pull request introduces a new feature for the SQLGraph backend: the ability to create explicit database indexes on node and edge attributes to improve query performance, especially for frequently filtered attributes. The documentation and tests have been updated to reflect and validate this functionality.

SQLGraph indexing improvements:

  • Added methods ensure_node_attr_index and ensure_edge_attr_index to SQLGraph for creating indexes on node and edge attribute columns, including support for composite and unique indexes. (src/tracksdata/graph/_sql_graph.py)
  • Updated documentation to describe the new index feature and provide usage examples for creating indexes on attributes. (docs/concepts.md)
  • Updated the project README to mention SQLGraph's ability to index frequently queried attributes for faster filtering. (README.md)

Testing and validation:

  • Added tests to ensure index creation works as expected, including checks for composite and unique indexes, and error handling for missing columns. (src/tracksdata/graph/_test/test_graph_backends.py)
  • Added sqlalchemy import to support index inspection in tests. (src/tracksdata/graph/_test/test_graph_backends.py)

@codecov-commenter
Copy link

codecov-commenter commented Dec 10, 2025

Codecov Report

❌ Patch coverage is 85.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.43%. Comparing base (c5af9f3) to head (a5a4efd).

Files with missing lines Patch % Lines
src/tracksdata/graph/_sql_graph.py 85.00% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #223      +/-   ##
==========================================
- Coverage   88.45%   88.43%   -0.02%     
==========================================
  Files          55       55              
  Lines        3890     3910      +20     
  Branches      674      678       +4     
==========================================
+ Hits         3441     3458      +17     
- Misses        267      268       +1     
- Partials      182      184       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yfukai
Copy link
Contributor Author

yfukai commented Dec 10, 2025

Maybe we need drop functions

Copy link
Member

@JoOkuma JoOkuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yfukai, awesome PR, I had hadn't thought of adding this.

Comment on lines +25 to +26
SQLGraph lets you create indexes on node or edge attributes to keep repeated
filters fast:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yfukai this is awesome. Could you briefly mention what kind of speed-up we can expect with this? 2x, 10x?

unique: bool = False,
name: str | None = None,
) -> str:
"""Ensure an index exists for the given node attribute columns.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yfukai, is this true?
I added this comment because I was trying to understand why it is ensure.

Also, could you highlight in the docs what the benefit of the name parameter is?
I have the impression that there's also a reason why you're returning the name.
I think it would enhance the user experience when using these methods.

This could be duplicated in the ensure_edge_attr_index docs.

Suggested change
"""Ensure an index exists for the given node attribute columns.
"""
Ensure an index exists for the given node attribute columns.
If they are already indexed, they are kept as they are.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants