fix(database): Resolve schema case mismatch in DatabaseDataSet metadata lookup - #928
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideResolves case-mismatch issues in schema-qualified table lookups by normalizing schema names against database metadata when table-name case sensitivity is disabled, and adds regression coverage and changelog entries for issue #656 across H2 unit tests and PostgreSQL integration tests. Sequence diagram for schema-qualified table lookup with case-insensitive schema resolutionsequenceDiagram
participant Client
participant DatabaseDataSet
participant Connection
participant DatabaseMetaData
participant IMetadataHandler
Client->>DatabaseDataSet: getTable(schema, tableName)
activate DatabaseDataSet
DatabaseDataSet->>DatabaseDataSet: initialize(schema)
activate DatabaseDataSet
DatabaseDataSet->>Connection: getConnection()
activate Connection
Connection-->>DatabaseDataSet: jdbcConnection
deactivate Connection
DatabaseDataSet->>Connection: getMetaData()
activate Connection
Connection-->>DatabaseDataSet: databaseMetaData
deactivate Connection
DatabaseDataSet->>DatabaseDataSet: resolveActualSchemaName(databaseMetaData, schema)
activate DatabaseDataSet
DatabaseDataSet->>DatabaseMetaData: getSchemas()
activate DatabaseMetaData
DatabaseMetaData-->>DatabaseDataSet: ResultSet
deactivate DatabaseMetaData
DatabaseDataSet-->>DatabaseDataSet: actualSchema (case-insensitive match or original)
deactivate DatabaseDataSet
DatabaseDataSet->>IMetadataHandler: getTables(databaseMetaData, actualSchema, tableName, types)
activate IMetadataHandler
IMetadataHandler-->>DatabaseDataSet: table metadata
deactivate IMetadataHandler
DatabaseDataSet-->>Client: ITable / NoSuchTableException (unchanged semantics for nonexistent schema)
deactivate DatabaseDataSet
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 49 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesSchema resolution fix
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Test
participant DatabaseDataSet
participant DatabaseMetaData
participant PostgreSQL
Test->>DatabaseDataSet: request schema-qualified table
DatabaseDataSet->>DatabaseMetaData: getSchemas()
DatabaseMetaData-->>DatabaseDataSet: lowercase database schema
DatabaseDataSet->>PostgreSQL: query table metadata using resolved schema
PostgreSQL-->>DatabaseDataSet: matching table
DatabaseDataSet-->>Test: return table data
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider wrapping the call to resolveActualSchemaName() in DatabaseDataSet.initialize() with existing SQLException-to-DataSetException handling so that callers never see raw SQLExceptions and error reporting stays consistent with the rest of the method.
- resolveActualSchemaName() scans all schemas on each initialize() call; if this runs frequently on databases with many schemas, you may want to cache a case-insensitive mapping of schema names per connection to avoid repeated full catalog scans.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider wrapping the call to resolveActualSchemaName() in DatabaseDataSet.initialize() with existing SQLException-to-DataSetException handling so that callers never see raw SQLExceptions and error reporting stays consistent with the rest of the method.
- resolveActualSchemaName() scans all schemas on each initialize() call; if this runs frequently on databases with many schemas, you may want to cache a case-insensitive mapping of schema names per connection to avoid repeated full catalog scans.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
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 `@src/test/java/org/dbunit/ext/postgresql/PostgresqlUppercaseSchemaIT.java`:
- Around line 90-94: Update the AssertJ descriptions in the row-count and
username assertions of PostgresqlUppercaseSchemaIT so each .as() message ends
with a period, preserving the existing assertion logic.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 14bbfcf9-199e-4c9b-9445-e833fad52fe2
📒 Files selected for processing (4)
src/changes/changes.xmlsrc/main/java/org/dbunit/database/DatabaseDataSet.javasrc/test/java/org/dbunit/database/DatabaseDataSet_MultiSchemaTest.javasrc/test/java/org/dbunit/ext/postgresql/PostgresqlUppercaseSchemaIT.java
…ta lookup DatabaseDataSet#initialize() passed a schema-qualified table's schema name straight into IMetadataHandler#getTables(), whose underlying DatabaseMetaData#getTables() matches the schema pattern case sensitively against the live catalog regardless of dbUnit's own FEATURE_CASE_SENSITIVE_TABLE_NAMES setting. A database that folds unquoted identifiers to a canonical case at creation time (e.g. PostgreSQL folds them to lower case) then found zero tables on the first request for a differently-cased schema, and DatabaseDataSet's own per-schema initialization cache permanently remembered that schema as already-initialized-but-empty, so even a later, correctly-cased retry kept failing too. Add DatabaseDataSet#resolveActualSchemaName(), which resolves a requested schema name against DatabaseMetaData#getSchemas()'s actual reported names, case-insensitively, before querying for tables, whenever FEATURE_CASE_SENSITIVE_TABLE_NAMES is off (the default). * Add a DatabaseDataSet_MultiSchemaTest case reproducing the mismatch against H2 (which folds unquoted identifiers upper case) by requesting a table via its lower-cased schema before any other, correctly-cased access has primed the schema cache. * Add PostgresqlUppercaseSchemaIT, confirming the original report's exact scenario against a live PostgreSQL 16 container: a FlatXmlDataSet row qualified with an upper-case schema against a live, unquoted (lower-case) schema, under FEATURE_QUALIFIED_TABLE_NAMES. Refs: 656 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017rt6YcFdhBCwafLp6n7mcZ
e23cc75 to
ffec4a3
Compare
|
Re: Sourcery's two suggestions - both already hold true in the code as written, no change needed:
|
Summary
FlatXmlDataSetrow qualified with a schema name cased differently than the live database (e.g.CORE.USERagainst acoreschema PostgreSQL folded to lower case because it was created unquoted) threwNoSuchTableException, but only on the first access to that schema - a later, differently-cased access to the same schema worked, because it reused metadata cached by the earlier request.DatabaseDataSet#initialize()passed the requested schema name straight intoIMetadataHandler#getTables(), andDatabaseMetaData#getTables()matches its schema pattern case-sensitively against the live catalog regardless of dbUnit's ownFEATURE_CASE_SENSITIVE_TABLE_NAMESsetting. A wrongly-cased first request therefore found zero tables - andDatabaseDataSet's own per-schema initialization cache then permanently remembered that schema as already-initialized-but-empty, so even a later, correctly-cased retry kept failing too.DatabaseDataSetnow resolves a requested schema name againstDatabaseMetaData#getSchemas()'s actual reported names, case-insensitively, before querying for tables, wheneverFEATURE_CASE_SENSITIVE_TABLE_NAMESis off (the default - dbUnit's own case-insensitive-table-names mode). A schema that doesn't exist under any casing still resolves to zero tables exactly as before, so behavior for a genuinely nonexistent schema is unchanged.Test plan
DatabaseDataSet_MultiSchemaTestcase reproduces the mismatch deterministically against H2 (which folds unquoted identifiers to upper case) by requesting a table via its lower-cased schema before any other, correctly-cased access has primed the schema cache - fails withNoSuchTableExceptionbefore the fix, passes after.PostgresqlUppercaseSchemaITconfirms the original report's exact scenario against a live PostgreSQL 16 Docker container: aFlatXmlDataSetrow qualified with an upper-case schema against a live, unquoted (lower-case) schema, underFEATURE_QUALIFIED_TABLE_NAMES,CLEAN_INSERTs and reads back successfully.postgresql-16(358 tests),h2-1-4,hsqldb-2-7, andderby-10-14.🤖 Generated with Claude Code
https://claude.ai/code/session_017rt6YcFdhBCwafLp6n7mcZ
Summary by Sourcery
Resolve schema case-mismatch issues in DatabaseDataSet metadata lookup so qualified table access works reliably when the database folds schema identifiers to a canonical case.
Bug Fixes:
Enhancements:
Documentation:
Summary by CodeRabbit