Skip to content

feat: Optimize PG source_kind, relation deletion using indices - BED-8832#101

Open
StephenHinck wants to merge 5 commits into
mainfrom
BED-8832
Open

feat: Optimize PG source_kind, relation deletion using indices - BED-8832#101
StephenHinck wants to merge 5 commits into
mainfrom
BED-8832

Conversation

@StephenHinck

@StephenHinck StephenHinck commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Description

Introduces new, optional functions for deleting nodes and relationships by kind, enabling the use of indices for more performant bulk deletes. This primarily supports the various database management capabilities built into BloodHound, but may be used for other bulk delete operations as necessary.

Implemented in

Resolves: BED-8832

Type of Change

  • Chore (a change that does not modify the application functionality)
  • Bug fix (a change that fixes an issue)
  • New feature / enhancement (a change that adds new functionality)
  • Refactor (no behaviour change)
  • Test coverage
  • Build / CI / tooling
  • Documentation

Testing

  • Unit tests added / updated
  • Integration tests added / updated
  • Full test suite run (make test_all with CONNECTION_STRING set)

Screenshots (if appropriate):

Driver Impact

  • PostgreSQL driver (drivers/pg)
  • Neo4j driver (drivers/neo4j)

Checklist

  • Code is formatted
  • All existing tests pass
  • go.mod / go.sum are up to date if dependencies changed

Summary by CodeRabbit

  • New Features
    • Added PostgreSQL server-side operations to delete nodes by selected kinds, with safe include/exclude filtering.
    • Added PostgreSQL server-side operations to delete relationships by kind.
  • Bug Fixes
    • Improved cascade edge cleanup to work correctly when multiple nodes are deleted in a single batch (statement-level trigger behavior).
    • Undefined kinds now act as a safe no-op for includes, and deletes avoid widening when exclude kinds are undefined.
  • Tests
    • Added manual PostgreSQL integration tests for node deletes, relationship deletes, and cascade behavior.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 548bdf8b-0a18-4143-a6df-5f77c24b961f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

Adds PostgreSQL driver methods for deleting nodes and relationships by kind, updates kind-ID resolution, changes the node-edge delete trigger to statement-level cleanup, and adds integration tests for batch cascade behavior and both delete-by-kind paths.

Changes

PostgreSQL Bulk Delete by Kind

Layer / File(s) Summary
Statement-level cascade trigger in schema SQL
drivers/pg/query/sql/schema_up.sql
Replaces the FOR EACH ROW delete_node_edges trigger with a FOR EACH STATEMENT trigger using REFERENCING OLD TABLE AS deleted_nodes.
resolveKindIDs and bulk-delete methods
drivers/pg/driver.go
Adds resolveKindIDs, DeleteNodesByKinds, and DeleteRelationshipsByKinds, plus dynamic SQL predicate construction and wrapped connection/exec errors.
Statement-level cascade integration test
integration/pgsql_delete_cascade_test.go
Adds the batch-delete cascade test and countByCypher helper to verify remaining node and edge counts.
Delete-by-kind integration tests
integration/pgsql_delete_by_kind_test.go, integration/pgsql_delete_relationships_by_kind_test.go
Adds PostgreSQL integration tests for node and relationship deletion by kind, including capability checks, fixtures, subtests, and cleanup.
Indirect dependency updates
go.mod
Updates indirect dependency versions and adds github.com/clipperhouse/uax29/v2.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

enhancement, go

Suggested reviewers

  • zinic
  • superlinkx

Poem

🐇 Hop goes the batch, not row by row,
Kinds are resolved, and the edge winds know.
One clean trigger for nodes that fall,
Then tests nibble through kind deletes all.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately reflects the PostgreSQL kind-based bulk delete optimization.
Description check ✅ Passed The description mostly follows the template, but the "Implemented in" line is incomplete and the dependency checklist is not fully marked.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch BED-8832

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@drivers/pg/driver.go`:
- Around line 231-254: The delete builder in the `Delete` flow is too tolerant
when resolving `excludeAny`, which can turn a protected delete into an unguarded
`delete from node`. Update the logic around `resolveKindIDs` in
`drivers/pg/driver.go` so unresolved exclude kinds fail closed instead of being
silently dropped, and make the `predicates`/`arguments` construction in the
delete statement require valid `excludeIDs` before adding the `not (kind_ids
operator ...)` clause. Keep `includeAny` behavior unchanged, but ensure
`excludeAny` cannot widen the delete when `resolveKindIDs` returns an empty or
partial match.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0dd1938a-2de7-4984-b84d-79a28b13ad41

📥 Commits

Reviewing files that changed from the base of the PR and between 8fbfba4 and 4347bb9.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (6)
  • drivers/pg/driver.go
  • drivers/pg/query/sql/schema_up.sql
  • go.mod
  • integration/pgsql_delete_by_kind_test.go
  • integration/pgsql_delete_cascade_test.go
  • integration/pgsql_delete_relationships_by_kind_test.go

Comment thread drivers/pg/driver.go Outdated
@StephenHinck StephenHinck changed the base branch from main to BED-8796 June 30, 2026 19:30
@StephenHinck StephenHinck changed the base branch from BED-8796 to main June 30, 2026 22:17
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