fix: add information_schema collation applicability view - #27472
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
aunjgr
left a comment
There was a problem hiding this comment.
Reviewed exact head 27a4db4. New tenants initialize the view, existing tenants receive a definition-checked/repaired v4.0.6 upgrade, and the view derives its two MySQL-compatible columns directly from INFORMATION_SCHEMA.COLLATIONS. No blocking issue found.
XuPeng-SH
left a comment
There was a problem hiding this comment.
Request changes on exact HEAD 27a4db4.
[P1 compatibility] The new object must contain the collation mapping; creating an empty view does not fix the ORM introspection path from #25103.
InformationSchemaCollationCharacterSetApplicabilityDDL selects from information_schema.COLLATIONS, but that object is an unpopulated physical table. The new BVT explicitly locks in COUNT(*) = 0. Meanwhile SHOW COLLATION exposes five supported collations from frontend.Collations.
Doctrine DBAL uses this object as data, not only as an existence probe. Its MySQL platform query does:
FROM information_schema.TABLES t
INNER JOIN information_schema.COLLATION_CHARACTER_SET_APPLICABILITY ccsa
ON ccsa.COLLATION_NAME = t.TABLE_COLLATIONWith this HEAD, the empty mapping removes every table from that introspection result. The original missing-object error therefore becomes an empty schema result.
Please populate or derive COLLATIONS/CCSA from one authoritative supported-collation list for both new and upgraded tenants, and add a regression that joins a known information_schema.TABLES row to CCSA and returns its CHARACTER_SET_NAME. At minimum every collation advertised by SHOW COLLATION and used by TABLES.TABLE_COLLATION must have a mapping row.
|
Addressed the latest P1 in
Validation passed with CGO-enabled focused tests: The new head is pushed and CI has been restarted. Please re-review |
aptend
left a comment
There was a problem hiding this comment.
Reviewed exact head a98ea1ffae2d519c48c545771a665374f4965984. The original empty-CCSA blocker is fixed, but one blocking metadata-consistency issue remains; see the inline comment.
Validation: go list, go build, go vet, and the full tests for pkg/util/sysview, pkg/frontend, and pkg/bootstrap/versions/v4_0_6 passed. The focused tests also passed with -race -count=20. A temporary cross-surface invariant test reproduced the reported mismatch and was removed afterward.
|
Addressed the remaining P1 metadata-consistency issue in commit
Validation passed: Please re-review the new head. |
XuPeng-SH
left a comment
There was a problem hiding this comment.
Deep-reviewed exact head 103487f. The two prior blockers are closed: COLLATIONS/CCSA now materialize the canonical six-row mapping for both new and upgraded tenants, and CHARACTER_SETS plus SCHEMATA/TABLES/VIEWS derive the same executable per-charset defaults. The Doctrine-style TABLES-to-CCSA join and cross-surface default invariant are covered. Upgrade checks are idempotent and exact for COLLATIONS; no resource, wait, or unbounded-growth issue found. Local build, vet, full tests for pkg/util/sysview, pkg/frontend, and pkg/bootstrap/versions/v4_0_6, diff check, and current-main merge-tree all passed.
aptend
left a comment
There was a problem hiding this comment.
Re-reviewed exact head a235a32be5819425dcadbed56de56b0753739d12 against the previously reviewed head a98ea1ffae2d519c48c545771a665374f4965984, including all review history/thread state, the incremental diff, and the complete current diff.
The prior cross-surface default-collation blocker is closed: one canonical definition now drives COLLATIONS, CHARACTER_SETS, SCHEMATA, TABLES, VIEWS, SHOW COLLATION, and the v4.0.6 same-version-offset tenant repair. The new CCSA view derives its mapping from the populated COLLATIONS table, and new-tenant ordering plus existing-tenant upgrade ordering are consistent. The exact utf8mb4_0900_ai_ci spelling is executable as the already-supported MySQL-8 compatibility alias, while the advertised PAD SPACE metadata matches MatrixOne's normalized general_ci semantics.
Verification passed: diff check; go list/build/vet for pkg/util/sysview, pkg/bootstrap/versions/v4_0_6, and pkg/frontend; full tests for those package trees; and focused race tests repeated 20 times. The two red Ubuntu jobs are unrelated failures in untouched packages (pkg/sql/plan self-FK routing and pkg/util/metric hardware detection). No blocking issue found.
LeftHandCold
left a comment
There was a problem hiding this comment.
Deep-reviewed exact head 1708610e812ace21b08310c75ce043f46cbcf30e. The latest delta only adds the unknown-charset fallback assertion and does not change production behavior. The full closure remains consistent: one canonical collation definition drives COLLATIONS, CCSA, CHARACTER_SETS, SCHEMATA/TABLES/VIEWS, SHOW COLLATION, new-tenant bootstrap, and the v4.0.6 existing-tenant repair; upgrade ordering and exact-content checks remain idempotent. No correctness, compatibility, leak, hang, or unbounded-growth blocker found. Fresh CGo-controlled full tests passed for pkg/util/sysview, pkg/bootstrap/versions/v4_0_6, and pkg/frontend with -p=1; diff check and merge with current main also passed. Current red CI signatures are in untouched areas (pkg/sql/plan self-FK routing and TestHardwareCPU) and are unrelated to this PR. APPROVE.
Merge Queue Status
This pull request spent 49 seconds in the queue, including 3 seconds running CI. Required conditions to merge
|
What type of PR is this?
Which issue(s) this PR fixes:
issue #25103
What this PR does / why we need it:
information_schema.COLLATION_CHARACTER_SET_APPLICABILITYas a view derived frominformation_schema.COLLATIONS.The JDBC
HAVINGcompatibility fix from PR #27244 is already present on currentmain; this PR addresses the remaining missing information-schema object from issue #25103.Validation
timeout 900s .agents/skills/mo-dev/scripts/mo-cgo-test ./pkg/util/sysview ./pkg/bootstrap/versions/v4_0_6 -count=1git diff --checkFixes #25103