terrymanu commented on PR #39832:
URL: https://github.com/apache/shardingsphere/pull/39832#issuecomment-5652996429
### Result
**Review Result: Mergeable**
**Blocking Issues: 0**
The fix is properly scoped and does not change any public API, SQL
statement, or JDBC call count.
Review recommendations: No changes recommended.
### Evidence
- `MySQLMetaDataLoader.java:52-99`
- Uses the existing `PRIMARY` index metadata to identify actual
primary-key columns, preventing a `UNIQUE NOT NULL` column in a table without a
primary key from being misidentified because `COLUMN_KEY=PRI`.
- This behavior is consistent with the [MySQL COLUMNS
documentation](https://dev.mysql.com/doc/refman/8.4/en/information-schema-columns-table.html).
- No additional database queries are introduced. The implementation only
matches already-loaded column and index metadata.
- `MySQLMetaDataLoader.java:169-173`
- Parses `EXTRA` as a combination of attributes, correctly handling
`auto_increment INVISIBLE`.
- `VIRTUAL GENERATED INVISIBLE` is marked as invisible without being
incorrectly treated as a ShardingSphere-generated key. This combination is
documented in [MySQL invisible
columns](https://dev.mysql.com/worklog/task/?id=10905).
- `MySQLMetaDataLoaderTest.java:150-236`
- Covers an actual `PRIMARY` index.
- Covers the false-positive case where `COLUMN_KEY=PRI` is returned for a
non-primary index.
- Covers combined `EXTRA` attributes.
Verification results:
- Focused tests: 6 tests passed.
- MySQL connector module: 85 tests passed.
- Spotless: Passed.
- Checkstyle: 0 violations.
### Coverage
- Candidate: Standalone local candidate.
- Baseline: `47f4a2dbceb9d8272cc5aed420b64c109fd2171d`
- Changed files: 2, both reviewed.
- Root-cause, blast-radius, tests/runtime, and convergence reviews completed.
- CI was not reviewed; this result applies only to the current local code.
- Remaining risk: No integration test against a live MySQL instance was
executed. The behavior is supported by MySQL’s official metadata contract and
focused unit tests.
- The existing Locale sensitivity of `COLUMN_TYPE.toUpperCase()` is outside
the scope of this fix and was not worsened by these changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
