#706 Add the ability to delete bookkeeping tables from the database via Bookeeper trait

[yruslan]

Closes #706

Summary by CodeRabbit

• New Features

  • Add ability to delete table metadata using wildcard and prefix patterns.
  • Bookkeeper implementations now support explicit close/cleanup semantics.

• Tests

  • Added tests covering deletion by prefix and wildcard patterns.

• Bug Fixes

  • Fixed a log message typo ("Bookkeeping").

[coderabbitai]

Walkthrough

The Bookkeeper trait gains AutoCloseable and a new deleteTable(tableWithWildcard: String): Seq[String] API. Implementations were updated: JDBC provides a full implementation and close(), some implementations provide stubs or no-op behavior, tests and mocks updated to exercise deletion behavior.

Changes

Cohort / File(s)	Summary
Core trait & base pramen/core/src/main/scala/za/co/absa/pramen/core/bookkeeper/Bookkeeper.scala, pramen/core/src/main/scala/za/co/absa/pramen/core/bookkeeper/BookkeeperBase.scala	Bookkeeper now extends AutoCloseable; new abstract deleteTable(tableWithWildcard: String): Seq[String]; BookkeeperBase provides override def close(): Unit = {}.
JDBC implementation pramen/core/src/main/scala/za/co/absa/pramen/core/bookkeeper/BookkeeperJdbc.scala	Added fully implemented deleteTable(...) with wildcard handling, match-limit validation (≤100), cascading deletions across bookkeeping/schema/offsets/metadata tables, logging of deleted counts; added idempotent close() using isClosed.
Other implementations (stubs / trivial) pramen/core/src/main/scala/za/co/absa/pramen/core/bookkeeper/BookkeeperDeltaPath.scala, .../BookkeeperMongoDb.scala, .../BookkeeperText.scala, .../BookkeeperNull.scala	Added deleteTable(...) overrides: most are placeholders (???) except BookkeeperNull which returns empty Seq.
DeltaTable (versioned sources) pramen/core/src/main/scala_2.11/za/co/absa/pramen/core/bookkeeper/BookkeeperDeltaTable.scala, .../scala_2.12/.../BookkeeperDeltaTable.scala, .../scala_2.13/.../BookkeeperDeltaTable.scala	Added override def deleteTable(...) stubs (???) across Scala-versioned implementations.
Tests & mocks pramen/core/src/test/scala/za/co/absa/pramen/core/mocks/bookkeeper/SyncBookkeeperMock.scala, pramen/core/src/test/scala/za/co/absa/pramen/core/tests/bookkeeper/BookkeeperJdbcSuite.scala	Mock implements wildcard-aware deleteTable(...) and close() no-op; BookkeeperJdbcSuite gains tests for deletion by prefix and wildcard verifying schema and chunk removals.
Minor config fix pramen/core/src/main/scala/za/co/absa/pramen/core/app/config/BookkeeperConfig.scala	Typo fix in log message: "Bookeeping" → "Bookkeeping".

Sequence Diagram

sequenceDiagram
    actor Client
    participant BookkeeperJdbc as Bookkeeper JDBC
    participant DB as Database
    participant Logger

    Client->>BookkeeperJdbc: deleteTable(pattern)
    activate BookkeeperJdbc
    BookkeeperJdbc->>BookkeeperJdbc: escape pattern & validate (≤100)
    BookkeeperJdbc->>DB: SELECT matching table names
    DB-->>BookkeeperJdbc: matched names
    BookkeeperJdbc->>DB: DELETE FROM bookkeeping WHERE table IN (matched)
    DB-->>BookkeeperJdbc: bookkeeping delete counts
    BookkeeperJdbc->>DB: DELETE FROM schema WHERE table IN (matched)
    DB-->>BookkeeperJdbc: schema delete counts
    BookkeeperJdbc->>DB: DELETE FROM offsets WHERE table IN (matched)
    DB-->>BookkeeperJdbc: offsets delete counts
    BookkeeperJdbc->>DB: DELETE FROM metadata WHERE table IN (matched)
    DB-->>BookkeeperJdbc: metadata delete counts
    BookkeeperJdbc->>Logger: log deletion summaries
    BookkeeperJdbc-->>Client: Seq[deleted_table_names]
    deactivate BookkeeperJdbc

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• #699 Add retries for Bookkeeping database, and make sure only 1 connection is active. #700 — Related Bookkeeper changes and DB connection handling that touch the same modules and lifecycle concerns.
• #677 Add batch Id and the number of appended record to the bookeeping #682 — Modifies Bookkeeper trait and implementations; closely related to the new API surface added here.

Poem

🐰 Hop, hop, the ledger clears,

wildcards chased through dusty layers.
JDBC nibbles, mocks pretend,
Close the gate — the cleanup's friend,
A rabbit's cheer for tidy prayers.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding the ability to delete bookkeeping tables via the Bookkeeper trait, which aligns perfectly with the PR's objectives.
Linked Issues check	✅ Passed	The PR implements all coding requirements from issue #706: adds deleteTable method to Bookkeeper interface, implements deletion across bookkeeping/schemas/offsets tables in BookkeeperJdbc, and provides implementations for all concrete classes.
Out of Scope Changes check	✅ Passed	All changes are scoped to the deletion feature: Bookkeeper trait updates, concrete implementations, corresponding tests, and a minor typo fix in a related configuration file.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/706-bookkeeping-records-deletion

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pramen/core/src/main/scala/za/co/absa/pramen/core/bookkeeper/BookkeeperText.scala (1)

201-206: ⚠️ Potential issue | 🟠 Major

deleteTable is unimplemented and will throw at runtime.

??? raises NotImplementedError, so any call will crash pipelines using BookkeeperText. Please implement deletion (e.g., filter and rewrite bookkeeping.csv and schemas.json, mirroring wildcard logic used in SyncBookkeeperMock) or explicitly document unsupported behavior and handle it safely.

🤖 Fix all issues with AI agents

In
`@pramen/core/src/main/scala/za/co/absa/pramen/core/bookkeeper/Bookkeeper.scala`:
- Around line 54-60: Update the Scaladoc comment in the Bookkeeper class for the
method that "Deletes tables matching the given wildcard pattern" by correcting
the typo "bookeeping" to "bookkeeping" so the sentence reads "The method deletes
just the metadata about selected tables in every bookkeeping table except
journal, which is used for logging only."; locate and edit the Scaladoc block
above the delete-tables method in Bookkeeper.scala to apply this change.

In
`@pramen/core/src/main/scala/za/co/absa/pramen/core/bookkeeper/BookkeeperJdbc.scala`:
- Around line 196-231: The current deletes (BookkeepingRecords.records,
SchemaRecords.records, OffsetRecords.records, MetadataRecords.records) are
executed separately via SlickUtils.executeAction which can leave the DB in an
inconsistent state if one delete fails; combine these four delete actions into a
single Slick DB action using .andThen(...) chaining and mark it .transactionally
(or otherwise run as one transaction) so all deletes either commit or roll back
together, then execute that single transactional action instead of calling
SlickUtils.executeAction for each individual delete.
- Around line 180-208: The LIKE handling doesn't escape '_' so single-character
wildcards can match unintended tables; update the pattern construction to also
replace "_" with "\\_" (in addition to replacing "%" with "\\%") when building
tableNameEscaped/likePattern, keep the existing conversion of '*' to '%' for
wildcards, and then use Slick's like(pattern, escape) overload with escape =
'\\' when querying (update the .like usages on BookkeepingRecords.records in
listQuery and deletionQuery and any other .like calls in this method) so queries
use .like(likePattern, '\\') and prevent accidental matches.

In
`@pramen/core/src/test/scala/za/co/absa/pramen/core/mocks/bookkeeper/SyncBookkeeperMock.scala`:
- Around line 159-179: The deleteTable mock mixes SQL LIKE syntax with regex
matching: update the likePattern construction in deleteTable so it produces a
regex, not a SQL LIKE; specifically change the non-wildcard branch to append
"->.*" (regex) instead of "->%" so that keysToDelete filtering using
tblName.matches(...) works correctly. Keep using the existing tableNameEscaped,
chunks, and the .matches(...) checks; no other logic changes required.

🧹 Nitpick comments (1)

pramen/core/src/main/scala/za/co/absa/pramen/core/bookkeeper/BookkeeperJdbc.scala (1)

41-42: Make close() atomic for concurrent callers.

The if (!isClosed) check isn’t atomic, so concurrent callers can double-close. Consider AtomicBoolean.

🔒 Atomic close

+import java.util.concurrent.atomic.AtomicBoolean
@@
-  `@volatile` private var isClosed = false
+  private val isClosed = new AtomicBoolean(false)
@@
   override def close(): Unit = {
-    if (!isClosed) {
-      db.close()
-      isClosed = true
-    }
+    if (isClosed.compareAndSet(false, true)) {
+      db.close()
+    }
   }

Also applies to: 238-243

[github-actions]

Unit Test Coverage

Overall Project	84.09% -0.09%	🍏
Files changed	77.4%	❌

Module	Coverage
pramen:core Jacoco Report	86.02% -0.1%	❌

Files

Module	File	Coverage
pramen:core Jacoco Report	BookkeeperDeltaPath.scala	99.44% -0.56%	❌
	BookkeeperText.scala	96.83% -0.43%	❌
	BookkeeperDeltaTable.scala	94.24%	🍏
	Bookkeeper.scala	91.92%	🍏
	BookkeeperConfig.scala	90.48%	🍏
	BookkeeperMongoDb.scala	76.54% -0.28%	❌
	BookkeeperBase.scala	64.26% -0.19%	❌
	BookkeeperJdbc.scala	63.34% -4.71%	❌
	BookkeeperNull.scala	53.33% -10%	❌