Guard SqlChangeLogParser.generateId against null DB connection by filipelautert · Pull Request #7719 · liquibase/liquibase

filipelautert · 2026-05-13T14:12:45Z

Summary

Widens the inner catch in SqlChangeLogParser.generateId from DatabaseException to Exception so the buildInterimIdIndex path falls back to "raw" cleanly when the JDBC connection is null.
Adds a unit test covering the regression.

Background

PR #7674 added a buildInterimIdIndex path that queries DATABASECHANGELOG during changelog parsing. The pre-existing try/catch only wraps DatabaseException, missing the IllegalArgumentException thrown by PreparedStatementFactory when database.getConnection() returns null.

Stack:

SqlChangeLogParser.parse → generateId → buildInterimIdIndex
  → StandardChangeLogHistoryService.getRanChangeSets
    → SnapshotGeneratorFactory.checkLiquibaseTablesExistence
      → JdbcExecutor.queryForInt → new PreparedStatementFactory(null) → IllegalArgumentException

This is hit by, among other paths, the changelog-scope checks run flow against H2-generated SQL with no --url.

Behavior change

Behavior unchanged when a live JDBC connection is present. When the lookup fails (any reason), parsing falls back to "raw". The "retries on transient failure" semantics provided by computeIfAbsent's no-cache-on-throw behavior continue to apply, for both DatabaseException and IllegalArgumentException.

Tests

New: SqlChangeLogParserTest > generateId falls back to 'raw' when the index build throws a non-DatabaseException — fails on main (IAE escapes), passes with this change.
Existing: full SqlChangeLogParserTest (7/7) green.

The buildInterimIdIndex path added in #7674 queries DATABASECHANGELOG during changelog parsing. The inner try/catch wraps only DatabaseException, missing the IllegalArgumentException thrown by PreparedStatementFactory when database.getConnection() returns null (offline mode, e.g. changelog-scope `checks run` against H2 generated SQL with no --url). Widen the inner catch from DatabaseException to Exception so the fall-back-to-"raw" path triggers for any failure surfaced by the history-service lookup. The "retries on transient failure" semantics (via computeIfAbsent's no-cache-on-throw behavior) still apply for both DatabaseException and IllegalArgumentException. Test: SqlChangeLogParserTest gains "generateId falls back to 'raw' when the index build throws a non-DatabaseException", which fails on master (IllegalArgumentException escapes generateId) and passes with this change.

coderabbitai · 2026-05-13T14:13:18Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4ded28ca-2d28-482b-983b-0142a83b1a8d

📥 Commits

Reviewing files that changed from the base of the PR and between 1873538 and aed5a18.

📒 Files selected for processing (2)

liquibase-standard/src/main/java/liquibase/parser/core/sql/SqlChangeLogParser.java
liquibase-standard/src/test/groovy/liquibase/parser/core/sql/SqlChangeLogParserTest.groovy

📝 Walkthrough

Walkthrough

SqlChangeLogParser.generateId() now catches all Exception types instead of only DatabaseException when building the interim changeset index, allowing fallback to a "raw" ID for broader error conditions. A test validates this behavior for non-DatabaseException failures.

Changes

SQL Changelog ID Generation Exception Handling

Layer / File(s)	Summary
Broaden exception handling in ID generation `liquibase-standard/src/main/java/liquibase/parser/core/sql/SqlChangeLogParser.java`, `liquibase-standard/src/test/groovy/liquibase/parser/core/sql/SqlChangeLogParserTest.groovy`	`generateId()` changes from catching only `DatabaseException` to catching any `Exception` in the cached `computeIfAbsent()` path, enabling fallback to `"raw"` ID for broader failure conditions. A new test verifies the fallback behavior when `IllegalArgumentException` is thrown during index retrieval.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

liquibase/liquibase#7674: This PR extends the existing SqlChangeLogParser.generateId() cached interim-id lookup by broadening the computeIfAbsent exception handling to cover non-DatabaseException errors and adding a matching Spock test.

Suggested labels

TypeEnhancement

Suggested reviewers

wwillard7800
rberezen

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title mentions guarding against null DB connection, but the actual change is broader: widening exception handling from DatabaseException to Exception to catch IllegalArgumentException and other errors, not just null connection issues.	Revise title to be more accurate, such as: 'Widen exception handling in SqlChangeLogParser.generateId to catch non-DatabaseException errors' or 'Fix SqlChangeLogParser.generateId to handle exceptions when building interim ID index.'
Description check	❓ Inconclusive	The description includes comprehensive background, behavior changes, and test coverage, but does not follow the template structure with explicit sections (Impact type checkboxes, structured Description format, Things to be aware of, etc.).	Restructure the description to match the template: add Impact checkboxes (Bug fix is appropriate), use the template sections, and format as specified in the repository guidelines.

✅ Passed checks (3 passed)

Check name	Status	Explanation
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.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/sql-parser-null-connection-offline

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

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

claude · 2026-05-13T14:13:22Z

Claude finished @filipelautert's task in 34s —— View job

Code Review

LGTM — This fix correctly addresses the null connection regression.

Summary

Exception handling widened from DatabaseException to Exception to catch IllegalArgumentException thrown by PreparedStatementFactory(null) in offline mode
The outer try-catch still catches UnexpectedLiquibaseException and falls back to "raw", so the fallback behavior is unchanged
Retry semantics preserved: computeIfAbsent won't cache failed lookups, allowing retries on subsequent calls
New test properly validates the regression: IllegalArgumentException → "raw" fallback

Issues Found

None. The fix is minimal, focused, and the comment explains the rationale clearly.

Next steps: Add one of the required labels (e.g., TypeBug for this regression fix) before merge.

CLAassistant · 2026-05-13T14:14:32Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

wwillard7800

LGTM!

_{Reviewed in CodeRabbit Review Stack}

filipelautert requested a review from wwillard7800 as a code owner May 13, 2026 14:12

filipelautert temporarily deployed to internal May 13, 2026 14:13 — with GitHub Actions Inactive

filipelautert added the skipReleaseNotes label May 13, 2026

filipelautert added this to the 1NEXT milestone May 13, 2026

filipelautert temporarily deployed to internal May 13, 2026 14:28 — with GitHub Actions Inactive

filipelautert temporarily deployed to internal May 13, 2026 14:42 — with GitHub Actions Inactive

filipelautert requested a review from rberezen May 13, 2026 14:51

filipelautert merged commit 4d815ea into main May 13, 2026
62 of 66 checks passed

filipelautert deleted the fix/sql-parser-null-connection-offline branch May 13, 2026 17:54

wwillard7800 approved these changes May 13, 2026

View reviewed changes

Navigation Menu

Guard SqlChangeLogParser.generateId against null DB connection - #7719

Guard SqlChangeLogParser.generateId against null DB connection#7719
filipelautert merged 1 commit into
mainfrom
fix/sql-parser-null-connection-offline

filipelautert commented May 13, 2026

Uh oh!

coderabbitai Bot commented May 13, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

claude Bot commented May 13, 2026 •

edited

Loading

Uh oh!

CLAassistant commented May 13, 2026

Uh oh!

Uh oh!

wwillard7800 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

filipelautert commented May 13, 2026

Summary

Background

Behavior change

Tests

Uh oh!

coderabbitai Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

claude Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review

Summary

Issues Found

Uh oh!

CLAassistant commented May 13, 2026

Uh oh!

Uh oh!

wwillard7800 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented May 13, 2026 •

edited

Loading

claude Bot commented May 13, 2026 •

edited

Loading