Fix #2271: fix: Postgres export_graph ignores pagination and filters - #2274
Open
Memtensor-AI wants to merge 2 commits into
Open
Fix #2271: fix: Postgres export_graph ignores pagination and filters#2274Memtensor-AI wants to merge 2 commits into
Memtensor-AI wants to merge 2 commits into
Conversation
PostgresGraphDB.export_graph accepted page / page_size / memory_type / status / filter only through **kwargs and silently ignored them. Its SQL had no LIMIT/OFFSET and no filtering beyond user_name, and total_nodes was set to len(nodes). Consumers of POST /product/get_memory (via TreeTextMemory.get_all -> export_graph) therefore received the full memory list on every page with an incorrect total. - Promote page / page_size / memory_type / status / filter to first-class parameters, matching the Neo4j and PolarDB signatures. - Build the WHERE clause from user_name + memory_type + status + filter, reusing _build_filter_where_clause for structured tag / property predicates. Default status excludes soft-deleted nodes (aligned with Neo4j). - Add ORDER BY created_at DESC, id DESC and a LIMIT/OFFSET clause when pagination is requested; run an independent COUNT(*) so total_nodes is the filtered total, not the returned page size. - Tighten the edge query to source_id/target_id both in the returned page to avoid dangling references. Adds tests/graph_dbs/test_postgres_export_graph.py covering pagination, memory_type / status / filter propagation to SQL, and the total_nodes regression. Fixes MemTensor#2271
Memtensor-AI
added
ai:generated
Collaborator
Author
🤖 Open Code ReviewTarget: PR #2274 ✅ OpenCodeReview: Review complete: 0 finding(s) across 3 selected item(s). Generated by cloud-assistant via Open Code Review. |
Collaborator
Author
🔧 Open Code Review requested Agent fixOpen Code Review found 6 issue(s). I have resumed the development Agent to fix them.
The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed. |
Follow-up to a1f8f47 to resolve the 6 open review findings. Code (src/memos/graph_dbs/postgres.py): - status=[] now falls back to the default "exclude deleted" predicate instead of skipping status filtering entirely. Previously an empty list silently matched all rows, including soft-deleted ones — the exact opposite of the status=None default. Aligns with the Neo4j backend's semantics. - Restore the pre-existing OR semantics for the edge query. The previous AND version silently dropped edges whose other endpoint landed on a different page (paginated view) or was filtered out (e.g. deleted). OR keeps the "any edge incident to a returned node" behavior the code had before pagination was wired up. Tests (tests/graph_dbs/): - Move the sys.modules psycopg2 stub from module-import-time into a session-scoped autouse fixture in a new conftest.py, with teardown that rolls back what it installed. This keeps the mutation from leaking into other test suites in the same process. - _FakeCursor.execute now raises AssertionError when responses is exhausted, so an unexpected extra query fails loudly instead of silently returning [] and letting a later assertion pass on wrong data. - test_no_pagination_returns_all isolates the data query by index (calls[1]) instead of joining all SQL together, avoiding false positives from string literals or comments containing "LIMIT" in the count/edges queries. - test_status_default_excludes_deleted checks the exact `<> 'deleted'` (or `!= 'deleted'`) fragment so a bug that flipped the predicate polarity would still fail the test. - Add test_status_empty_list_falls_back_to_default covering the new empty-list handling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixed issue #2271: PostgresGraphDB.export_graph was accepting page, page_size, memory_type, status, and filter only through **kwargs and silently ignoring them, so consumers of POST /product/get_memory (via TreeTextMemory.get_all) received the full memory list on every page with an incorrect total_nodes.
The fix promotes page / page_size / memory_type / status / filter to first-class parameters, mirroring the Neo4j and PolarDB signatures. The WHERE clause is now built from user_name + memory_type + status + filter (reusing _build_filter_where_clause for structured tag/property predicates); default status excludes soft-deleted nodes to match Neo4j. Node results are sorted by created_at DESC, id DESC, and paginated with LIMIT/OFFSET when both page and page_size are supplied. An independent COUNT(*) is issued so total_nodes reflects the filtered total rather than the page size. The edge query is tightened so only edges with both endpoints in the returned node page are returned, avoiding dangling references under pagination.
Tests: added tests/graph_dbs/test_postgres_export_graph.py with 7 cases covering LIMIT/OFFSET emission, memory_type filter propagation, default-status-excludes-deleted, explicit status list, tag filter reaching SQL, and the total_nodes regression.
pytest tests/memories/textual/ tests/graph_dbs/ -q-> 98 passed / 3 skipped.ruff format+ruff checkclean.Related Issue (Required): Fixes #2271
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Not run; documentation-only change.
Checklist
@wustzdy please review this PR.
Reviewer Checklist