feat(db): add SQLC queries and generated code for graph tables#468
Conversation
Add SQL queries for graph-based entity storage operations: - UpsertGraphNode: Insert or update nodes with duplicate counting - UpdateGraphNodeData: Update node data and schema version - FindGraphNode: Find node by type and external_id - UpsertGraphEdge: Insert or update edges with properties - UpsertGraphEdgeWithConfidence: Edges with confidence scoring - GetGraphNodesByType: Paginated nodes by type - GetConnectedNodes: Get nodes connected via edges - GetNodesByFrequency: Get frequently observed nodes - SearchGraphNodes: Full-text search across nodes - GetGraphStats/GetGraphStatsByType: Analytics queries - Snapshot queries for historical tracking Generated types: GraphNode, GraphEdge, GraphNodeSnapshot
mfiedorowicz
requested review from
MicahParks,
grant-nbl,
jajeffries,
leoparente,
manrodrigues and
paulstuart
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2f2d65ef2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Go test coverage
Total coverage: 54.5% |
Fix FindNodesByFieldMatch query where NULL optional parameters would cause the OR clause to match all nodes of a given type. Changed from: (field IS NULL OR data->>field = value) Changed to: (field IS NOT NULL AND data->>field = value) Now callers must provide at least one field filter, and NULL parameters are properly skipped rather than becoming match-all conditions.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 558e506808
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add ::text casts to all field name and value parameters in graph queries to ensure SQLC generates string types instead of []byte. Without casts, sqlc infers []byte (bytea) for untyped JSON key/value params, causing runtime failures since Postgres doesn't support jsonb ->> bytea or text = bytea comparisons. Affected queries: - FindNodesByFieldMatch - FindNodesByMultiFieldMatch - FindNodesByFieldPattern - FindNodesByComplexMatch
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b20db4463a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Use COALESCE defaults for snapshot columns from LEFT JOIN LATERAL to
prevent scan errors when a node has no snapshots:
- snapshot_data: defaults to '{}' (empty JSON object)
- sequence_number: defaults to 0
Callers can check sequence_number == 0 to determine if no snapshot exists.
This avoids the need for pointer types which would affect other queries.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a909eb5df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Change first_seen from MIN(updated_at) to MIN(created_at) so it correctly represents when nodes of each type were first created, not when they were last modified. updated_at changes on every node update (via trigger), causing first_seen to drift forward and break historical analytics.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2dbaa810b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The nested-field branch was gated on nested_field but actually used nested_path for the JSON lookup. This made nested matching silently fail unless an otherwise unused parameter was set. Now gates on nested_path which is the parameter actually used in the data#>> operator. Removed the unused nested_field parameter.
Address schema validation issues: - High: Add CHECK (confidence_score >= 0.0 AND confidence_score <= 1.0) to graph_edges to enforce documented 0.0-1.0 range - Medium: Add NOT NULL to all timestamp columns (created_at, updated_at) to ensure time tracking for ordering/filtering - Low: Add CHECK (sequence_number > 0) to graph_node_snapshots to enforce positive monotonic sequences Note: edge_subtype uniqueness not changed - it's intentionally a classification of the edge, not a separate edge type. One edge per type between nodes, subtype is updateable metadata.
High: Make InsertSnapshot atomic using INSERT...SELECT to compute sequence_number in a single statement, preventing race conditions when concurrent inserts compute the same next sequence. Removed GetNextSequenceNumber as it's no longer needed. Medium: Require non-empty search_term in SearchGraphNodes to prevent accidental full table scans. Use GetGraphNodesByType for listing. Low: Added warning comment to CleanupOldSnapshots that limit=0 deletes all snapshots for the node.
High: InsertSnapshot now uses pg_advisory_xact_lock(node_id) to serialize concurrent inserts for the same node, preventing duplicate sequence numbers from SELECT MAX without locking. Medium/Low: Improved SearchGraphNodes documentation: - search_term is REQUIRED (non-null, non-empty) - Empty string returns no results (not an error) - node_type is optional (NULL searches all types) - Noted that very long search terms may cause slow scans
Thanks, optimizations will be addressed separately |
…t match
Replace slow ILIKE pattern matching (full table scan) with GIN-indexed
JSONB containment query:
- Old: data->>$field ILIKE '%pattern%' (no index, full scan)
- New: data @> $match_filter::jsonb (uses idx_graph_nodes_data_gin)
Callers pass match_filter as JSONB, e.g., {"name": "value"}.
Fuzzy/pattern matching is handled in the application layer (matching pkg).
2 participants
Add SQL queries for graph-based entity storage operations:
Generated types: GraphNode, GraphEdge, GraphNodeSnapshot