[CAMEL-23481] replacing retired Apache Derby with HSQL and MariaDB in camel-sql#23597
[CAMEL-23481] replacing retired Apache Derby with HSQL and MariaDB in camel-sql#23597tmielke wants to merge 5 commits into
Conversation
stored procedures and stored function tests in sql-stored: component. While HSQL has support for stored procedures, it does not support SQL stored functions, neither does H2 support them. So using an embedded MariaDB4j instance to test SQL stored functions in class SqlFunctionDataSourceTest. Running an embedded MariaDB4j instance is slower to initialize, so using it only for the single test class SqlFunctionDataSourceTest. Made with help from AI tools.
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
It seems derby is using as testing for some other components Error: The project org.apache.camel:camel-spring-jdbc:4.21.0-SNAPSHOT (/home/runner/work/camel/camel/components/camel-spring-parent/camel-spring-jdbc/pom.xml) has 2 errors |
|
Yes, I initially replaced Derby in all components that use it but realised there are quite some changes and decided to go component by component. I need to reintroduce Derby until the last component does not use it. Let me fix that. |
…ntil no component uses Derby.
| <debezium-mysql-connector-version>9.7.0</debezium-mysql-connector-version> | ||
| <derby-version>10.16.1.1</derby-version> |
| <version>${derby-version}</version> | ||
| <groupId>com.h2database</groupId> | ||
| <artifactId>h2</artifactId> | ||
| <version>${h2-version}</version> |
|
🧪 CI tested the following changed modules:
✅ POM dependency changes: targeted tests included Changed properties: debezium-mysql-connector-version, derby-version, mariadb-version, mariadb4j-version Modules affected by dependency changes (9)
All tested modules (233 modules)
|
davsclaus
left a comment
There was a problem hiding this comment.
Thank you for tackling the Derby replacement — this is useful work and the HSQLDB migration for stored procedures looks solid.
I found a couple of issues that need to be addressed before merging:
-
MariaDB4j version 3.3.1 does not exist on Maven Central — the latest released version is 3.2.0. Please verify the correct version.
-
MariaDB4j lacks ppc64le/s390x native binaries — Camel CI tests on
amd64,ppc64le, ands390x. MariaDB4j only ships binaries for linux64 (x86_64), macos-arm64, and win-x64.SqlFunctionDataSourceTestwill fail on those architectures. Consider skipping it on unsupported archs viaskipITs.ppc64le/skipITs.s390xMaven properties or a JUnit condition. -
Tab characters in
components/camel-sql/pom.xmlandparent/pom.xml— the project uses 4-space indentation. -
Commented-out debug code — three test classes have identical commented-out
static {}blocks that are superseded by the surefire configuration. These should be removed. -
Missing trailing newlines in several SQL files.
This review is a project-rules and conventions check. It does not replace specialized review tools such as CodeRabbit or SonarCloud.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Claus Ibsen
| <hibernate-validator-version>9.1.0.Final</hibernate-validator-version> | ||
| <hibernate-version>6.3.2.Final</hibernate-version> | ||
| <hsqldb-version>2.7.4</hsqldb-version> | ||
| <mariadb-version>3.5.3</mariadb-version> |
There was a problem hiding this comment.
MariaDB4j version 3.3.1 does not appear to exist on Maven Central — the latest released version of ch.vorburger.mariaDB4j:mariaDB4j is 3.2.0. Could you verify the correct version? The CI passed but downstream consumers may not have this artifact available.
| <debezium-version>3.5.1.Final</debezium-version> | ||
| <debezium-mysql-connector-version>9.7.0</debezium-mysql-connector-version> | ||
| <derby-version>10.16.1.1</derby-version> | ||
| <debezium-mysql-connector-version>9.7.0</debezium-mysql-connector-version> |
There was a problem hiding this comment.
These two lines use tab indentation instead of 4 spaces, which is inconsistent with the rest of the file:
| <debezium-mysql-connector-version>9.7.0</debezium-mysql-connector-version> | |
| <debezium-mysql-connector-version>9.7.0</debezium-mysql-connector-version> |
| <debezium-mysql-connector-version>9.7.0</debezium-mysql-connector-version> | ||
| <derby-version>10.16.1.1</derby-version> | ||
| <debezium-mysql-connector-version>9.7.0</debezium-mysql-connector-version> | ||
| <derby-version>10.16.1.1</derby-version> |
There was a problem hiding this comment.
| <derby-version>10.16.1.1</derby-version> | |
| <derby-version>10.16.1.1</derby-version> |
|
|
||
| <!-- test dependencies --> | ||
| <dependency> | ||
| <dependency> |
There was a problem hiding this comment.
Tab character used here — should be 8 spaces (2 levels of 4-space indent) to match the rest of the POM:
| <dependency> | |
| <dependency> |
| <version>${derby-version}</version> | ||
| <scope>test</scope> | ||
| </dependency> | ||
| <dependency> |
There was a problem hiding this comment.
Tab character used here:
| <dependency> | |
| <version>${h2-version}</version> |
| private TemplateParser templateParser; | ||
| private EmbeddedDatabase db; | ||
| private JdbcTemplate jdbcTemplate; | ||
| private CallableStatementWrapperFactory factory; |
There was a problem hiding this comment.
This commented-out static {} block is unnecessary since the hsqldb.method_class_names property is already set via the surefire plugin configuration in pom.xml. Please remove this dead code (also applies to ProducerInOutTest and ProducerTest).
| import org.springframework.jdbc.core.JdbcTemplate; | ||
| import org.springframework.jdbc.datasource.DriverManagerDataSource; | ||
| import org.springframework.jdbc.datasource.init.ResourceDatabasePopulator; | ||
|
|
There was a problem hiding this comment.
MariaDB4j ships native binaries only for linux64 (x86_64), macos-arm64, and win-x64. Camel CI also tests on ppc64le and s390x, where this test will fail because no MariaDB binary is available.
Consider:
- Adding
skipITs.ppc64leandskipITs.s390xMaven properties in the POM, or - Using a JUnit
@DisabledIfSystemProperty/ architecture check to skip on unsupported platforms.
[CAMEL-23481] replacing retired Apache Derby with HSQL and MariaDB for stored procedures and stored function tests in
sql-stored:component.While HSQL has support for stored procedures, it does not support SQL stored functions, neither does H2 support them.
So using an embedded MariaDB4j instance to test SQL stored functions in class
SqlFunctionDataSourceTest.Running an embedded MariaDB4j instance is slower to initialize, so using it only for the single test class SqlFunctionDataSourceTest.
Made with help from AI tools.