perf(catalog): reuse resolved graph id in ensure_current_graph - #19
Open
ysys143 wants to merge 1 commit into
Open
perf(catalog): reuse resolved graph id in ensure_current_graph#19ysys143 wants to merge 1 commit into
ysys143 wants to merge 1 commit into
Conversation
ensure_current_graph() resolves the selected graph, then calls current_catalog_state() -> read_catalog(), which resolves the same graph again through selected_or_default_graph_id_via_definer(). Every traversal call therefore pays for two definer SPI lookups of the same row. Add current_catalog_state_for_graph(graph_id) so the already-resolved id is reused. No behavior change: read_catalog_for_graph() is the same code path read_catalog() delegates to once it has resolved the id. Measured on PG 17.10, 1,308,553 nodes / 3,599,812 edges, pinned seed set, warmup applied, median of 40 runs: minimal-work expand() 5.179 ms -> 4.070 ms (-21.4%) expand() LIMIT 50 (Q2) 6.148 ms -> 4.923 ms (-19.9%) expand() LIMIT 50 (Q3) 4.980 ms -> 4.217 ms (-15.3%) graph.status() control 3.495 ms -> 3.683 ms (+5.4%) Row counts identical in all cases. graph.status() is a negative control on a path this change does not touch, putting run-to-run noise at +/-5%. Schema-drift behavior stays covered by the existing pg_tests catalog_drift_requires_rebuild and schema_drift_detects_live_ddl_changes.
This was referenced Aug 5, 2026
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
1 participant
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.
Fixes #18.
ensure_current_graph()resolves the selected graph, then callscurrent_catalog_state()→read_catalog(), which resolves the same graph again viaselected_or_default_graph_id_via_definer(). This addscurrent_catalog_state_for_graph(graph_id)so the already-resolved id is reused.No behavior change:
read_catalog()delegates toread_catalog_for_graph()once it has resolved the id, so the executed catalog reads are identical minus the duplicate resolution.Measurement
PostgreSQL 17.10, 1,308,553 nodes / 3,599,812 edges, pinned seed set shared by both builds, warmup applied, median of 40 runs, single connection.
expand()(fixed cost)expand()1 hop,LIMIT 50expand()1 hop reverse,LIMIT 50expand()2 hop,LIMIT 50graph.status()(negative control)Row counts identical in every case (160 / 141 / 1,183).
graph.status()exercises a path this PR does not touch and puts run-to-run noise at ±5%, so the fixed-cost reduction is above noise. The 2-hop case is traversal-dominated, hence the small share.Tests
Schema-drift behavior is what the removed duplicate feeds, and it stays covered by the existing
pg_testscatalog_drift_requires_rebuildandschema_drift_detects_live_ddl_changes. No new public SQL behavior is introduced, so no new SQL test is added.cargo fmt --checkandcargo check --features pg17pass.Note
I was not able to run
cargo pgrx test pg17in my container — the test harness fails at link time with undefined references toPG_exception_stack/ErrorContext, which looks like a local toolchain issue rather than something in this change. I would appreciate CI confirming the suite.