Add MariaDB CI job and integration test for native UUID column#144
Merged
Conversation
The encoding fix from amphp#142 made prepared-statement string parameters round-trip correctly against MariaDB's native UUID column type, but that integration path was never exercised in CI. This adds: - A tests-mariadb job (PHP 8.4 + 8.5) that installs MariaDB via apt and runs the new test against a live server - An integration test that creates a UUID column, inserts via prepared statement, reads back, and asserts the value survives the binary protocol path The test skips on MySQL (no native UUID type) and on MariaDB < 10.7, so it's a no-op on the existing 5 MySQL jobs and only runs on the new MariaDB jobs. Verified locally against MariaDB 12.2.2 (passes). A controlled revert of MysqlEncodedValue::fromValue() to LongBlob encoding reproduces the original "Incorrect uuid value (1292/22007)" failure, confirming the test catches the regression it's meant to guard.
Drops --filter and exercises the whole test suite against MariaDB, with explicit skips for tests that hit MariaDB/MySQL semantic differences: - testDateType YEAR rows: MariaDB rejects CAST(... AS YEAR); MySQL-only syntax. The actual YEAR storage type is unaffected. - testJson: MariaDB has no native JSON wire type (aliased to LONGTEXT) and rejects CAST(... AS JSON). The test was designed around MySQL's JSON column type. - testPrepared parameter-definition assertion: MariaDB returns MYSQL_TYPE_NULL for parameter placeholders by design, rather than the resolved types MySQL returns. Wraps just that block; the rest of the test (column metadata + execution + result iteration) still runs. - testTransactionsCallbacksOnDestruct: rollback callbacks don't fire within delay(0) on MariaDB; needs a separate timing-tolerant test. Adds MysqlTestCase::isMariaDb() helper that does a SELECT VERSION() once and caches the result. Reused by all four skips so future divergences have one obvious place to hook. Locally: 239 tests, 752 assertions, 11 skipped, 0 failures against MariaDB 12.2.2.
Remove reference to non-existent method.
Member
|
I'm not certain why I move the MariaDB CI jobs into the existing matrix to avoid a lot of duplication. Thank you for setting this up! |
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
2 participants
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.
Follow-up to #142. Adds MariaDB to the CI matrix with the full test suite running against a live MariaDB server, plus an integration test exercising the prepared-statement binary protocol against MariaDB's native
UUIDcolumn.What's added
tests-mariadbjob in.github/workflows/ci.yml(PHP 8.4 + 8.5, MariaDB installed via apt)test/MariaDbUuidTest.phpcovering the UUID round-trip: createUUIDcolumn, prepared insert, select, assertMysqlTestCase::isMariaDb()helper that doesSELECT VERSION()once and caches the resultThe UUID test skips on MySQL and on MariaDB < 10.7 (when the native type landed), so it's a no-op on the existing 5 jobs and only runs on the new ones.
Vendor-divergence skips
MysqlDataTypeTest::testDateType(YEAR rows only)CAST(... AS YEAR). MySQL-only cast target; the YEAR storage type itself works on both.MysqlDataTypeTest::testJsonLONGTEXT) and rejectsCAST(... AS JSON). Test was designed around MySQL's JSON column type.MysqlLinkTest::testPreparedparameter-definition assertion onlyMYSQL_TYPE_NULLfor parameter placeholders by design (https://mariadb.com/kb/en/com_stmt_prepare/), unlike MySQL which returns resolved types. The rest of the test (column metadata + execution + result iteration) still runs.MysqlConnectionTest::testTransactionsCallbacksOnDestructdelay(0)on MariaDB. Needs a timing-tolerant variant; out of scope here.Each skip uses
markTestSkippedwith a one-line reason so future readers can find them.Verification
MysqlEncodedValue::fromValue()to the pre-Use VarString for string parameter encoding in binary protocol #142LongBlobencoding: the new UUID test fails withIncorrect uuid value (1292/22007), confirming the regression signal@trowski / @kelunik, happy to adjust scope (e.g., pin a specific MariaDB version, or tighten the
testTransactionsCallbacksOnDestructsituation in a follow-up).