fix: ensure proper type handling for column existence and index creation in MySQLRecordManager#5726
Conversation
…ence checks and index creation
Summary of ChangesHello @prd-hoang-doan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves critical MySQL migration errors by refining type handling and index creation within the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request effectively addresses two critical issues within the MySQLRecordManager. First, it corrects a type comparison bug when checking for column existence by explicitly converting the database result to a number. Second, and more significantly, it resolves a long-standing MySQL migration error by implementing logic to dynamically determine column types and apply appropriate index prefix lengths for TEXT and BLOB columns. These changes enhance the stability and reliability of the MySQL record manager, ensuring smoother database operations and migrations.
| if (Number(Check[0].IndexIsThere) === 0) { | ||
| // Check column data type to determine if prefix length is needed | ||
| const columnTypeCheck = await queryRunner.manager.query( | ||
| `SELECT DATA_TYPE FROM INFORMATION_SCHEMA.COLUMNS | ||
| WHERE table_schema=DATABASE() AND table_name='${tableName}' AND column_name='${column}';` | ||
| ) | ||
|
|
||
| // For TEXT/BLOB columns, use prefix length of 255 | ||
| if (columnTypeCheck.length > 0) { | ||
| const dataType = columnTypeCheck[0].DATA_TYPE.toLowerCase() | ||
| if (dataType.includes('text') || dataType.includes('blob')) { | ||
| await queryRunner.manager.query(`CREATE INDEX \`${column}_index\` | ||
| ON \`${tableName}\` (\`${column}\`(255));`) | ||
| } else { | ||
| await queryRunner.manager.query(`CREATE INDEX \`${column}_index\` | ||
| ON \`${tableName}\` (\`${column}\`);`) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The previous code attempted to create an index on LONGTEXT columns without specifying a prefix length, which is not allowed in MySQL and would lead to a migration error. The new logic correctly identifies TEXT or BLOB column types and applies a prefix length of 255, resolving this critical database migration issue. This is a significant improvement for database compatibility and stability.
if (Number(Check[0].IndexIsThere) === 0) {
// Check column data type to determine if prefix length is needed
const columnTypeCheck = await queryRunner.manager.query(
`SELECT DATA_TYPE FROM INFORMATION_SCHEMA.COLUMNS
WHERE table_schema=DATABASE() AND table_name='${tableName}' AND column_name='${column}';`
)
// For TEXT/BLOB columns, use prefix length of 255
if (columnTypeCheck.length > 0) {
const dataType = columnTypeCheck[0].DATA_TYPE.toLowerCase()
if (dataType.includes('text') || dataType.includes('blob')) {
await queryRunner.manager.query(`CREATE INDEX `${column}_index`
ON `${tableName}` (`${column}`(255));`)
} else {
await queryRunner.manager.query(`CREATE INDEX `${column}_index`
ON `${tableName}` (`${column}`);`)
}
}
}
|
Any idea if and when this is going to be implemented and the issue fix? |
2 participants
fix: ensure proper type handling for column existence and index creation in MySQLRecordManager
Description
Ticket: #5574
Reproduce