feat(mariadb): Add MariaDB support - #910
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 41 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 (16)
📝 WalkthroughWalkthroughAdds MariaDB support through a dedicated data type factory, MariaDB 11.4 integration-test profile, database environment, schema fixtures, CI configuration, documentation, and release notes. ChangesMariaDB support
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant MariaDbEnvironment
participant MariaDbDataTypeFactory
participant MariaDB
TestRunner->>MariaDbEnvironment: activate mariadb profile
MariaDbEnvironment->>MariaDbDataTypeFactory: configure MariaDB type handling
TestRunner->>MariaDB: run schema and integration test
MariaDB-->>TestRunner: return metadata and round-trip values
Possibly related PRs
🚥 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 |
Reviewer's GuideAdds first-class MariaDB support by introducing a MariaDbDataTypeFactory, a MariaDB-specific test environment and Maven/Docker profile, wiring MariaDB into the CI matrix and docs, and handling MariaDB Connector/J metadata quirks (UUID/INET* type mapping and catalog filtering). Sequence diagram for MariaDB metadata handling and type mappingsequenceDiagram
participant DbUnitTest
participant DatabaseConfig
participant MariaDbDataTypeFactory
participant Connection
participant DatabaseMetaData
participant MySqlMetadataHandler
DbUnitTest->>DatabaseConfig: setProperty(PROPERTY_DATATYPE_FACTORY, MariaDbDataTypeFactory)
DbUnitTest->>Connection: getConnection()
Connection->>DatabaseMetaData: getMetaData()
DatabaseMetaData->>MySqlMetadataHandler: getTables(null, schema, "%", types)
Note over DatabaseMetaData,MySqlMetadataHandler: JDBC URL includes nullCatalogMeansCurrent=true
loop for each column
DatabaseMetaData->>MariaDbDataTypeFactory: createDataType(sqlType, sqlTypeName)
alt sqlType == Types.OTHER and sqlTypeName in {UUID, INET4, INET6}
MariaDbDataTypeFactory-->>DatabaseConfig: DataType.VARCHAR
else other MariaDB/MySQL types
MariaDbDataTypeFactory-->>DatabaseConfig: super.createDataType(sqlType, sqlTypeName)
end
end
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The MariaDB profile declares an additional
mariadb-java-clientdependency without a version, even though a versioned test-scope dependency is already added in the main POM; consider relying on the managed version to avoid future divergence. - The
nullCatalogMeansCurrent=trueJDBC URL parameter is hard-coded in both the GitHub Actions matrix andmariadb-dbunit.properties; consider centralizing this configuration or documenting a single source of truth to reduce the chance of the values drifting.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The MariaDB profile declares an additional `mariadb-java-client` dependency without a version, even though a versioned test-scope dependency is already added in the main POM; consider relying on the managed version to avoid future divergence.
- The `nullCatalogMeansCurrent=true` JDBC URL parameter is hard-coded in both the GitHub Actions matrix and `mariadb-dbunit.properties`; consider centralizing this configuration or documenting a single source of truth to reduce the chance of the values drifting.
## Individual Comments
### Comment 1
<location path="pom.xml" line_range="351-359" />
<code_context>
<version>${hsqldbDriverVersion}</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.mariadb.jdbc</groupId>
+ <artifactId>mariadb-java-client</artifactId>
</code_context>
<issue_to_address>
**question:** The Oracle JDBC dependency in the MariaDB profile looks out of place and could be an accidental inclusion.
In the `mariadb-11-4` profile, there’s both `mariadb-java-client` and `ojdbc8`. If this profile is intended to be MariaDB-only and doesn’t run Oracle tests, consider dropping `ojdbc8` or documenting why it’s needed to avoid unnecessary JDBC drivers and cross-profile coupling.
</issue_to_address>
### Comment 2
<location path="src/main/java/org/dbunit/ext/mysql/MariaDbDataTypeFactory.java" line_range="72-81" />
<code_context>
+ }
+
+ @Override
+ public DataType createDataType(int sqlType, String sqlTypeName) throws DataTypeException
+ {
+ if (sqlType == Types.OTHER)
+ {
+ if (SQL_TYPE_NAME_UUID.equalsIgnoreCase(sqlTypeName)
+ || SQL_TYPE_NAME_INET4.equalsIgnoreCase(sqlTypeName)
+ || SQL_TYPE_NAME_INET6.equalsIgnoreCase(sqlTypeName))
+ {
+ return DataType.VARCHAR;
+ }
+ }
+
+ return super.createDataType(sqlType, sqlTypeName);
+ }
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** createDataType assumes sqlTypeName is non-null, which can lead to a NullPointerException for Types.OTHER with missing type names.
`sqlTypeName` is used in `equalsIgnoreCase` without a null check, so a `null` type name (common for `Types.OTHER` in some drivers) will throw an NPE. Please add a null guard, e.g. an early `if (sqlTypeName == null) return super.createDataType(sqlType, sqlTypeName);` or wrap the comparisons with `sqlTypeName != null && ...` before calling `equalsIgnoreCase`.
</issue_to_address>
### Comment 3
<location path="src/test/java/org/dbunit/ext/mysql/MariaDbDataTypeFactoryTest.java" line_range="44-53" />
<code_context>
+ }
+ }
+
+ @Test
+ void testMariaDbNativeTypes_withUuidInet4Inet6Columns_roundTripThroughDatabase()
+ throws Exception
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for lowercase INET4/INET6 type names, mirroring the UUID case
Since the driver already reports `uuid` in varying cases and you test for that, please also add coverage for lowercase `inet4`/`inet6` (e.g. `createFactory().createDataType(Types.OTHER, "inet4")`) so the `equalsIgnoreCase` behavior is exercised for all MariaDB-specific types.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
jeffjensen
force-pushed
the
706-mariadb-support
branch
from
August 6, 2026 01:49
c7afbcc to
52e91e4
Compare
Member
Author
|
On the two overall comments from Sourcery's review:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/test/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactoryTest.java (1)
59-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEnd the new AssertJ descriptions with periods.
The six
.as("type")descriptions do not end with a period. Keep failure messages consistent with the test convention.Proposed fix
- assertThat(actual).as("type").isSameAs(expected); + assertThat(actual).as("type.").isSameAs(expected);Also applies to: 70-70, 79-79, 87-87, 96-96, 104-104
🤖 Prompt for 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. In `@src/test/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactoryTest.java` at line 59, Update the six AssertJ descriptions in the affected assertions to use the existing “type.” wording, including the assertions near lines 59, 70, 79, 87, 96, and 104, without changing the assertions themselves.Source: Coding guidelines
src/main/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactory.java (1)
67-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd complete JavaDoc to the new public APIs.
The public API documentation is incomplete at these sites.
src/main/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactory.java#L67-L73: Add complete JavaDoc for both public overrides, including parameter, return, and throws descriptions where applicable.src/test/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactoryTest.java#L39-L43: Add JavaDoc for the publiccreateFactory()override.src/test/java/org/dbunit/MariaDbEnvironment.java#L28-L35: Add a complete topic sentence forMariaDbEnvironmentand JavaDoc for its public constructor.src/test/java/org/dbunit/MariaDbEnvironment.java#L59-L65: Add complete JavaDoc forconvertString(String), including parameter and return descriptions.As per coding guidelines, “Write JavaDoc comments on all public classes and methods; use complete sentences beginning with a capital letter and ending with a period for topic text, parameters, and return descriptions.”
🤖 Prompt for 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. In `@src/main/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactory.java` around lines 67 - 73, Complete the JavaDoc for the public overrides getValidDbProducts() and createDataType(int, String) in src/main/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactory.java, including complete topic, parameter, return, and applicable throws descriptions. Document the public createFactory() override in src/test/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactoryTest.java. In src/test/java/org/dbunit/MariaDbEnvironment.java, add a complete topic sentence for MariaDbEnvironment, JavaDoc for its public constructor, and complete parameter and return documentation for convertString(String); ensure all descriptions are capitalized, complete sentences ending with periods.Source: Coding guidelines
🤖 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/site/asciidoc/databases/mariadb.adoc`:
- Around line 12-13: Update both API links in the MariaDB documentation,
including the reference around MariaDbDataTypeFactory and the corresponding link
near the later referenced section, to use deployment-relative
link:../apidocs/... paths instead of root-relative link:/dbunit/apidocs/...
paths. Preserve the existing API targets and link text.
In `@src/site/asciidoc/databases/mysql.adoc`:
- Around line 5-7: Update the MySQL guide text around org.dbunit.ext.mysql to
explicitly state that MariaDB has no dedicated metadata handler and uses
MySqlMetadataHandler, while retaining the reference to MariaDB’s dedicated
factory.
In `@src/test/java/org/dbunit/MariaDbEnvironment.java`:
- Around line 51-56: Update DatabaseEnvironment.getConnection() to call
setupDatabaseConfig(config) before returning the DatabaseConnection, then assign
that configured DatabaseConfig to the returned connection. Preserve
MariaDbEnvironment.setupDatabaseConfig() so its MariaDbDataTypeFactory and
MySqlMetadataHandler overrides are applied.
---
Nitpick comments:
In `@src/main/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactory.java`:
- Around line 67-73: Complete the JavaDoc for the public overrides
getValidDbProducts() and createDataType(int, String) in
src/main/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactory.java, including
complete topic, parameter, return, and applicable throws descriptions. Document
the public createFactory() override in
src/test/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactoryTest.java. In
src/test/java/org/dbunit/MariaDbEnvironment.java, add a complete topic sentence
for MariaDbEnvironment, JavaDoc for its public constructor, and complete
parameter and return documentation for convertString(String); ensure all
descriptions are capitalized, complete sentences ending with periods.
In `@src/test/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactoryTest.java`:
- Line 59: Update the six AssertJ descriptions in the affected assertions to use
the existing “type.” wording, including the assertions near lines 59, 70, 79,
87, 96, and 104, without changing the assertions themselves.
🪄 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: d8fdc9e5-0fba-443c-a5b8-2ae7abff440e
📒 Files selected for processing (16)
.github/workflows/build-any-branch-with-all-dbs.ymldatabase-profiles.txtpom.xmlsrc/changes/changes.xmlsrc/main/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactory.javasrc/site/asciidoc/databases.adocsrc/site/asciidoc/databases/mariadb.adocsrc/site/asciidoc/databases/mysql.adocsrc/site/site.xmlsrc/test/java/org/dbunit/DatabaseEnvironment.javasrc/test/java/org/dbunit/MariaDbEnvironment.javasrc/test/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactoryIT.javasrc/test/java/org/dbunit/ext/mariadb/MariaDbDataTypeFactoryTest.javasrc/test/java/org/dbunit/ext/mysql/MySqlDataTypeFactoryTest.javasrc/test/resources/mariadb-dbunit.propertiessrc/test/resources/sql/mariadb.sql
* Add MariaDbDataTypeFactory in its own org.dbunit.ext.mariadb package (extends org.dbunit.ext.mysql.MySqlDataTypeFactory to reuse its real, verified-shared type handling, but kept in a separate package since MariaDB is its own distinct, independently-branded product - mirroring how org.dbunit.ext.netezza stays independent of org.dbunit.ext.postgresql despite Netezza's Postgres lineage). Declares "mariadb" as a valid database product, silencing the "might cause problems with the current database" warning, and recognizes MariaDB's native UUID (10.7+) and INET4/INET6 (10.10+) column types, which MariaDB Connector/J reports as SQL type OTHER with no MySQL equivalent. MariaDB's JSON type is a LONGTEXT alias and already worked via the inherited longtext handling. * Add a mariadb-11-4 Maven profile, driver dependency, and Docker-backed IT suite (MariaDbEnvironment, MariaDbDataTypeFactoryIT) mirroring the existing mysql-9-20 profile, wired into database-profiles.txt and the all-DBs GitHub Actions matrix. * Add a dedicated, self-contained databases/mariadb.adoc site page and navigation entry instead of folding MariaDB coverage into the MySQL page, so a user who only knows to look for "MariaDB" can find it. * Found and worked around a MariaDB Connector/J gap while wiring up the IT suite: unlike MySQL Connector/J, it has no nullCatalogMeansCurrent-equivalent default, so an unfiltered DatabaseMetaData#getTables() call leaks information_schema/ performance_schema tables into dbUnit's table map, surfacing as a SQLSyntaxErrorException the moment an operation like DELETE_ALL touches one. Fixed via MySqlMetadataHandler registration (passes the schema as the JDBC catalog argument, which the driver does honor) plus nullCatalogMeansCurrent=true on the JDBC URL for defense in depth, and documented both in the new mariadb.adoc page. * Address Sourcery review: add lowercase inet4/inet6 createDataType test cases mirroring the existing lowercase uuid one, and a keep-these-in-sync comment on both copies of the nullCatalogMeansCurrent=true JDBC URL parameter (the workflow matrix's override and mariadb-dbunit.properties' default). * Address CodeRabbit review: clarify mysql.adoc's MariaDB cross- reference wording - MariaDB reuses MySqlMetadataHandler rather than having "its own" metadata handling, which the prior phrasing implied. Refs: 706
jeffjensen
force-pushed
the
706-mariadb-support
branch
from
August 6, 2026 02:08
52e91e4 to
90c4180
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MariaDbDataTypeFactory(extendsMySqlDataTypeFactory): declares"mariadb"as a valid database product (silences the "might cause problems with the current database" warning) and recognizes MariaDB's nativeUUID/INET4/INET6column types, which MariaDB Connector/J reports as SQL typeOTHERwith no MySQL equivalent. MariaDB'sJSONtype is aLONGTEXTalias and already worked via inherited handling.mariadb-11-4Maven profile + Docker-backed IT suite (MariaDbEnvironment,MariaDbDataTypeFactoryIT) mirroring the existingmysql-9-20profile, wired intodatabase-profiles.txtand the all-DBs GitHub Actions matrix.nullCatalogMeansCurrent-equivalent default, so an unfilteredDatabaseMetaData#getTables()call leaksinformation_schema/performance_schematables into dbUnit's table map, surfacing as aSQLSyntaxErrorExceptionthe moment an operation likeDELETE_ALLtouches one. Fixed viaMySqlMetadataHandlerregistration plusnullCatalogMeansCurrent=trueon the JDBC URL; documented both inmysql.adoc.Test plan
./mvnw clean test— 1964 tests, 0 failures./mvnw clean verify -Pmariadb-11-4— 356 tests, 0 failures/errors (Docker, local)./mvnw clean verify -Phsqldb-2-7— sanity check other profiles unaffected./mvnw clean install site— site builds; spot-checked rendereddatabases/mysql.htmlFixes #706
Summary by Sourcery
Add first-class MariaDB support alongside MySQL, including a dedicated data type factory, environment/profile configuration, CI coverage, and documentation updates.
New Features:
Bug Fixes:
Build:
CI:
Documentation:
Tests:
Summary by CodeRabbit
New Features
Documentation