Skip to content

Fix MariaDB 11.x collation error in the v4 backfill and log schema fa… - #18

Merged
TheWitness merged 2 commits into
mainfrom
fix/json-type-collation
Nov 8, 2026
Merged

TheWitness merged 2 commits into
mainfrom
fix/json-type-collation

Conversation

@bmfmancini

Copy link
Copy Markdown
Member

The v4 device backfill compared JSON_TYPE() output (server default utf8mb3_uca1400_ai_ci on MariaDB 11.x) with a literal in Cacti's connection collation (utf8mb3_general_ci), failing with error 1267 and leaving the plugin at "Awaiting Configuration". Compare the bytes with CAST(... AS BINARY) instead.

Schema steps also failed silently; every failure path now logs the step, reason and database error to the Cacti log.

Fixes #15

…ilures

The v4 device backfill compared JSON_TYPE() output (server default
utf8mb3_uca1400_ai_ci on MariaDB 11.x) with a literal in Cacti's
connection collation (utf8mb3_general_ci), failing with error 1267 and
leaving the plugin at "Awaiting Configuration". Compare the bytes with
CAST(... AS BINARY) instead.

Schema steps also failed silently; every failure path now logs the step,
reason and database error to the Cacti log.

Fixes #15
Copilot AI balanced review requested due to automatic review settings November 6, 2026 03:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@somethingwithproof somethingwithproof left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocker: the new failure-reporting paths in includes/schema.php fail the existing changed-line coverage gate in all five CI jobs: 11/33 executable lines covered (33.33%). SetupTest covers the v4 write failure and a direct helper call, but leaves the other changed migration failure branches unexercised. Extend the existing tests to drive those database-read/write failures and verify that each migration returns false, logs the correct step, and does not advance its version; alternatively, keep this PR focused on the collation fix. The existing 100% patch gate must pass before approval. The CI unit tests themselves pass; the failure is the subsequent coverage gate.

Each failure path added to includes/schema.php is now driven by a
SetupTest data set: the step returns false, logs one line naming the
step and reason (with the database error only for storage failures),
and does not advance the schema version. A healthy install against the
same fake database logs nothing and reaches v14.

Changed-line coverage of the PR goes from 11/33 to 32/32.
@bmfmancini

Copy link
Copy Markdown
Member Author

Thanks @somethingwithproof. I extended the existing tests rather than narrowing the PR (9d6378d).

SetupTest now has a data set for every failure branch this PR adds to includes/schema.php, covering v1 to v14: the unsupported database version checks, storage reads, failed writes and the InnoDB checks. Each case checks that the step returns false, logs exactly one schema step vN failed (reason) line (with the database error only for storage failures), and does not write a schema version. A companion case runs a full install against the same fake database and checks that it logs nothing and reaches v14, so each failure comes from its own fault rather than from the fake.

Locally, under Pest with pcov, tests/bin/patch-coverage.php reports 32/32 changed lines covered (it was 11/33). The 31 new cases pass, and php-cs-fixer is clean on the changed test.

@sonarqubecloud

sonarqubecloud Bot commented Nov 7, 2026

Copy link
Copy Markdown

@TheWitness
TheWitness merged commit bbf0865 into main Nov 8, 2026
7 checks passed
@TheWitness
TheWitness deleted the fix/json-type-collation branch November 8, 2026 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stuck at "Awaiting Configuration" on MariaDB 11.8: Error 1267 (Illegal mix of collations) in v4 backfill

4 participants