Skip to content

Add table-based node lookup to GraphQL - #2598

Open
jlhester wants to merge 1 commit into
mainfrom
table-node-lookup
Open

jlhester wants to merge 1 commit into
mainfrom
table-node-lookup

Conversation

@jlhester

Copy link
Copy Markdown
Collaborator

Adds a tables filter to findNodes/findNodesPaginated matching nodes by physical table, and a nodesForTable query that returns the source nodes on a table plus everything downstream. Table entries may be fully or partially qualified, or bare.

Indexes noderevision on (catalog_id, schema_, table) to back the filter.

Summary

Test Plan

  • PR has an associated issue: #
  • make check passes
  • make test shows 100% unit test coverage

Deployment Plan

@netlify

netlify Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for thriving-cassata-78ae72 canceled.

Name Link
🔨 Latest commit 207d7c3
🔍 Latest deploy log https://app.netlify.com/projects/thriving-cassata-78ae72/deploys/6abe8bd931d8250008510728

@greptile-apps

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Critical risk] Adds database migration and table-based node lookup to GraphQL.

The PR should not merge until nodesForTable can return all matching sources and their downstream nodes without silent truncation.

Findings

  1. P1 Table results are silently truncated ▶
  2. P2 Index misses table lookups ▶

Summary

Adds case-insensitive physical-table filtering to both node-list queries and a nodesForTable query that finds matching sources and traverses their downstream graph.

  • Adds a composite node-revision index and GraphQL coverage for qualified, partial, and bare table names.
  • The new lookup can silently truncate a table’s source nodes; the index does not align with its lookup predicates.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  T["Physical table input"] --> F["Filter current source revisions"]
  F --> S["Up to 1,000 source nodes"]
  S --> D["Traverse downstream per source"]
  S --> R["Deduplicate and filter node types"]
  D --> R
  R --> O["nodesForTable result"]
Loading

Reviews (1) · Last reviewed commit: "Add table-based node lookup to GraphQL"

# A physical table backs one source node in practice, a handful at most (the
# same table registered under different namespaces). ``find_by`` always applies
# some limit, so pick one well clear of that.
MAX_SOURCE_NODES = 1000

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Table results are silently truncated

If more than 1,000 source nodes reference one physical table, this limit returns only the newest 1,000. The query never traverses the omitted sources, so nodesForTable also leaves out their downstream nodes despite promising everything downstream of the table. Source registration does not enforce a per-table limit.

UniqueConstraint("version", "node_id"),
Index("ix_noderevision_node_id", "node_id"),
# Backs the ``tables`` filter (find nodes by physical table).
Index("ix_noderevision_table", "catalog_id", "schema_", "table"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Index misses table lookups

The filter compares lower(table) and, for qualified names, lower(schema_), but this index stores the original column values. Bare-table lookups also do not constrain its leading catalog_id column. As a result, the new index does not provide the intended lookup benefit while still adding index maintenance cost.

Adds a `tables` filter to findNodes/findNodesPaginated matching nodes by
physical table, and a `nodesForTable` query that returns the source nodes
on a table plus everything downstream. Table entries may be fully or
partially qualified, or bare.

Indexes noderevision on (catalog_id, schema_, table) to back the filter.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant